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

Review usage of isDirectory in dest symlink implementation #248

Closed
phated opened this issue Jun 23, 2017 · 7 comments
Closed

Review usage of isDirectory in dest symlink implementation #248

phated opened this issue Jun 23, 2017 · 7 comments

Comments

@phated
Copy link
Member

phated commented Jun 23, 2017

As per @erikkemperman's comments in #246, we need to review the way we are handling directories in the symlink path of dest.

@erikkemperman
Copy link
Member

And, depending on the outcome, possibly in symlink as well.

@phated
Copy link
Member Author

phated commented Aug 24, 2017

@erikkemperman I'm not sure if you are still on vacation but when do you think you'll have something for this? We're getting very close to gulp 4 release - just a few dependency updates remaining and then this will become blocking.

I also wanted to ask if you think we could ship 3.0 without this and then if we had a working proposal before gulp 4, we could bump this to 4.0 and get it in gulp 4.

@erikkemperman
Copy link
Member

@phated I'm back -- just insanely busy at work, and on top of that my GF is moving in with me and has, um, ideas about decorating which somehow all involve me getting my powertools out :-)

I have some work in progress:
https://github.com/erikkemperman/vinyl/commits/symbolic-struct
https://github.com/erikkemperman/vinyl-fs/commits/symbolic-struct

Functionally I think it is an improvement, but boy is it ugly :-(

I've tried to add further integration tests for the other case I mentioned here but those are failing.

You're welcome to take a look of course, perhaps you have some quick thoughts on improving it. Unfortunately I probably won't find a lot of time for this any time soon, probably not any at all for the next couple of days...

@phated
Copy link
Member Author

phated commented Aug 25, 2017

Thanks! As long as we have a starting point. (funny enough, I'm moving in with my gf right now too)

@erikkemperman
Copy link
Member

Forgot to answer your second question... This, or something like this, would be breaking in that the stats on the vinyl would now be symbolic after symlink or after dest given a symbolic vinyl.

IMHO the current behavior in that regard is incorrect (not just on Windows with junctions) and so it seems to me that ought to be addressed before 3.0 if at all feasible?

Having said that, I'm not too happy with the approach in the WIP I linked above... Perhaps we can come up with a way to remember both link and target stats within vinyl-fs (needed for handling junctions properly) but not expose the latter externally on symbolic vinyls?

@erikkemperman
Copy link
Member

By the way, congrats! Wish you two all the best!

@phated
Copy link
Member Author

phated commented Nov 10, 2017

Completed in #254

@phated phated closed this as completed Nov 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants