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

Tracking: Revert #5102 when possible #5213

Assignees
Labels
fs Issues and PRs related to the fs subsystem / file system. meta Issues and PRs related to the general management of the project.
Milestone

Comments

@jasnell
Copy link
Member

jasnell commented Feb 13, 2016

PR #5102 was landed as a temporary measure to get npm in master working again. This issue is intended to serve as a reminder that it needs to be reverted as planned.

/cc @ChALkeR @thealphanerd @nodejs/ctc

@jasnell jasnell added fs Issues and PRs related to the fs subsystem / file system. meta Issues and PRs related to the general management of the project. labels Feb 13, 2016
@thefourtheye
Copy link
Contributor

Can we revert after the next npm release or we are going to wait till the major release?

@bnoordhuis
Copy link
Member

The plan is to release in v6 and remove again in v7 to give people time to upgrade their dependencies.

@ChALkeR
Copy link
Member

ChALkeR commented Feb 13, 2016

@jasnell npm is not the sole user of graceful-fs (and there might be other libs that do the same), my proposal was to keep this in 6.x branch and revert in master as soon as 6.x is branched.

@MylesBorins
Copy link
Contributor

@ChALkeR do you know how many modules rely on the pre fix version?

@ChALkeR
Copy link
Member

ChALkeR commented Feb 13, 2016

@thealphanerd Uh-oh. Not yet. For now, I can check only direct dependants.
Data from 2016-01-28, all direct dependants of graceful-fs: https://gist.github.com/ChALkeR/e51abea04b6facfb9bd0.

Note that v4.x are fixed, v3.x and below are affected, so I excluded everything that works with 4.x.

Update: less is fixed in current master, but no release has been published yet, [email protected] still depends on graceful-fs v3.

@ChALkeR
Copy link
Member

ChALkeR commented Feb 13, 2016

@thealphanerd I will check indirect deps too, but I can't say when yet.

@ChALkeR
Copy link
Member

ChALkeR commented Feb 14, 2016

More modules that directly use fs source code in current versions (downloads/month on the left):

1272    guanlecoja-0.6.1.tgz/vendors.js:84525:var src = pre + process.binding('natives').fs + post
1021    bitballoon-0.2.2.tgz/browser/bitballoon.js:8462:var src = pre + process.binding('natives').fs + post
425 eris-db-0.14.1.tgz/test/browser/test_js/test_rpc_ws.js:3231:var src = pre + process.binding('natives').fs + post
223 openapi-node-3.0.3.tgz/pakmanaged.js:4167:    var src = pre + process.binding('natives').fs + post
178 gulp-display-help-1.3.0.tgz/pakmanaged.js:26140:    var src = pre + process.binding('natives').fs + post
36  uber-micro-0.0.0.tgz/pakmanaged.js:1901:    var src = pre + process.binding('natives').fs + post
32  yade-1.3.3.tgz/runtime.js:1601:var src = pre + process.binding('natives').fs + post
21  moduloteste-1.0.0.tgz/graceful-fs/fs.js:8:var src = pre + process.binding('natives').fs + post
20  guanlecoja_test-0.6.0.tgz/vendors.js:61174:var src = pre + process.binding('natives').fs + post
18  crowdjs-0.2.0.tgz/pakmanaged.js:1751:    var src = pre + process.binding('natives').fs + post

Update: All of the above looks like they are being polluted by graceful-fs source code, perhaps in the build process through the deps or so, and one directly copied graceful-fs inside the package.

@ChALkeR
Copy link
Member

ChALkeR commented Feb 14, 2016

@thealphanerd Indirect deps seem to be huge.
For example, current version of karma is 0.13.x (0.13.0 released last summer), and 0.12.x use broken graceful-fs. I will prepare a full checker in the next few days, I hope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment