lib / rootfs-create: make rm command more robust#9060
Conversation
this fixes the issue raised in armbian/apa#32
WalkthroughModified the rootfs creation script to add the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-10-24T04:46:22.901ZApplied to files:
🧬 Code graph analysis (1)lib/functions/rootfs/rootfs-create.sh (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ This PR has been reviewed and approved — all set for merge! |
|
I'm wondering that this isn't the wrong thing. yes, I did mention it in armbian/apa#32 as a possible solution, but I'm thinking that perhaps this needs doing later in the build process. as late as possible. additionally, I would ask: how has this been tested and under what circumstances would the build break without this but not break with this change. |
|
the current code assumes the existence of a file to be removed. this PR merely relaxes this assumption. the improved code leads to the (apparently) desired outcome in a more robust fashion (file is absent) when before it only did so under a specific circumstance. this code change makes no statement as to whether or not dbus should be installed or when. if you think there is something wrong or if things can be done better still, then by all means go ahead and discuss it or raise a PR |
|
Before this PR, it was, right after debootstrap, deleting the resolv.conf and machine-id files.
At the time of these calls [strictly speaking, immediately after] it made sure the files were absent. This behaviour isn't changed.
The change this PR makes rather is that they could have been absent before the rm calls, and this wasn't allowed, causing a failure. That is, their existence was assumed and expected.
As to ordering, if we start changing when packages get installed by changing dependencies, these files might get instantiated later. In main, APA gets called during image creation which is subsequent to this code.
Or alternatively, we have extensions/network/, which again get called during the image phase. The results of any extension/hook may create the files were they not created by prior pkg installs [which again we are talking about changing how/when pkgs get installed in the near future].
|
|
This is the opposite of making it "more robust". |
this fixes the issue raised in armbian/apa#32
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.