set_file_perms: if the file is already executable, keep it executable #1141
Conversation
|
Not sure what these traivs failures are, the logs are empty... |
|
Thanks for the PR! Is there a reason we can't just preserve the permissions exactly as they are in the source package? Did previous packages have incorrect permissions, and we want to retain backwards compatibility? I think it would be helpful to have tests covering the specific cases we want to support. |
They definitely used to have incorrect permissions: rust-lang/rust#25479
How would these tests look like? Are tests allowed to access the internet, so that they could download the latest stable, beta, nightly and check the permissions of some files? |
|
We have code in I also think we should try to avoid messing with the permissions as much as possible, given that they are now correct. For example, instead of overwriting all permissions, we could preserve them, but specifically add the "execute" permission to all files from the bin directory, where it's not already present. |
|
Hm, apparantly there is precedent for only preserving the execute bit, in eg. git. - #313 (comment) |
|
Yes. Although Git tree format supports all permission bits, only one bit is actually used. |
Is there any testcase already using that? I couldn't find any, and that makes it kind of hard to figure out how the API is supposed to be used... EDIT: Ah, I found rustup-dist/tests/install.rs. That looks good. There even is a unix_permissions test in there. I could extend that. |
|
All right I (a) did some refactoring in |
|
I can of course change this to just bit-OR |
|
📌 Commit d1d694b has been approved by |
set_file_perms: if the file is already executable, keep it executable This was the smallest change I could come up with to fix <rust-lang/rust#36488>: Maintain the X bit when fixing up permissions. However, the code is still somewhat inconsistent in that it does apply different rules when applied to a file vs. when applied to a directory -- the code recursively walking the directory still removes the X bits from all files, just like it previously ignored the "bin" directory. If you want, I can refactor this some more to be consistent here. Fixes: #1140
|
☀️ Test successful - status-appveyor, status-travis |
This was the smallest change I could come up with to fix rust-lang/rust#36488: Maintain the X bit when fixing up permissions.
However, the code is still somewhat inconsistent in that it does apply different rules when applied to a file vs. when applied to a directory -- the code recursively walking the directory still removes the X bits from all files, just like it previously ignored the "bin" directory. If you want, I can refactor this some more to be consistent here.
Fixes: #1140