Skip to content

GitVersionTask throws logging exceptions when task is reused, causing builds to fail. #1643

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

Closed
tpaxatb opened this issue Mar 20, 2019 · 2 comments
Labels
Milestone

Comments

@tpaxatb
Copy link
Contributor

tpaxatb commented Mar 20, 2019

When GitVersionTask is added to a wixproj (and probably other project types), and the build is hosted within Visual Studio (or executed via devenv), the MSBuild Tasks have the potential to be reused (i.e. the Execute() method can be called multiple times on the same Task object instance). Because the Execute() method Resets the Logger hooks at the end of the function, subsequent calls to the Execute causes the logging to fail with an exception and the build process fails, since the logging is not Setup again (it's only performed during construction). This failure does not occur when running via a plain msbuild, since msbuild is directing the build and constructs the tasks in a different manner. The failure does not occur with C# and VB projects because of the fact that those tasks are delegated by visual studio to external build.

There are a couple of wonky design issues I have with the logging:

  1. Failure to set up a logging callback should not cause failure to build.
  2. Since the logging methods is a callback, any object that uses it needs to be in its own AppDomain, because it is static...the last one to set it wins, which most definitely isn't the intended behavior.
  3. The logging should not be reset in the execute function if it did not set it. There is a HUGE assumption that the task object will not be reused by doing so.

(As an aside, it was darn near impossible to figure out how to build and deploy a debug version of GitVersionTask to test the theory because I didn't realize except by trial and error that the UtilPack.NuGet.MsBuild package was actually loading from the nuget cache directory)

(See also issue #1512, this seems to be the root cause)

@arturcic arturcic added this to the 5.0.0 milestone Mar 22, 2019
@arturcic
Copy link
Member

The PR was merged. @tpaxatb can you confirm that the latest version works as expected?

@tpaxatb
Copy link
Contributor Author

tpaxatb commented Mar 22, 2019

Works as expected, I can now build within Visual Studio with no problems. Took me a second to find it, because VS nuget was insisting that 5.0.0-beta2-6 is later than 5.0.0-beta2-42

@tpaxatb tpaxatb closed this as completed Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants