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

process.release.headersUrl: distributors might want to provide an absolute file url #20066

Closed
kapouer opened this issue Apr 16, 2018 · 9 comments
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.

Comments

@kapouer
Copy link
Contributor

kapouer commented Apr 16, 2018

In which case it wouldn't be a .tar.gz file, but a directory (for example file:///usr/include/nodejs on debian).
This would be useful for configurability of node-gyp, as explained in nodejs/node-gyp#1415.

On debian we build nodejs against potentially different versions of shared libraries required by nodejs.
Their respective headers are all available in system-installed places for gyp to discover.
Until now it didn't change anything, however it becomes problematic when building a module using openssl: node-gyp pulls the "official" nodejs 8 headers with openssl 1.0 headers, but the system-installed node has been built against openssl 1.1.

Of course, the node-gyp debian package is already patched so that it uses the right include directory.
But that doesn't prevent npm-installed modules from installing their own node-gyp version which will download the "wrong" headers.

cc-ing @nodejs/delivery-channels here again since many distributions are already using openssl 1.1.

@bnoordhuis bnoordhuis added the build Issues and PRs related to build files or the CI. label Apr 16, 2018
@bnoordhuis
Copy link
Member

That should already work if you do ./configure --release-urlbase=file:///usr/share/nodejs but node-gyp needs to be taught to recognize file paths.

(The file:// prefix might not be necessary, a leading slash should be sufficiently unambiguous.)

@kapouer
Copy link
Contributor Author

kapouer commented Apr 16, 2018

Why i am asking here is to document those two usages:

  • using a file path in headersUrl
  • allowing that file path to be a directory path

Of course node-gyp needs to handle this, but maybe some other software is relying on headersUrl and will break if i start changing it. It's a breaking change !

@kapouer
Copy link
Contributor Author

kapouer commented Apr 16, 2018

@bnoordhuis
Copy link
Member

Meteor is its own thing so probably not a concern.

@kapouer
Copy link
Contributor Author

kapouer commented Apr 26, 2018

Ok, but it's still some kind of API change, isn't it ? It's a documented field.

@kapouer
Copy link
Contributor Author

kapouer commented Apr 28, 2018

It's surely not a good idea to change the behavior of those properties as they are computed in

node/src/node.cc

Lines 3026 to 3046 in b55a11d

#if defined(NODE_RELEASE_URLBASE)
# define NODE_RELEASE_URLPFX NODE_RELEASE_URLBASE "v" NODE_VERSION_STRING "/"
# define NODE_RELEASE_URLFPFX NODE_RELEASE_URLPFX "node-v" NODE_VERSION_STRING
READONLY_PROPERTY(release,
"sourceUrl",
OneByteString(env->isolate(),
NODE_RELEASE_URLFPFX ".tar.gz"));
READONLY_PROPERTY(release,
"headersUrl",
OneByteString(env->isolate(),
NODE_RELEASE_URLFPFX "-headers.tar.gz"));
# ifdef _WIN32
READONLY_PROPERTY(release,
"libUrl",
OneByteString(env->isolate(),
strcmp(NODE_ARCH, "ia32") ? NODE_RELEASE_URLPFX "win-"
NODE_ARCH "/node.lib"
: NODE_RELEASE_URLPFX
"win-x86/node.lib"));
# endif
.
I also note there is a libUrl property for windows.
Maybe a includePath property would be more meaningful here.

@benjamn
Copy link
Contributor

benjamn commented May 15, 2018

Just to be clear, I don't think this feature request has any positive or negative consequences for Meteor. Meteor uses official Node releases when we possibly can (unless there's a critical fix that requires a temporary custom build), so we'll be using the default process.release.headersUrl when we pre-bundle Meteor's dependencies. Meteor is a "distributor" of Node only in the (weak) sense that we bundle a specific version of Node so our developers don't have to install it independently, and so that we can (occasionally) fix Node problems that affect Meteor more urgently than the general Node developer community.

@jasnell jasnell added the doc Issues and PRs related to the documentations. label Jun 19, 2020
@jasnell
Copy link
Member

jasnell commented Jun 19, 2020

There's been zero activity on this for over 2 years. It's still something that could likely be documented but given that it hasn't been picked up yet, it's unlikely. Recommend closing for now.

@jasnell
Copy link
Member

jasnell commented Jun 25, 2020

Closing given lack of responses and activity.

@jasnell jasnell closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

4 participants