Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable NodeReuse by default (FAKE 4) #1600

Merged
merged 1 commit into from Jul 27, 2017
Merged

Disable NodeReuse by default (FAKE 4) #1600

merged 1 commit into from Jul 27, 2017

Conversation

pchinery
Copy link
Contributor

The default for NodeReuse was a bit arbitrary, as it only considered TeamCity, TFS and Jenkins. As we were experiencing problems with NodeReuse on GitLab as well, I suggest to disable this for alle build servers (=> only enable for local builds).

Most likely, environments like AppVeyor, where a new instance is created per build job don't have a problem with this, but as they have so many things being run anyway, I would not expect a measurable performance penalty.

Please gibe any comments if this is acceptable or what else would be to consider. If this is accepted, shall I prepare a PR for the v5/master branch as well?

@0x53A
Copy link
Contributor

0x53A commented Jun 28, 2017

My opinion: n̗ͬò̱ͣ̏̓ͦd̤̄̑e̘̻͔̻̣͇̣ ͖̣̹͍̆̂̃̔͌ͭr͓̽̾e̱̳̹̯͕ͮͨ͂u͇̜̺̦̳͈ͪs͚̖̣͇̒͐͆̚ĕ͓̥̯̔̏ͧ is evil, and should always be disabled.

I've had so many issues with this inside Visual Studio that I have set the global environment variable to disable it.

If I had the permissions, I would add a global domain group policy to force this variable (and disable node reuse) on all machines.

@pchinery
Copy link
Contributor Author

@0x53A: I did not want to put it that drastic, but yes: we had a lot of problems with node reuse (i.e. when changing framework versions and moving between branches of different versions) and would like to remove it. While looking for ways to do this in FAKE, I came across this default for a few arbitrary CI servers and wanted to make it consistent across all CI servers. I do not know about the performance impact, but I would not mind disabling it completely as well (by default).

@matthid
Copy link
Member

matthid commented Jul 8, 2017

I think disable by default is probably fine. I had problems before with it on my local machine as well.

Please send a PR for https://github.com/fsharp/FAKE/blob/master/src/app/Fake.DotNet.MsBuild/MsBuild.fs#L242 as well (info the master branch)

@pchinery pchinery changed the title Disable NodeReuse for all build servers (enable only for local builds) Disable NodeReuse by default (FAKE 4) Jul 10, 2017
@pchinery
Copy link
Contributor Author

I have now just set it to false by default and also created a PR for the master branch.

As a side note: I would like to see this merged as quickly as possible, as we are experiencing problems on our build servers with node reuse

@matthid
Copy link
Member

matthid commented Jul 10, 2017

I thought you could just set the parameter to false for now?

@pchinery
Copy link
Contributor Author

that's not that easy, as the params are hidden in a large cascase of build calls (unleass I am mistaken and have missed something)

@matthid matthid merged commit d0d5b2b into fsprojects:hotfix_fake4 Jul 27, 2017
@matthid
Copy link
Member

matthid commented Jul 28, 2017

Released in https://www.nuget.org/packages/FAKE/4.63.0

@pchinery
Copy link
Contributor Author

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants