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

Updating WixSharp leads to malfuncion of SetEvVar #569

Closed
digitalsigi opened this issue Jan 16, 2019 · 21 comments
Closed

Updating WixSharp leads to malfuncion of SetEvVar #569

digitalsigi opened this issue Jan 16, 2019 · 21 comments

Comments

@digitalsigi
Copy link

Hi,
recently I updated WiXSharp to V1.9.3. Afterwards the Setup Project did not build anymore. I got a System.ArgumentNullException in a statement refering to Environment variables like this:

            string solutionDir = Environment.GetEnvironmentVariable(EnvSolutionDir);
            string configuration = Environment.GetEnvironmentVariable(EnvConfigurationName);
            string FilesToCollect = Path.Combine(solutionDir, ProjectName, "bin", configuration, "*.*");

Further investigation did reveal that the update process reordered some lines in csproj:
Before:

 <Import Project="..\..\packages\WixSharp.1.9.2\build\WixSharp.targets" Condition="Exists('..\..\packages\WixSharp.1.9.2\build\WixSharp.targets')" />
  <UsingTask AssemblyFile="$(SolutionDir)packages\WixSharp.1.9.2\build\SetEnvVar.dll" TaskName="SetEnvVar" />
  <Target Name="BeforeBuild">
    <SetEnvVar Values="ProjectName=$(ProjectName);ProjectDir=$(ProjectDir);SolutionName=$(SolutionName);SolutionDir=$(SolutionDir);ConfigurationName=$(ConfigurationName)" />
  </Target>

After Update:

  <Target Name="BeforeBuild">
    <SetEnvVar Values="ProjectName=$(ProjectName);ProjectDir=$(ProjectDir);SolutionName=$(SolutionName);SolutionDir=$(SolutionDir);ConfigurationName=$(ConfigurationName)" />
  </Target>
  <Import Project="..\..\packages\WixSharp.bin.1.9.3\build\WixSharp.bin.targets" Condition="Exists('..\..\packages\WixSharp.bin.1.9.3\build\WixSharp.bin.targets')" />
  <Import Project="..\..\packages\WixSharp.1.9.3\build\WixSharp.targets" Condition="Exists('..\..\packages\WixSharp.1.9.3\build\WixSharp.targets')" />
  <UsingTask AssemblyFile="$(SolutionDir)packages\WixSharp.1.9.3\build\SetEnvVar.dll" TaskName="SetEnvVar" />

After manually reordering the UsingTask before SetEnvVar everything was fine again.
Correct order:

  <Import Project="..\..\packages\WixSharp.bin.1.9.3\build\WixSharp.bin.targets" Condition="Exists('..\..\packages\WixSharp.bin.1.9.3\build\WixSharp.bin.targets')" />
  <Import Project="..\..\packages\WixSharp.1.9.3\build\WixSharp.targets" Condition="Exists('..\..\packages\WixSharp.1.9.3\build\WixSharp.targets')" />
  <UsingTask AssemblyFile="$(SolutionDir)packages\WixSharp.1.9.3\build\SetEnvVar.dll" TaskName="SetEnvVar" />
  <Target Name="BeforeBuild">
    <SetEnvVar Values="ProjectName=$(ProjectName);ProjectDir=$(ProjectDir);SolutionName=$(SolutionName);SolutionDir=$(SolutionDir);ConfigurationName=$(ConfigurationName)" />
  </Target>

This is an ugly behavoir and should be avoided.
Josef

@oleg-shilo
Copy link
Owner

Sorry Josef but I will need your help with reproducing what you call "an ugly behavoir".

A simple test with the default project content created from WixSharp template revealed no problems:

image

The initial successful build was done with the NuGet package "WixSharp v1.9.2". After upgrading the package to "WixSharp v1.9.3" I was still able to build and the order of the project package elements stayed the same:
image

If you prefer not to use WixSharp.targets then you will be better off by using "WixSharp.bin" package instead of "WixSharp". This package will not modify project file at all but you will need to "run" the instead of "build" in order to author your msi.

@digitalsigi
Copy link
Author

Hi Oleg,
what you is true. However, you test seems not to include the SetEnvVars Statement. Thats actually the issue. It looks to me, that it gets in the wrong position and seems then to be not working at least not setting the Environment variables which in turn leads to trouble in the WixSharp script. I had this already before.
I will try to provide a sample, but that will take a couple of days.
But may be you can try yourself:

  1. Create a Project with lets say WixSharp 1.9.2 ( I guess any other older will do)
  2. Include SetEnvVar as provided in the code sippet above
  3. In your Build Script get the "SolutionName", "ProjectName" and "Configuration" as shown above
  4. Put them together using Path.Combine
  5. Run it -> should succeed
  6. Play around with the Position of SetEnvVar in csproj. If placed before UsingTask -> not working
  7. Get back to working situation
  8. Use Nuget to update WixSharp to 1.9.3
  9. Inspect csproj file -> UsingTask is now last line -> Build crashes
    About WixSharp.targets: The only thing I did was to add the SetEnvVars in csproj. I do not work with WixSharp.targets because they will - to my knowledge - be replace when upgrading to a new WixSharp Release.
    Hope it helps.

@oleg-shilo
Copy link
Owner

OK, txs.
I will try to have a look at it over weekend.

@oleg-shilo
Copy link
Owner

OK I think I may have an idea about what is going on.

It is not expected that user modifies .csproj file by inserting <SetEnvVar Values="ProjectName=$(ProjectName);. If indeed you want to call SetEnvVar then you need to modify WixSharp.targets instead. This way you have warranty that you will not affect the project file.:

image

I specifically created this target so the project file manipulations during package install/upgrade are safe and do not depend on any other element in the project file. Thus

  • on install
    <UsingTask ... TaskName="SetEnvVar" /> element is appended to the end of the project element.
  • on upgrade
    Existing <UsingTask element is removed and a new instance of it is created and appended to the end of the project element.

@digitalsigi
Copy link
Author

OK. How's about updating WixSharp? Isn't WixSharp.Targets replaced? I believe I had this situation in the past. As workaround I put the setenvvar in csproj. Would it be a solution to create a WixSharpCustom.targets and include it in csproj?

@oleg-shilo
Copy link
Owner

I think you are right. The targets file will be replaced.

Then editing targets is not an option. I will need to see how to change the ps1 algorithm to insert the <UsingTask.. element before <Target Name="BeforeBuild"...`.

Changing the issue to "Enhancement".

Though there is a very simple practical solution that can be used until the fix is available:

  • Install WixSharp NuGet package and adjust project or targets files as you want.
  • Never update WixSharp instead update WixSharp.bin.

That's it.

It will work simply because WixSharp.bin contains the actual WixSharp compilers. And WixSharp contains only ps1 script, targets and code samples. Nothing else.

@digitalsigi
Copy link
Author

Ah, thx, good hint.
As csproj is XML I used it already to update csproj in anoter project. It gives the ability to work with System.XML methods, e.g. InsertBefore. Hope it helps.

@oleg-shilo
Copy link
Owner

Yeah, I know it's XML. XML is not a problem.
I am injecting <UsingTask.. from ps1 so the infrastructure is already there anyway. It's just quite clumsy to do it from PowerShell script comparing to C# but there is no fundamental problem doing it.

Will try to wrap it up rather sooner than later.

@oleg-shilo
Copy link
Owner

oleg-shilo commented Jan 19, 2019

I have done some investigation and found that the problem is not caused by the WixSharp ps1 script but rather by VS NuGet integration. Let's recapture:

Before Update (works):

Injected by VS - <Import Project="..\..\packages\WixSharp.bin.1.9.2\.../>
Injected by VS - <Import Project="..\..\packages\WixSharp.1.9.2\.../>
Injected by WixSharp - <UsingTask AssemblyFile="$(SolutionDir)packages.../>
Injected by manually - <Target Name="BeforeBuild">...>

After Update (does not work):

Injected by manually - <Target Name="BeforeBuild">...>
Injected by VS - <Import Project="..\..\packages\WixSharp.bin.1.9.2\.../>
Injected by VS - <Import Project="..\..\packages\WixSharp.1.9.2\.../>
Injected by WixSharp - <UsingTask AssemblyFile="$(SolutionDir)packages.../>

Now, I deliberately placed there Injected by... so we can see who does what.
The problem is actually caused by the VS injection but not the WxSharp one.

Position of <UsingTask.. is absolutely irrelevant. And since I have no control over NuGet/VS integration I cannot possibly fix it. The only thing I can do is to enable <SetEnvVar... in the WixSharp.targets so you don't have to do it manually.

BTW the only reason I disabled is that I was not sure if people are going need this feature. But since you do.... I have no problems of re-enabling this feature.

Will be available in the next release.

@digitalsigi
Copy link
Author

OK, interesting. Thx for your effort. Sorry for saying "ugly".
I just discovered one more difference:
WixSharp.targets executes Setenvvar "AfterBuild" while I have it placed "BeforeBuild" but probably not before WixSharp.targets which are also set to "AfterBuild".
So I guess there is no other chance as to modify WixSharp.targets when I need different/other/more Env Vars to be set. May be it is possible to somehow migrate this statement during update of WixSharp from previous version or make it version neutral.
For now I'll put a comment in my source file to not forget to update WixSharp.targets.

oleg-shilo pushed a commit that referenced this issue Jan 19, 2019
* VS project templates - added packages.config files
* Added Condition-s for .NET Frameworks 4.7.1 and 4.7.2
* Issue #569: Updating WixSharp leads to malfuncion of SetEvVar
* Added support for `UI.Error` WiX element
* Issue #552: Question: Install 2 windows services in same installer
* Issue #541: Installing 2 Services in the same installer results in …
* Samples update
* Issue #560: Semantic difference between UninstallCondition and IsUninstalling
* Issue #564: Correct variable name of SequentialGuid initialization.
* Issue #562: Typo in Compiler.cs comments
* Issue #561: Typo in WixProject.cs
* Added missing namespace in custom dialog template, fixed comment.
* Added explicit `WixEntity.ComponentId` property
* Issue #551: Cannot include extra .wxs as part of a bundle
* Issue #542: ServiceInstaller.StartOn/StopOn/RemoveOn - Documentation bug
* Issue #544: Failed while processing WebSites; added support for `IISVirtualDir.AttributesDefinition`
@monty241
Copy link
Contributor

After upgrading from 1.9.3 to 1.9.4 through NuGet packages, build raises an error previously not occurring and seemingly be related to this issue and commit:

5>C:\Users\xxx\.nuget\packages\wixsharp\1.9.4\build\WixSharp.targets(5,5): error MSB4036: The "SetEnvVar" task was not found. Check the following: 1.) The name of the task in the project file is the same as the name of the task class. 2.) The task class is "public" and implements the Microsoft.Build.Framework.ITask interface. 3.) The task is correctly declared with <UsingTask> in the project file, or in the *.tasks files located in the "C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\MSBuild\15.0\Bin" directory.
5>Done building project "xxx.csproj" -- FAILED.

After reverting back to 1.9.3 build runs fine again.

The csproj do not contain SetEnvVar. By upgrade from 1.9.3 to 1.9.4, no changes were made to csproj other than the version numbers of packages.

Did not yet find a cause.

@karlchaffey
Copy link

Same here as @monty241 - builds locally but fails on Teamcity with the following (paths redacted)
[MSIAuthoring] ....\packages\WixSharp.1.9.4\build\WixSharp.targets(5, 5): error MSB4062: The "SetEnvVar" task could not be loaded from the assembly ....\WixSharp.1.9.4\build\SetEnvVar.dll. Could not load file or assembly 'file:///.....\WixSharp.1.9.4\build\SetEnvVar.dll' or one of its dependencies. The system cannot find the file specified. Confirm that the declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask.

The dll is however in that path - did something change with the version of the dll or .net version or something? Could explain why it works locally (VS 2017) but not on team city which is running an older version.

@karlchaffey
Copy link

karlchaffey commented Jan 29, 2019

actually correction - the path looks wrong in my build server - it says can't find the following path
C:\TeamCity\BuildAgent3\work\546955958ee6b4e2\[solutionfolder]packages\WixSharp.1.9.4\build\SetEnvVar.dll

There is no slash between [solutionfolder] and packages

@digitalsigi
Copy link
Author

Hi Oleg, updated to 1.9.4 but so success, same as @monty241. I saw the setenvar is now uncommented in WixSharp.Targets but there seems still to be the problem of the order of the using task in csproj.

@digitalsigi
Copy link
Author

Had to revert to 1.9.3 and can confirm, that modifying WixSharp.targets works. Actually I had to add the Configuration=$(Configuration) Parameter.

@oleg-shilo
Copy link
Owner

Guys, I am relying on your feedback here. WixSharp integrattion with VS does not require SetEnvVar targets to be used. This approach was the first implement of the VS integration. Since then I have implemented another less flexible but less intrusive approach that does not require the use of "*.targets". The draw back is the loss of some flexibility.

Until v1.9.4 the use of SetEnvVar.dll was disabled but still present in the package since it was an interesting technical solution.

After this very thread was opened I have reevaluated the benefits of the old SetEnvVar.dll approach and decided to re-enable it. This is what v1.9.4. Though there were no technical reason to do that. Just a single feedback on a less trivial use-case.

But... I am happy to roll it back and disable SetEnvVar.dll for good if it is causing any problems.

What do you (as true WixSharp) users think?

@monty241
Copy link
Contributor

We don't use SetEnvVar (I think). For now, I propose commenting it out completely. Git comes to the rescue when ever needed again :-)

@digitalsigi
Copy link
Author

Hi Oleg, we user SetEnvVar get the configuration name an create slightly different msi packages base on that. Another benefit is, to have the SolutionPath to make thing more independet from actual directory structure. If there is another way to get this information easily its welcome.

@digitalsigi
Copy link
Author

Regarding 1.9.4:
Here I found the problem - which seems to be a Visual Studio one:
I named the main folder "C:\GIT@SOMETHING". This was translated to "file:\\C:\GIT%40SOMETHING",
when renaming it to "C:\GITatSOMETHING" compile with 1.9.4 did run OK.
THe only thing was I had to add the ConfigurationName to SetEnvVar in WixSharp.targets.

@oleg-shilo
Copy link
Owner

OK then. Since 1.9.4 does not exhibit any WiX/WixSharp problems I am leaning towards keeping SetEnvVar enabled. If a true problem does surfaced then I will disable it with the idea that it can be enabled manually.

@oleg-shilo
Copy link
Owner

Thank you guys for the feedback.

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

No branches or pull requests

4 participants