-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
[WIP] Get the wix tool working #3294
Conversation
if wix_env: | ||
wix_installs = [os.path.join(wix_env, 'bin')] | ||
else: | ||
wix_path = r'C:\Program Files (x86)\WiX Toolset v*.*\bin' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only would ever exist in "C:\Program Files (x86)" and not the 64 bit variant "C:\Program Files"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, apparently, the tools themeselves are 32-bit only (which I think is also true of all the visual studio stuff). No 64-bit installer exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, maybe... seems I've later been told if you do find a 32-bit windows install the path used by a installers will be "Program Files", not "Program Files (x86)"... I don't have one of those setups so I don't know if that applies to all packages, some packages, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes That's the case. We had to do this for finding vswhere.exe as well.
On a 32 bit install there is only Program files. On a 64 bit windows install there is Program Files (which is 64 bit), and Program Files (x86) which is 32 bit.
try: | ||
files = os.listdir(p) | ||
if e['WIXCANDLE'] in files and e['WIXLIGHT'] in files: | ||
e.PrependENVPath('PATH', p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the "exists" function to mean "I want to know if this thing currently exists in my environment" not "find and add it to my environment". Is there any documentation regarding the goal of the exists function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find any - I was asking this not long ago on the list (seems everybody just writes a tool by stealing from another tool, which is fine). Yes, I expect you're right about the intent, this is the resurrection of an old patch before I'd seen some of the recent discussion. I'm still willing to add some verbiage to the docs on generate
/exists
if I figure out what we should say.
The problem is... if we find the tool and so report (only), then somebody else has to repeat the work later to find it again, and that's hardly ideal either? We had that issue with the java tests until things got more unified.
return 1 | ||
|
||
# Wix is not in scons' PATH. Let's look in typical install locations. | ||
wix_env = os.environ.get('WIX', None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the construction environment should tend not to be dependent on the system environment?
So this did find wix, and did fail in the way the other PR did, but without forcing the install in appveror. I've closed the older PR, and we can use this one to decide whether further changes to the approach should be made. So far, haven't sorted where the problem comes from. For reference, here is the error we get (all three msi tests show the same):
|
The wix tool uses the WiX Toolset for msi packaging. It isn't working, and is never run by scons' CI infrastructure so the problems are not reported. Add some code for better findability. Problem #1: a language setting is required, and none of the tests do so. Signed-off-by: Mats Wichmann <[email protected]>
exists function is much simplified, and does not set anything. We have have to move some of the version-specific code up into generate, the old exists has been preserved until we see how the experiment works. Signed-off-by: Mats Wichmann <[email protected]>
Sigh. The linux build breaks because the wix test (not the msi packaging test) has a mock wix and the way it's found doesn't work (don't know why it worked before). Should skip. And the windows packaging tests are skipped because... they call whereis to decide whether to proceed or not, but do so in a context where packaging tool hasn't done the setup. So I got no actual answer to me experiment. |
Is there any point in leaving this around, or shall I close and wait for someone to report needing it? |
Withdrawn. We can bring this back from the dead if it becomes a problem. |
The wix tool uses the WiX Toolset for msi packaging. It isn't
working, and is never run by scons' CI infrastructure so the problems
are not reported.
Add some code for better findability.
Problem #1: a language setting is required, and none of the tests do so.
Signed-off-by: Mats Wichmann [email protected]
Contributor Checklist:
master/src/CHANGES.txt
directory (and read theREADME.txt
in that directory)