-
Notifications
You must be signed in to change notification settings - Fork 28
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
rsc: Disallow hidden file dependencies #1649
Conversation
e6e6e15
to
f56a6d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks correct to me, just one question about error handling.
symlinksUpload | ||
| findFail | ||
|< map getCachePostRequestOutputSymlinkPath | ||
else True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are unable to get the target for an output symlink, we are assuming that means this job didn't output it?
Or are we just choosing to ignore that error here and let some other part of the code handle that failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we just choosing to ignore that error here and let some other part of the code handle that failure
This one. The only failure here is a failure to readlink
and if that fails then another part of the code (just a few lines below) will fully abort the job push
Co-authored-by: Colin Schmidt <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
When a job creates both a file and a symlink to that file but only outputs the symlink it creates a hidden dependency on that untracked file.
This is generally not a huge problem but now is since the RSC is moving to only track true dependencies. The symlink would be restored but not the "hidden" file.
This PR panics when that situation occurs for a cached job. Broken jobs can avoid this by opting out of caching or by also outputting the hidden file