Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

internal/fs: fix os.Chmod on Windows with long paths #925

Merged
merged 1 commit into from
Aug 22, 2017

Conversation

ibrasho
Copy link
Collaborator

@ibrasho ibrasho commented Jul 30, 2017

What does this do / why do we need it?

os.Chmod doesn't work when the passed path is longer than regular path on Windows (<= 248 characters) unless the path in extended-length form.

What should your reviewer look out for in this PR?

Correctness.

Which issue(s) does this PR fix?

Fixes #774

@ibrasho ibrasho requested a review from sdboyer July 30, 2017 19:52
@ibrasho ibrasho force-pushed the fix-chmod-on-windows branch 2 times, most recently from 40956b4 to 07b9077 Compare July 31, 2017 05:23
Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

OK, seems fine - i'm just not even gonna try to fully grok all that path manipulation stuff, and trust that if the test you have in there exceeds the 248 length and passes (which it does) then we're 👍 😄

just need rebase now!

@ibrasho ibrasho force-pushed the fix-chmod-on-windows branch 3 times, most recently from 98c7da5 to 57b5130 Compare August 4, 2017 08:51
@sdboyer
Copy link
Member

sdboyer commented Aug 6, 2017

still having an error on windows tests 😢

@sdboyer
Copy link
Member

sdboyer commented Aug 10, 2017

bump

@ibrasho ibrasho force-pushed the fix-chmod-on-windows branch 3 times, most recently from c94cee9 to 220d940 Compare August 19, 2017 12:22
copy the same functions used in os to convert long paths on Windows to
the extended-length form.

Fixes golang#774

Signed-off-by: Ibrahim AshShohail <[email protected]>
@ibrasho
Copy link
Collaborator Author

ibrasho commented Aug 19, 2017

Turns out the tests were wrong. 😅
Fixed now.

@F21 could you test this branch, please?

@F21
Copy link

F21 commented Aug 20, 2017

@ibrasho Just tested your branch and it works correctly! Thank you :)

@ibrasho ibrasho merged commit a917880 into golang:master Aug 22, 2017
@ibrasho ibrasho deleted the fix-chmod-on-windows branch August 22, 2017 17:15
markwest1 pushed a commit to markwest1/dep that referenced this pull request Aug 24, 2017
copy the same functions used in os to convert long paths on Windows to
the extended-length form.

Fixes golang#774

Signed-off-by: Ibrahim AshShohail <[email protected]>

Document generating graph visualizations golang#975
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chmod fails on Windows
4 participants