Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Skip recycle bin and part fix for moby/moby 32838 #154

Merged
merged 1 commit into from
Jan 3, 2018

Conversation

lowenna
Copy link
Contributor

@lowenna lowenna commented Jan 3, 2018

Signed-off-by: John Howard jhoward@microsoft.com

Changes the legacyLayerReader to skip the recycle bin. In addition, it skips files which are in the recycle bin which have Unicode characters which appear to have lost fidelity between UTF16 and UTF8 conversion. See moby/moby#32838 (comment) for the dockerfile which causes this problem.

@jstarks @darrenstahlmsft PTAL

legacy.go Outdated
// UTF16 to UTF8 in files which are left in the recycle bin. Os.Lstat
// which is called by filepath.Walk will fail when a filename contains
// unicode characters.
if strings.Contains(err.Error(), syscall.Errno(syscall.ERROR_FILE_NOT_FOUND).Error()) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just skip walking Files\$Recycle.Bin altogether? I can't see why a user would ever want to commit the recycle bin directory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically I think you just want to return filepath.SkipDir for that directory so that it doesn't even try to enumerate those files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know that existed. Will take a look, would make a simpler fix certainly.

Signed-off-by: John Howard <jhoward@microsoft.com>
@lowenna lowenna force-pushed the moby-partfix-32838 branch from 79e49ec to 0d5bb78 Compare January 3, 2018 18:20
@lowenna
Copy link
Contributor Author

lowenna commented Jan 3, 2018

@jstarks Updated and verified it fixes the reported issue.

@darstahl darstahl merged commit 45ef154 into master Jan 3, 2018
@lowenna lowenna deleted the moby-partfix-32838 branch January 3, 2018 19:42
dcantah pushed a commit to dcantah/hcsshim that referenced this pull request Mar 17, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants