Skip to content

Added 'EnsureWixToolsetInstalled' BeforeTarget to GetVersionTask.targets #1220

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

Merged

Conversation

forestb
Copy link

@forestb forestb commented May 17, 2017

The Wix toolset does not trigger any of the following events/targets: CoreCompile;GetAssemblyVersion;GenerateNuspec;_GenerateRestoreProjectSpec, and as a result the GetVersion target never gets triggered, which means none of the environment variables are populated as a result of using MSBuild.

The EnsureWixToolsetInstalled is set as the default InitialTargets="EnsureWixToolsetInstalled" value for all Wix projects.

It would be hugely helpful to have the GitVersion variables seamlessly available as part of the build process for Wix projects!

RFR

@asbjornu
Copy link
Member

This won't fail in non-Wix projects that don't have EnsureWixToolsetInstalled defined?

@forestb
Copy link
Author

forestb commented May 18, 2017

This won't fail in non-Wix projects that don't have EnsureWixToolsetInstalled defined?

Correct.

It used to be that GitVersion was more-incompatible with Wix until a fix was added in GitVersionTask v4.

After adding a reference to a Wix project using GitVersionTask v3.6.5 v.s.
After adding a reference to a Wix project using GitVersionTask v4.Beta

Now that a reference to GitVersionTask.targets is being added automatically, we just need to trigger the GetVersion target in order to populate the environment variables.

The addition I'm proposing only adds the condition, "Hey, I noticed the EnsureWixToolsetInstalled target is about to be executed; Let's execute GetVersion first."

It does not add a dependency, it only subscribes to that additional event if it's found.

@asbjornu
Copy link
Member

Aha, I'm such an MSBuild noob that I misunderstood BeforeTargets to mean "run these targets before the current one", when it seems to mean exactly the opposite, which I understand won't break anything. Excellent! 👍

@asbjornu asbjornu merged commit 105fad6 into GitTools:master May 19, 2017
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.

2 participants