Add wix installer toolset in appveyor setup

[mwichmann]

The msi packaging tests won't be run if this is not installed.
These is no change at all to scons itself - so no doc or test, and no change to CHANGES.txt.

Signed-off-by: Mats Wichmann [email protected]

Contributor Checklist:

• I have created a new test or updated the unit tests to cover the new/changed functionality.
• I have updated master/src/CHANGES.txt directory (and read the README.txt in that directory)
• I have updated the appropriate documentation

[bdbaddog]

Should be wixtoolset

[mwichmann]

aaaargh

[coveralls]

Coverage decreased (-0.6%) to 80.079% when pulling db59b0e on mwichmann:add-wixtoolset into ca839c4 on SCons:master.

[mwichmann]

Hmmm, it installs fine but apparently isn't being found, as the tests are still skipped. I had this working after manual installation, does chocolatey not add the path?

139/294 (47.28%) C:\\Python27\\python.exe -tt test\packaging\msi\explicit-target.py
No 'candle' found; skipping test
NO RESULT for test of C:\projects\scons\src\script\scons.py
	at line 46 of test\packaging\msi\explicit-target.py
140/294 (47.62%) C:\\Python27\\python.exe -tt test\packaging\msi\file-placement.py
No 'candle' found; skipping test
NO RESULT for test of C:\projects\scons\src\script\scons.py
	at line 45 of test\packaging\msi\file-placement.py
141/294 (47.96%) C:\\Python27\\python.exe -tt test\packaging\msi\package.py
No 'candle' found; skipping test
NO RESULT for test of C:\projects\scons\src\script\scons.py
	at line 45 of test\packaging\msi\package.py

[mwichmann]

expecting is to have landed in c:\Program Files (x86)\WiX Toolset 3.11\bin

[mwichmann]

So not sure where to go with this. I checked in a clean windows VM; chocolatey installs wix just fine, but does not add it either to the user's or system environment's path. Do we have an extra step for provisioning to do this, or should the tests be more aggressive about seeking out the executables and adding their path to the scons construction environment PATH?

[bdbaddog]

Take a look at what I did with clang and java for windows. Something similar is likely right, and/or we add chocolatey's path to the default windows path.. That might be the simplest.. Try that?

[mwichmann]

No, the choco path won't help, that's mainly its own internal stuff. If the thing it's installing is a standard pkg, it just goes the normal place, for wix it's the one a few comments above in the 32-bit Program FIles. If it's not a full installer thing, it goes into whatever is the ChocolateyToolsLocation path, but even there you get subdirs, keeping the helpful Windows model of everything-gets-their-very-own-directory. All this from experimentation the other day, I'm no choco expert. I had to learn about ChocolateyToolsLocation because it defaults to C:\tools, which an anti-ransomware tool I have objects to and end ends up blocking - things can't write to the "root directory'.

[bdbaddog]

I have a blurb of logic to check reasonable default paths for wix install and add to PATH.
See:
bdbaddog@a30df63

All the msi tests then fail with undefined X_MSI_LANGUAGE. Care to chase it down?

[mwichmann]

Hmmm, choco does make shims for some of the things it installs, I think only for the ones that don't have a classic installer and end up in Program Files* ... so that doesn't include wix.

[bdbaddog]

I added a note with a pointer to a commit on a branch in my repo which fixes finding the tool, but then yields some errors in the logic. Care to take on resolving that and pulling in my location finding logic?

…

On Sat, Oct 6, 2018 at 10:52 AM Mats Wichmann ***@***.***> wrote: Hmmm, choco does make shims for *some* of the things it installs, I think only for the ones that don't have a classic installer and end up in Program Files* ... so that doesn't include wix. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3204 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFBNDJHasHjrx6sBUIghqxN9KmaiuXqks5uiO3tgaJpZM4W_p66> .

[mwichmann]

I seem to

I have a blurb of logic to check reasonable default paths for wix install and add to PATH.
Se
bdbaddog@a30df63

All the msi tests then fail with undefined X_MSI_LANGUAGE. Care to chase it down?

Sure.

I think I put a note on that other commit... it's listed as a required parameter to packaging/msi.py/package(), and nobody (that is, no test) supplies it. Based on some of the other packaging tests, it seems it just comes from the env.Package call, but I don't quite see how the keyword parameters there end up as positional arguments to the packaging tool's call so I'm missing something in the convoluted logic. It should be able to fail more gracefully than that - and indeed there is some logic that fails more gracefully, it's just it fails before that with the undefined value.

[mwichmann]

Hmmm, magical new failure, about multiple environments. e.g. the file-placement test. I don't recall seeing this when I was creating the patch. Guess this will need more work.

File "C:\Users\appveyor\AppData\Local\Temp\1\testcmd.4516.toeggl23\SConstruct", line 19, in <module>
140/294 (47.62%) C:\\Python36\\python.exe -tt test\packaging\msi\file-placement.py
FAILED test of C:\projects\scons\src\script\scons.py
	at line 624 of C:\projects\scons\testing\framework\TestCommon.py (_complete)
	from line 727 of C:\projects\scons\testing\framework\TestCommon.py (run)
	from line 393 of C:\projects\scons\testing\framework\TestSCons.py (run)
	from line 67 of test\packaging\msi\file-placement.py
C:\projects\scons\src\script\scons.py returned 2
STDOUT =========================================================================
scons: Reading SConscript files ...
STDERR =========================================================================
scons: *** Two environments with different actions were specified for the same target: foo-1.2
File "C:\Users\appveyor\AppData\Local\Temp\1\testcmd.4980.bbqbmgk_\SConstruct", line 12, in <module>

[mwichmann]

So I still don't get this: all three msi tests fail with the same kind of error, which claims multiple environments. I see only one environment being created... this is the one from explicit-target.py.

scons: *** Two environments with different actions were specified for the same target: mypackage.msi

[mwichmann]

So this should be finding candle and light without adding the toolset with choco, since we hear the Appveyor images already come with it. Should I remove the .appveyor.yml bit and see if it does find it? I have installed the standard package and it sets the WIX environment varialble, which the patch now fetches; and also the last-chance location should be the correct one.

[bdbaddog]

It doesn't. they're not all in choco's path. I've gone down this route before trust me on this...

…

On Mon, Feb 4, 2019 at 5:49 PM Mats Wichmann ***@***.***> wrote: So this should be finding candle and light *without* adding the toolset with choco, since we hear the Appveyor images already come with it. Should I remove the .appveyor.yml bit and see if it does find it? I have installed the standard package and it sets the WIX environment varialble, which the patch now fetches; and also the last-chance location should be the correct one. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3204 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFBNMY8bzOLdvSL4KGpOwfCowkrQuP3ks5vKLj4gaJpZM4W_p66> .

[dmoody256]

Appveyor responded this: http://help.appveyor.com/discussions/questions/34138-where-is-the-wix-binaries-located

…

On Mon, Feb 4, 2019, 5:25 PM William Deegan ***@***.*** wrote: It doesn't. they're not all in choco's path. I've gone down this route before trust me on this... On Mon, Feb 4, 2019 at 5:49 PM Mats Wichmann ***@***.***> wrote: > So this should be finding candle and light *without* adding the toolset > with choco, since we hear the Appveyor images already come with it. Should > I remove the .appveyor.yml bit and see if it does find it? I have installed > the standard package and it sets the WIX environment varialble, which the > patch now fetches; and also the last-chance location should be the correct > one. > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#3204 (comment)>, or mute > the thread > < https://github.com/notifications/unsubscribe-auth/AAFBNMY8bzOLdvSL4KGpOwfCowkrQuP3ks5vKLj4gaJpZM4W_p66 > > . > — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3204 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF2YzMIyuEQbo88mLRxV0tK45TP8Q_tXks5vKMFjgaJpZM4W_p66> .

[mwichmann]

It doesn't. they're not all in choco's path. I've gone down this route before trust me on this...
…

In my test VM it does find it with the code in this PR, I just tried - and the location is the same as we just heard back from appveyor.

[mwichmann]

Closing this, will proceed with #3294 instead.