walkdir: avoid symlink loops when follow_symlinks == false
#35006
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
staticfloat
added
filesystem
staticfloat
added a commit
to JuliaPackaging/BinaryBuilder.jl
that referenced
this pull request
Mar 5, 2020
Julia has a bug with `walkdir()` and symlink loops: JuliaLang/julia#35006
staticfloat
added a commit
to JuliaPackaging/BinaryBuilder.jl
that referenced
this pull request
Mar 5, 2020
Julia has a bug with `walkdir()` and symlink loops: JuliaLang/julia#35006
Because `isdir()` attempts to dereference symlinks, attempting to `walkdir()` trees that contain symlink loops errors out. This change modifies `walkdir()` to treat all symlinks as files when `follow_symlinks == false`.
|
Fantastic, it's working. I'm rebase-squashing down to two commits, then we're good to merge. |
staticfloat
force-pushed
the
sf/walkdir_symlink_loops
branch
from
b46ec9d to
6e2177e
Compare
staticfloat
added a commit
to JuliaLang/Pkg.jl
that referenced
this pull request
Mar 14, 2020
When doing Artifact tests, we need to be resilient to strange umasks that we have inherited from our environment, screwing up git tree hash calculations. To do this, we have a `walkdir()` -> `chmod()` wrapper function that started setting permissions on what it thought were files (due to the fixed `follow_symlink == false` behavior in JuliaLang/julia#35006) but were actually directories. This caused `chmod()` to apply `0o664` permissions to directories, which were then inaccessible to `rm()` as it tried to delete them. This fixes #1716
staticfloat
added a commit
to JuliaLang/Pkg.jl
that referenced
this pull request
Mar 14, 2020
When doing Artifact tests, we need to be resilient to strange umasks that we have inherited from our environment, screwing up git tree hash calculations. To do this, we have a `walkdir()` -> `chmod()` wrapper function that started setting permissions on what it thought were files (due to the fixed `follow_symlink == false` behavior in JuliaLang/julia#35006) but were actually directories. This caused `chmod()` to apply `0o664` permissions to directories, which were then inaccessible to `rm()` as it tried to delete them. This fixes #1716
KristofferC
pushed a commit
that referenced
this pull request
Mar 23, 2020
* walkdir: avoid symlink loops when `follow_symlinks == false` Because `isdir()` attempts to dereference symlinks, attempting to `walkdir()` trees that contain symlink loops errors out. This change modifies `walkdir()` to treat all symlinks as files when `follow_symlinks == false`. * rm: When checking `filemode()`, use `lstat()` to avoid following symlinks (cherry picked from commit 178ac97)
27 tasks
ravibitsgoa
pushed a commit
to ravibitsgoa/julia
that referenced
this pull request
Apr 9, 2020
…ng#35006) * walkdir: avoid symlink loops when `follow_symlinks == false` Because `isdir()` attempts to dereference symlinks, attempting to `walkdir()` trees that contain symlink loops errors out. This change modifies `walkdir()` to treat all symlinks as files when `follow_symlinks == false`. * rm: When checking `filemode()`, use `lstat()` to avoid following symlinks
KristofferC
pushed a commit
that referenced
this pull request
Apr 11, 2020
* walkdir: avoid symlink loops when `follow_symlinks == false` Because `isdir()` attempts to dereference symlinks, attempting to `walkdir()` trees that contain symlink loops errors out. This change modifies `walkdir()` to treat all symlinks as files when `follow_symlinks == false`. * rm: When checking `filemode()`, use `lstat()` to avoid following symlinks
fredrikekre
pushed a commit
to JuliaLang/Pkg.jl
that referenced
this pull request
Apr 17, 2020
When doing Artifact tests, we need to be resilient to strange umasks that we have inherited from our environment, screwing up git tree hash calculations. To do this, we have a `walkdir()` -> `chmod()` wrapper function that started setting permissions on what it thought were files (due to the fixed `follow_symlink == false` behavior in JuliaLang/julia#35006) but were actually directories. This caused `chmod()` to apply `0o664` permissions to directories, which were then inaccessible to `rm()` as it tried to delete them. This fixes #1716 (cherry picked from commit 7366c8c, PR #1721)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Without this change,
walkdir()errors out while trying to determine if a symlink is a directory or not. Whenfollow_symlinksisfalse, we should never attempt to dereference a symlink.This has a slight behavioral change in that valid symlinks to directories are now returned in
filesinstead ofdirswhenfollow_symlinksis set tofalse, which seems like the more correct behavior here.