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

Upgrade to tar v3 #1212

Closed
wants to merge 2 commits into from
Closed

Upgrade to tar v3 #1212

wants to merge 2 commits into from

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented May 31, 2017

Tar version 3 performs better and is more well tested than its
predecessor. npm will be using this in the near future, so there is no
benefit in shipping a node-gyp that uses the slower and less reliable
fstream-based tar.

Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

lib/install.js Outdated Show resolved Hide resolved
@isaacs
Copy link
Contributor Author

isaacs commented May 31, 2017

Note: This drops support for node 0.x, and thus should be considered a breaking semver-major change, and not landed until those node versions are no longer supported.

@isaacs
Copy link
Contributor Author

isaacs commented May 31, 2017

@bnoordhuis I see at the LTS schedule that 0.12 was only in "maintenance" through 2016-12-31. I think that means it isn't nominally supported? Still a major version bump, of course.

@Fishrock123
Copy link
Member

0.12 is no longer supported by Node.js, whether or not 0.12 is still supported by the current version of node-gyp, I am not really sure.

@addaleax
Copy link
Member

I’d say you can’t just remove 0.x support in a non-major release; too many CI configurations that use node-gyp still have those in them.

@bnoordhuis
Copy link
Member

I suppose that now that node 8 is out, we might as well bump major and drop support for node < 4. There still seem to be quite a few v0.10.x users but those people probably don't upgrade frequently in general and node-gyp in particular.

@refack
Copy link
Contributor

refack commented May 31, 2017

Is there an LTS/maintenance plan for node-gyp?
FWIW there seem to be very low code traffic and IMHO maintaining a parallel v3.x branch should be very easy.

BTW: to the package owners we need a v3.6.2 for #1206, and #1205

@refack
Copy link
Contributor

refack commented May 31, 2017

@isaacs IMHO this PR should come with a "engines" : { "node" : ">=4" }, clause in package.json. Or make this PR dependent on a PR with this change.

@Fishrock123
Copy link
Member

Ehhhh, if people really want a 3.x maintenance we'll probably only know if they complain about it so I'd say leave it be and consider doing something if people post issues about that specifically.

How do we manage these around here? Merge everything into master or another branch or?

@gibfahn
Copy link
Member

gibfahn commented Jun 1, 2017

Ehhhh, if people really want a 3.x maintenance we'll probably only know if they complain about it so I'd say leave it be and consider doing something if people post issues about that specifically.

Agreed

Merge everything into master or another branch or?

Might as well stay on master, we could create a 3.x branch if we need to do more 3.x releases going forward.

@refack
Copy link
Contributor

refack commented Jun 1, 2017

How do we manage these around here? Merge everything into master or another branch or?

I don't know of a president precedent here. I agree with @gibfahn work on master, and if needed create a branch from last known good v3, and backport any requested fixes.

@isaacs
Copy link
Contributor Author

isaacs commented Jun 3, 2017

You all could probably check the nodejs.org log files to see how many 0.x users are downloading gyp headers, but here at npm, we see that 0.10 and 0.12 together account for a little less than 6% of npm registry traffic, and they're almost all using older versions of npm (and thus, won't get a major version bump of node-gyp). All other 0.x node versions make up less than 0.1% of npm registry traffic.

In other words, it's almost certainly safe to drop support for 0.x in a major version bump.

@refack
Copy link
Contributor

refack commented Jun 3, 2017

@isaacs if I read this thread correctly there's consensus on dropping < 4 support in a major version bump.

AFAICT We just don't have an established procedure on how to maintain two version lines of a package (only personal experience). IMHO it does not need to be as strict as node core's. Especial since node-gyp if very low in code traffic.

I say we just wing it 😉 since collectively we have plenty of prior experience in maintaining packages.

Tar version 3 performs better and is more well tested than its
predecessor.  npm will be using this in the near future, so there is no
benefit in shipping a node-gyp that uses the slower and less reliable
fstream-based tar.

This drops support for node 0.x, and thus should be considered a
breaking semver-major change.
@isaacs
Copy link
Contributor Author

isaacs commented Jun 5, 2017

@refack updated package.json engines field to specify >= 4

@refack
Copy link
Contributor

refack commented Jun 6, 2017

@refack updated package.json engines field to specify >= 4

10x, lets see how much the CI is broken

@refack
Copy link
Contributor

refack commented Jun 6, 2017

refack pushed a commit to refack/node-gyp that referenced this pull request Jun 6, 2017
Tar version 3 performs better and is more well tested than its
predecessor.  npm will be using this in the near future, so there is no
benefit in shipping a node-gyp that uses the slower and less reliable
fstream-based tar.

This drops support for node 0.x, and thus should be considered a
breaking semver-major change.

PR-URL: nodejs#1212
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@refack
Copy link
Contributor

refack commented Jun 6, 2017

Landed in 5f924ce & 75cfae2

@refack refack closed this Jun 6, 2017
@richardlau richardlau mentioned this pull request Jul 31, 2018
refack pushed a commit to refack/node-gyp that referenced this pull request Apr 12, 2019
Tar version 3 performs better and is more well tested than its
predecessor.  npm will be using this in the near future, so there is no
benefit in shipping a node-gyp that uses the slower and less reliable
fstream-based tar.

This drops support for node 0.x, and thus should be considered a
breaking semver-major change.

PR-URL: nodejs#1212
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@refack refack mentioned this pull request Apr 12, 2019
3 tasks
rvagg pushed a commit that referenced this pull request Apr 24, 2019
Tar version 3 performs better and is more well tested than its
predecessor.  npm will be using this in the near future, so there is no
benefit in shipping a node-gyp that uses the slower and less reliable
fstream-based tar.

This drops support for node 0.x, and thus should be considered a
breaking semver-major change.

PR-URL: #1212
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants