Skip to content

configure --tag allows any string, but certain characters break npm #21641

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

Closed
apaprocki opened this issue Jul 3, 2018 · 13 comments
Closed

configure --tag allows any string, but certain characters break npm #21641

apaprocki opened this issue Jul 3, 2018 · 13 comments
Labels
build Issues and PRs related to build files or the CI. question Issues that look for answers.

Comments

@apaprocki
Copy link
Contributor

The --tag option lets you append a custom build ID to the Node version. We have a standard build ID that contains characters from the following set [A-Za-z0-9_.+]. Node's configure --tag accepts this fine and the resulting string appears in node -v, but any npm command triggers the warning:

npm WARN invalid config node-version="v10.5.0-BUILD_ID"

... and there seems to be no way to configure the version npm sees to just be v10.5.0.

Is npm too restrictive here or does Node require documentation stating explicitly which characters are allowed to be in the --tag?

@TimothyGu TimothyGu added the build Issues and PRs related to build files or the CI. label Jul 3, 2018
@TimothyGu
Copy link
Member

This issue seems to be two-fold: yes, npm is probably too restrictive. At the same time, I believe we should allow other build tag prefixes than -, which per semver refers specifically to a pre-release. In your case, where a build ID is appended, a + should probably be used.

@apaprocki
Copy link
Contributor Author

apaprocki commented Jul 3, 2018

In our situation, I think it is the _ in the string portion messing it up. We'd like to use a Debian style string so I would pass something like --tag "2+b20180703T185700" and expect the resulting version string to look like v10.5.0-2+b20180703T185700. Would that work with the existing semver code?

@TimothyGu
Copy link
Member

Unfortunately, Debian and semver have different semantics for -. So while they should "work" as in they should both parse, the meaning would be different (in Debian 1.2.3-2 > 1.2.3, but in semver the opposite is true).

@TimothyGu TimothyGu added the question Issues that look for answers. label Jul 3, 2018
@apaprocki
Copy link
Contributor Author

Yeah, just read up on that now.. unfortunate. I'll stick to putting everything behind a +. Do you envision a compatible way to support adding just the +STRING? I suppose to be super clean --tag could remain for the pre-release tagging and a separate --build-id flag could be used to append the string after a + at the end. A tag beginning with + could also override the automatic append of -, but personally I dislike that.

@TimothyGu
Copy link
Member

/cc @nodejs/build-files

@joyeecheung
Copy link
Member

Is the request here about supporting something like 1.2.3+123 as the version (i.e. customizing the separator)? I suppose it should be trivial to add an additional option to configure that customize the separator so it can be done like ./configure --tag "123" --tag-seperator "+"

@apaprocki
Copy link
Contributor Author

I'm proposing keeping --tag the way it is, with the prepended -, and adding a new option --build-id that adds the string after the tag, but with a prepended +.

That lets you do something like --tag pre1 --build-id 20180704T123000 and the resulting version string would be v10.5.0-pre1+20180704T123000. If you do something like --tag-separator then it would never let you append a build identifier to a pre-release version.

@joyeecheung
Copy link
Member

@apaprocki Doesn't --tag pre1+20180704T123000 already do the trick for v10.5.0-pre1+20180704T123000?

@apaprocki
Copy link
Contributor Author

Yes, but that kind of forces the caller of configure to conflate the two things. Not a big deal, but just pointing out that you might want some part of the package build to pass the build ID without knowing anything about if a pre-release was specified. Either way, it would be fine as long as there is some knob to configure it. I temporarily patched configure to remove the prepended - as a temporary workaround.

In other news, adding a semver-compatible + build ID to the Node version broke rails/webpacker#1598

@TimothyGu
Copy link
Member

TimothyGu commented Jul 5, 2018

In reference to the ticket you filed, per semver the build identifier comes after prerelease or patch, so I think what you are trying to do is better represented by 8.11.3-0+b20180704T02124290.

@apaprocki
Copy link
Contributor Author

Yes, but I couldn't do that because if I package, for example, 8.12.0-pre1, then I still need to capture the Debian style build number and ID, so it would have to be 8.12.0-pre1+5-b20180704T02124290 (indicating the 5th rev of 8.12.0-pre1, with a build ID of b20180704T02124290).

@richardlau
Copy link
Member

This probably isn't going to be very helpful, but anecdotally when we first ported Node.js (v0.10.x(!)) onto IBM platforms we initially appended a build ID string via --tag but we ran into a lot of ecosystem compatibility issues (for example node-gyp attempted to download headers based on the version+tag (before process.release existed)).

We tried swapping between _, - and + but was unable to find one that was transparent to the user (different things broke with each character). (It was a long time ago, but IIRC for one of the characters the behaviour was different depending on which version of the semver module was in use). Eventually we internally patched Node.js so that it didn't append the tag to the version string (we had another patch to output the build version+tag in the --help output and the tag was still contained in process.config).

@jasnell
Copy link
Member

jasnell commented Jun 19, 2020

No further activity in nearly 2 years. Closing

@jasnell jasnell closed this as completed Jun 19, 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. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

5 participants