-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: gypify msi build #530
Conversation
dd981d0
to
4220fa3
Compare
I got stuck on finding a good way of incorporating code signing (signtool.exe) into the build. |
@piscisaureus what's the problem? it can't just be a separate, intermediate target prior to wix? |
That's the way to go indeed. However I didn't find a way to integrate this windows-only step nicely without affecting all the other platform builds.
|
Any progress here, or should I close this as stale? |
return | ||
|
||
o['variables']['wix_bin_dir'] = os.path.abspath(os.path.join( | ||
os.environ.get('WIX'), '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.
If WIX
is not there in the dictionary we will get NoneType doesn't have ...
error. It's better to fail with KeyError
in that case.
@piscisaureus ... What do you want to do with this one? |
It seemed like a neat idea to put all (windows) build steps inside gyp files, instead of spread out over gyp, a windows batch file, WiX. But I got stuck because I couldn't figure out how to do certain steps with gyp (e.g. signing the Currently I'm not working on this - feel free to close if you think it's not a priority. |
@nodejs/build |
/cc @nodejs/platform-windows |
Not sure if this a priority, though it's an interesting idea 😄 The main reason for using gyp is to generate projects for different platforms / build systems. Since MSI is 100% Windows-specific, dealing with Wix/MSBuild files directly actually seems easier to me. |
Marked as stalled. If there's no activity in the next few weeks I'll go ahead and close. |
Closing due to lack of activity. |
Does not work yet