Skip to content
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

Don't clear executable bits in set_readonly() on Windows #3349

Merged
merged 6 commits into from
Feb 28, 2023

Conversation

staticfloat
Copy link
Member

On windows, filemode() doesn't include executable bits. This is already worked around in gitmode() but we don't quite want to do something as intrusive as gitmode() in set_readonly(); let's just work around this one platform-specific foible.

This fixes problems like executable permissions disappearing when creating artifacts on Windows, packages that contain executable files losing those permissions on install, etc...

On windows, `filemode()` doesn't include executable bits.  This is
already worked around in `gitmode()` but we don't quite want to do
something as intrusive as `gitmode()` in `set_readonly()`; let's just
work around this one platform-specific foible.

This fixes problems like executable permissions disappearing when
creating artifacts on Windows, packages that contain executable files
losing those permissions on install, etc...
@staticfloat
Copy link
Member Author

For further discussion, see JuliaPackaging/ArtifactUtils.jl#8 (comment)

@KristofferC
Copy link
Member

Bump :)

@staticfloat
Copy link
Member Author

gulp here we go

@staticfloat staticfloat merged commit 0fb323e into master Feb 28, 2023
@staticfloat staticfloat deleted the sf/executable_read_only_windows branch February 28, 2023 15:51
@staticfloat
Copy link
Member Author

FTR I am not proposing we backport this to 1.9; I consider this a somewhat high-risk change, so let's let it settle on master a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants