coreutils
coreutils copied to clipboard
install: fix bad target directory permissions
- Correct bug that set the last directory to the mode the file should have been set to.
- Add unit test to verify correct functionality.
- Fixes #4360
This is ready to go, any feedback?
The test looks good, I'm just not sure I understand the change. You now have a structure like this:
if b.create_leading {
// ...
if !b.create_leading && chmod(...).is_err() {
// ...
}
}
Where b is behind a reference, so isn't mutated. Doesn't that just make the boolean always false? And because the && operator short-circuits we never execute chmod, right? Or am I missing something?
@kylemanna ping?
@tertsdiepraam you're right, it doesn't make sense. Turns out that chmod never need to run in that section, perhaps it a bug from an earlier version of install?
I figure out that you don't need to chmod the leading components, but didn't think much of the surrounding if b.create_leading until you pointed it out.
I've updated the code to never chmod leading components and the unit test remains unchanged from my earlier code.
All tests pass and this makes more sense.
Not sure why that code was introduced, certainly doesn't seem that the code path was subject to a unit-test until now.
Thanks!