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

Emit symlinks directly #21

Merged
merged 1 commit into from
Jul 16, 2019
Merged

Emit symlinks directly #21

merged 1 commit into from
Jul 16, 2019

Conversation

guybedford
Copy link
Contributor

This implements an internal realpath routine, which then emits symlinks as direct references.

This way "virtual" files within symlinked folders will not ever be emitted - rather the symlink folder itself is emitted, and then the files are emitted at their realpath.

On the consumer side, a direct lstat applied to the fileList can determine if a file is a symlink, and in those cases emit the directory or file symlink as appropriate, but that is the only check needed.

This should allow for a straightforward fix to zeit/now-builders#765.

@guybedford guybedford requested a review from styfle as a code owner July 16, 2019 18:54
@codecov-io
Copy link

Codecov Report

Merging #21 into master will increase coverage by 0.09%.
The diff coverage is 93.47%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #21      +/-   ##
=========================================
+ Coverage   89.51%   89.6%   +0.09%     
=========================================
  Files          10      10              
  Lines         944     972      +28     
=========================================
+ Hits          845     871      +26     
- Misses         99     101       +2
Impacted Files Coverage Δ
src/resolve-dependency.js 100% <100%> (ø) ⬆️
src/node-file-trace.js 89.16% <88%> (-0.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5d6db9...d15875a. Read the comment docs.

@styfle
Copy link
Member

styfle commented Jul 16, 2019

I think this is what @lucleray was talking about: Only emitting the symlink directory.

I am testing with the PR and so far so good https://github.com/zeit/now-builders/pull/765

@styfle styfle merged commit 4e712f9 into master Jul 16, 2019
@styfle styfle deleted the symlink-emit branch July 16, 2019 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants