-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
lib/fs: Handle deduplicated files on NTFS (fixes #1845) #4622
Conversation
lib/fs/fsfileinfo_windows.go
Outdated
|
||
func (e fsFileInfo) Mode() FileMode { | ||
m := e.FileInfo.Mode() | ||
if e.Size() > 0 { |
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.
Checking if symlink bit it set first is cheaper, only then check for size.
lib/fs/fsfileinfo_windows.go
Outdated
if e.Size() > 0 { | ||
return false | ||
} | ||
return e.FileInfo.Mode()&os.ModeSymlink == os.ModeSymlink |
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.
Do you even need this? Can't we just call e.Mode() instead of e.FileInfo.Mode()?
lib/fs/fsfileinfo_windows.go
Outdated
if e.Size() > 0 { | ||
return true | ||
} | ||
return e.FileInfo.Mode().IsRegular() |
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.
Do you even need this? Can't we just call e.Mode() instead of e.FileInfo.Mode()?
lib/fs/fsfileinfo_windows.go
Outdated
|
||
func (e fsFileInfo) IsSymlink() bool { | ||
// N.B. this uses *our* Mode() which appropriately masks this bit. | ||
return e.Mode()&FileMode(os.ModeSymlink) != 0 |
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.
Just ModeSymlink, which is already defined in fs.go.
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.
It actually isn't but maybe it should be
There, now I actually read your comments Audrius. |
lib/fs/fsfileinfo_windows.go
Outdated
m := e.FileInfo.Mode() &^ os.ModeSymlink | ||
return m.IsRegular() | ||
} | ||
return e.FileInfo.Mode().IsRegular() |
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.
Perhaps:
m := e.FileInfo.Mode()
if m&.ModeSymlink && .. .{
m ^= ModeSymlink
}
return m.IsRegular()
These files always have the symlink bit set, because they are reparse points. Nonetheless they are not symlinks, and Lstat reports a size for them. We use this fact to disambiguate, and hope fervently that nothing else matches this description so it comes back to bite us...
Good to go |
@st-review merge |
These files always have the symlink bit set, because they are reparse points. Nonetheless they are not symlinks, and Lstat reports a size for them. We use this fact to disambiguate, and hope fervently that nothing else matches this description so it comes back to bite us... GitHub-Pull-Request: #4622
These files always have the symlink bit set, because they are reparse points. Nonetheless they are not symlinks, and Lstat reports a size for them. We use this fact to disambiguate, and hope fervently that nothing else matches this description so it comes back to bite us...
Testing
@niphlod tested: