Gulp: [epic] vinyl-fs version 3.0.0

[phated]

I'm creating this issue to get some more visibility on outstanding issues in vinyl-fs for it to reach 3.0 (which we'd like to ship with gulp 4).

If you are able to tackle any of these, the help would be very much appreciated.

• [x] Decouple opening and closing of file descriptors from src and dest - gulpjs/vinyl-fs#158
• [x] Support changing uid/gid from the vinyl object to the file on disk - gulpjs/vinyl-fs#157
• [x] Support setting setuid/setgid/sticky bits - gulpjs/vinyl-fs#156
• [x] Support functions for all options - gulpjs/vinyl-fs#139
• [x] kebabCase the filenames - gulpjs/vinyl-fs#152
• [x] vfs.dest(outfolder, { base.. }) ignores outfolder? - gulpjs/vinyl-fs#141
• [x] The mkdirp module doesn't handle modes in the best way - gulpjs/vinyl-fs#165
• [x] Account for process.umask - gulpjs/vinyl-fs#166
• [x] Test refactor - gulpjs/vinyl-fs#194
• [x] Separate prepareWrite into own module - gulpjs/vinyl-fs#193
• [x] Fix Symlink TODOs (issues need to be created)
• [x] Sink the symlink stream - gulpjs/vinyl-fs#195
• [x] Get https://github.com/gulpjs/vinyl-sourcemap up-to-snuff
• [x] Rebase vinyl-fs to generate conventional changelog (https://github.com/gulpjs/vinyl-fs/issues/284)
• [x] Finalize documentation

[phated]

Updated! We've had some excellent contributions by @hgwood and @tmcgee123
Some of the improvements brought to light a couple of other things we need help with.

[phated]

Almost all these are wrapping up. We need to get some more tests in place for the changes and solidify the "Support functions for all options" stuff.

Still looking for someone to tackle the uid/gid stuff.

[odino]

What about a smaller update to vinyl? With this issue on minimatch everyone gets lots of sec. alerts :)

[phated]

@odino no, those alerts don't matter. If you are passing user input directly to gulp, you have bigger problems.

[odino]

That's true, but those alerts will keep coming due to gulp requiring the
library. So even if this has never been exploited people are gonna
"wtf?!?!" anyway unless they really dig into it and figure out that this is
not an issue at all.

If there's an easy way to upgrade to a version that has the patch I would
suggest to go for it, else if its a big PITA then never mind :)
On Jul 12, 2016 1:40 PM, "Blaine Bublitz" [email protected] wrote:

@odino https://github.com/odino no, those alerts don't matter. If you
are passing user input directly to gulp, you have bigger problems.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/gulpjs/gulp/issues/1604#issuecomment-231989564, or mute
the thread
https://github.com/notifications/unsubscribe/AAUC5BPlBGK1FkEyh0WVf2e_q6GkZku2ks5qU2ElgaJpZM4IIqCY
.

[strugee]

@phated the UID/GID item should be checked off, no?

[ilanbiala]

@phated any update on ^^?

[atomantic]

re @odino and security fix. this could be fixed just by updating the version of vinyl-fs that gulp installs. Currently you get vinyl-fs@^0.3.0 which translates to a really old version of vinyl-fs (0.3.14).

This is a problem for anyone requiring nsp check to pass for a build to succeed.

[Den-dp]

this could be fixed just by updating the version of vinyl-fs

@atomantic a lot of things was changed since [email protected] https://github.com/gulpjs/vinyl-fs/blob/master/CHANGELOG.md#v0314-20150921-2357-0000 so it is impossible just bump the version.
In fact, migration to [email protected] is a thing and the whole point of this issue.

[phated]

@strugee @ilanbiala thanks, I've updated the list. However, a few more things had to be added to account for things I've found during the test refactor.

[atomantic]

got it. Yeah, I noticed tests pass as long as I upgrade vinyl-fs to latest version and include glob-watcher 0.0.8--but that's still a really old version of glob-watcher. [email protected] has some differences that break it.

[phated]

@atomantic the gulp tests are just a smoke test because each underlying library has a full test suite that we aren't going to duplicate. Extreme amounts of stuff has changed and you should not assume everything will just work.

[phated]

I've submitted the test refactor for review at https://github.com/gulpjs/vinyl-fs/pull/194 - I'd like to get feedback before I push it so feel free to comment.

[pgilad]

Hey - any news on this? gulp is bringing in an outdated version of minimatch (with security issues)..

[phated]

@pgilad I'm pretty diligent about updating this thread when there are updates, so no. You can follow the linked issues to see progress on the outstanding items (and even contribute). The minimatch warning you mention doesn't mean anything because you aren't passing raw user input to the gulp methods (or you shouldn't be).

[addyosmani]

@phated First, thanks for all the awesome contributions you continue to make to Gulp. Second, looking at the prepareWrite note it appears this is done as of https://github.com/gulpjs/vinyl-fs/commit/cc99707d7acd832af91ca0823bd5fa6388bef168 and the associated tests also landed. Was there more work to do before it could be checked off the list?

[phated]

@addyosmani unfortunately that commit didn't solve everything and caused problems with the symlink method so it was never merged. There are some more option normalizations that are in progress, so I have been focusing on the glob-stream and vinyl dependencies.

[etheranl]

thanks for your contributions

[abhijeetkpawar]

This epic is taking epic time to resolve :(

[strugee]

@abhijeetkpawar everyone working on Gulp is a volunteer. If you want it to go faster, you are welcome to volunteer as well.

[phated]

We've also just received https://github.com/gulpjs/vinyl-sourcemap as a dependency for vinyl-fs. There's still plenty of work to be done on it so any little bit helps.

[mcdado]

I'm not that experienced, but I'd love to help.
I see that there are some points still unchecked. Any suggestions to which to roll up my sleeves at first, be it by priority or accessibility? (this last term used very vaguely, forgive me)

[strugee]

@nhaglind please don't comment on random issues with (somewhat) unrelated problems.

Normally I'd tell you to file a new issue, but it would just be a duplicate of #1571 anyway.

[keyboard-clacker]

Agreed @mcdado! I'd love to help in any way I can, any low hanging fruit that's just kind of tedious work for me to do? I just want to help out any way I can 😊

[strugee]

@phated in the interest of Gulp 4 hitting npm sooner, couldn't some of these issues be pushed past the release? E.g. supporting functions for all options and separating prepareWrite into a separate module don't _seem_ like semver-major changes (although I of course am not an expert). This is especially relevant given that Node 7 is now released and breaking Gulp 3 installs left and right.

[phated]

@strugee no, that's a breaking change. This list is breaking change exclusive and everything else is being pushed. Feel free to contribute on the issues.

[strugee]

@phated ah, okay. I still don't see how, but I'll take your word for it :)

Thanks for the quick response.

[phated]

I need some help writing tests for https://github.com/gulpjs/glob-stream/pull/85 if anyone is interested.

[keyboard-clacker]

I'd be down to help out @phated !

[phated]

@kyleholzinger awesome! shoot me a PR on the glob-stream repository when you have some tests. Many thanks!

[keyboard-clacker]

Will do

[CKGrafico]

Is this the last thing before the release?

[phated]

@CKGrafico this issue is the largest remaining change for the release. There might be small stuff sprinkled around (we've been labeling everything with "help wanted")

[CKGrafico]

Thanks @phated seems not a lot of work I will try to help with something, the community needs this release :)

[phated]

Thanks but we have many issues opened throughout the organization.

You can see everything labeled as "help wanted" at https://github.com/issues?utf8=%E2%9C%93&q=is%3Aopen+org%3Agulpjs+label%3A%22help+wanted%22+

We are tackling some of them this weekend at HackIllinois

[CKGrafico]

Amazing work, I'll wait to next week. Good luck at HackIllinois

[phated]

This is getting really close. I need help with https://github.com/gulpjs/vinyl-sourcemap/issues/19

[anru]

Any progress on this ? Looks like almost things from list are already done. May be I can help somehow ?

gulp still have very old vinyl-fs dependency, and I get following warning each time I install/upgrade gulp:
npm WARN deprecated [email protected]: graceful-fs version 3 and before will fail on newer node releases. Please update to graceful-fs@^4.0.0 as soon as possible

├─┬ [email protected]
│ └─┬ [email protected]
│ ├─┬ [email protected]
│ │ └─┬ [email protected]
│ │ └─┬ [email protected]
│ │ └─┬ [email protected]
│ │ └── [email protected]

[keyboard-clacker]

@anru If you check out that link phated posted there's a bunch of issues left in the org, I think those are all related to the upgrade

[mcdado]

@anru I believe the roadmap is here: https://github.com/gulpjs/gulp/issues/1945

More than the warning the main issue of the current version is the incompatibility with Node > 6. Later this year 8 will be LTS and then it will become more problematic to have gulp as a dependency.

I previously said I'm willing to help, I'll probably take a look at doc related issues.

[anru]

1945 is gulp 4 roadmap. And may be we could release gulp 3 with updated vinyl-fs dependency. Or is it will be more simple to help with gulp 4 where vinul-fs is already updated. What will be more quick/easy ?

I'm asking because there are no incomplete issues in issue description, except Fix Symlink TODOs (issues need to be created)

[strugee]

@anru @mcdado see #1571. The gulp team is aware of the graceful-fs problems, but it's impossible to upgrade Vinyl without breaking changes. Hence, gulp 4.

[subesokun]

Any updates on this? The NSP check fails on all my projects which are using Gulp when scanning also dev dependencies :( https://nodesecurity.io/advisories/118

[ghost]

@subesokun
It seems like that vulnerability requires an attacker to be able to pass an arbitrary glob string to minimatch... which seems like a bad idea in general.

[keyboard-clacker]

Is the "Fix Symlink TODOs (issues need to be created)" item the only thing that's left to do here?

[phated]

@kyleholzinger yes, it is a tough problem we are trying to solve over on the vinyl-fs repo.

[nwhitmont]

What can I do to help move this forward?

[phated]

@nwhitmont the work is happening at https://github.com/gulpjs/vinyl-fs/pull/254

[phated]

We've wrapped up the symlink/junction work. There's just some documentation/rebasing/cleanup work to do on vinyl-fs before the 3.0 release.

If you are interested in this stuff, please go try out the master branch and give us feedback - there are some big changes over there.

[phated]

Documentation has been finalized. Rebase has been completed but it needs review. Check out https://github.com/gulpjs/vinyl-fs/issues/284 if you are interested in reviewing.

[phated]

Based on the reactions, I expected more review/feedback on the rebase :frowning_face:

Anyway, 3.0.0 has been published. Changelog at https://github.com/gulpjs/vinyl-fs/releases/tag/v3.0.0

[CKGrafico]

Thanks for your awesome job @phated

[mcdado]

I tried to have a look but it was a bit overwhelming not knowing anything of the internals.

I second @CKGrafico, thank you and the whole team!