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
[staging] kernel/manual-config: prevent chmod errors when there is no arch specific Makefile #107525
[staging] kernel/manual-config: prevent chmod errors when there is no arch specific Makefile #107525
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even better than the original :-). Still LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide a commit message that explains the issue and the fix. I had to read the corresponding issue to understand why this change is needed, this should be clear from the commit message.
@@ -249,8 +249,7 @@ let | |||
find . -type f -name '*.lds' -print0 | xargs -0 -r chmod u-w | |||
|
|||
# Keep root and arch-specific Makefiles | |||
chmod u-w Makefile | |||
chmod u-w arch/$arch/Makefile* | |||
chmod u-w Makefile arch/"$arch"/Makefile* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change $arch
to "$arch"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its quoted in all other places in this file. Even if I somewhat doubt people will start adding architectures with spaces in them, its good to be consistent I guess?
…h specific Makefile In such cases the glob pattern might not match anything and chmod will complain about the missing argument.
99f3e33
to
2070808
Compare
I made the commit message more elaborate. This issue with |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
cc @SuperSandro2000 (needs merger) |
Motivation for this change
fix #107346
(same as #107473 but targeted at staging)
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)