Conversation
Sorry, something went wrong.
|
It would be really nice if this could land in time for Node.js 6.4.0 with the other build fixes. |
Sorry, something went wrong.
There was a problem hiding this comment.
Can you punctuate the comment?
Sorry, something went wrong.
|
LGTM with style nit. CI: https://ci.nodejs.org/job/node-test-pull-request/3535/ To be clear, this only applies to non-ssl Windows builds? |
Sorry, something went wrong.
|
I added a dot in the end. This applies to both ssl and non-ssl:
The main point is to always export zlib symbols, with or without ssl being exported. |
Sorry, something went wrong.
|
A more to the point but less "technical" commit description could be "build: export zlib symbols on Windows". |
Sorry, something went wrong.
|
Rebased, added reviewer & changed title. It passed the tests. |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
Base the generated openssl.def on existing zlib.def. We cannot specify more than one DEF file per executable so we need to merge the two DEF files to expose both OpenSSL and Zlib functionality to addons. If OpenSSL is not used, link against zlib.def itself. PR-URL: #7983 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Could this get merged this week? I do rebase it frequently. I have one other pending commit that is tagged semver-minor and will get into 6.4.0: #7576 because of similar changes. So I guess this commit also should be tagged semver-minor and thus it needs to be committed in time for 6.4.0. |
Sorry, something went wrong.
|
CI is green so I'm okay with it getting merged. That particular configuration is not tested, of course, but it's unlikely to make matters worse than they currently are. /cc @nodejs/release - there is no land-on-v6.x label, how should I tag it for v6? |
Sorry, something went wrong.
|
If it cherry-picks cleanly, then you really don't need to do anything. |
Sorry, something went wrong.
|
One more CI: https://ci.nodejs.org/job/node-test-commit/4444/ |
Sorry, something went wrong.
Base the generated openssl.def on existing zlib.def. We cannot specify more than one DEF file per executable so we need to merge the two DEF files to expose both OpenSSL and Zlib functionality to addons. If OpenSSL is not used, link against zlib.def itself. PR-URL: #7983 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Thanks 🎉 |
Sorry, something went wrong.
Base the generated openssl.def on existing zlib.def. We cannot specify more than one DEF file per executable so we need to merge the two DEF files to expose both OpenSSL and Zlib functionality to addons. If OpenSSL is not used, link against zlib.def itself. PR-URL: #7983 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Notable changes: * build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576 * child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838 * fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942 * repl: The REPL now supports editor mode. (Prince J Wesley) #7275 * util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013 Refs: #8020 PR-URL: #8070
Notable changes: * build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576 * child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838 * child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) #7696 * fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942 * repl: The REPL now supports editor mode. (Prince J Wesley) #7275 * util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013 Refs: #8020 PR-URL: #8070
Notable changes: * build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576 * child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838 * child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) #7696 * fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942 * repl: The REPL now supports editor mode. (Prince J Wesley) #7275 * util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013 Refs: #8020 PR-URL: #8070
Notable changes: * build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) nodejs#7983 and nodejs#7576 * child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) nodejs#7811 and nodejs#7838 * child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) nodejs#7696 * fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) nodejs#7942 * repl: The REPL now supports editor mode. (Prince J Wesley) nodejs#7275 * util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) nodejs#8013 Refs: nodejs#8020 PR-URL: nodejs#8070
Checklist
Affected core subsystem(s)
build, addons
Description of change
Base the generated openssl.def on existing zlib.def. We cannot specify more than one DEF file per executable so we need to merge the two DEF files to expose both OpenSSL and Zlib functionality to addons.
If OpenSSL is not used, link against zlib.def itself.
@bnoordhuis knows about the mkssldef.py