-
Notifications
You must be signed in to change notification settings - Fork 1.3k
contenthash: clear ModeIrregular before sending to archive/tar #5665
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
Conversation
Clears ModeIrregular from the checksum generation. This may be sent by the client when the version of Go used is post Go 1.23. The behavior of `os.Stat` was modified in Go 1.23 to set `ModeIrregular` on reparse points in Windows. This clears `ModeIrregular` when any mode is set which was the previous behavior of `os.Stat`. Signed-off-by: Jonathan A. Sternberg <[email protected]>
5887a6c to
45177da
Compare
tonistiigi
left a comment
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.
So are these files created as regular empty files on transfer?
| // bit to regular files with non-handled reparse points. | ||
| // Current versions of Go now apply them to directories too. | ||
| // archive/tar.FileInfoHeader does not handle the irregular bit. | ||
| if stat.Mode&uint32(os.ModeType&^os.ModeIrregular) != 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.
Isn't the negation wrong in the condition here?
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.
The intention is to use os.ModeType but to remove os.ModeIrregular because os.ModeIrregular&os.ModeType will always return non-zero. so this uses the os.ModeType constant but removes os.ModeIrregular from it.
It's a bit tricky because the original condition checks if m&os.ModeType == 0 and sets os.ModeIrregular if it is. But, because we've now set os.ModeIrregular, the condition will always be true so the reverse doesn't work.
|
I think they should get created as regular directories. Files will be excluded from this behavior and will still return an error which is what they previously did. While I used |
Where is that error happening? |
|
It's happening inside of https://cs.opensource.google/go/go/+/refs/tags/go1.23.4:src/archive/tar/common.go;l=677 |
|
When should this make it into a release? I'm still getting Docker Desktop 4.41.2 (191736)
|
|
@PeterDraex do you have a reproducer by any chance? This should be fixed by now. There might be something we've missed. |
|
I've opened another issue to fix this. I think I may have made an incorrect assumption in this original fix. I assumed that another mode type would be set at the same time as These seem to have been invalid assumptions. Looking at the string output of the file mode in both @PeterDraex and the original issue, I see that only |
Clears ModeIrregular from the checksum generation. This may be sent by the client when the version of Go used is post Go 1.23. The behavior of
os.Statwas modified in Go 1.23 to setModeIrregularon reparse points in Windows. This clearsModeIrregularwhen any mode is set which was the previous behavior ofos.Stat.Fixes docker/for-win#14083.