On Fri, Sep 06, 2024 at 02:15:22PM +0200, Florian Eckert wrote: > > On 2024-09-06 04:40, Elliott Mitchell wrote: > > > > Examining this situation, I wonder whether this is really the right way > > to go. > > > > There are 29 files which match [789]* (excluding `70_initramfs_test`). > > So you found this redundant check in >10% of files. This seems a rather > > high percentage. Perhaps it might be better to remove the `break` from > > `70_initramfs_test` and instead require explicitly checking $INITRAMFS ? > > I thought about that too, but decided against it, as I would then have to > adapt a lot more, as you mentioned. I have to admit that at the beginning > I didn't understand why my script wasn't called in 'preinit_main' until > I saw that there was a 'break' in 'boot_run_hook'. > > I would not have expected that! > > I would therefore also prefer as you notice the same to delete 'break' and > check the INITRAMFS flag. That makes the whole thing more transparent and > clear.
These haven't been through the copy/update cycle of the kernel configuration files, so `git blame` works on them: f43b7934d285 package/base-files/files/lib/preinit/80_mount_root John Crispin <j...@openwrt.org> 2013-03-13 11:11:19 a0b7fef0ffe4 package/utils/zyxel-bootconfig/files/95_apply_bootconfig David Bauer <m...@david-bauer.net> 2022-07-20 12:52:06 fe0081eecf43 target/linux/bcm27xx/base-files/lib/preinit/81_set_root_part Álvaro Fernández Rojas <nolt...@gmail.com> 2024-03-04 07:28:57 The authors are all members of the project and should therefore be pretty knowledgeable about OpenWRT. If even members of the project are getting it wrong, that seems strong evidence the existing approach doesn't work well. Add us and I think we've got a consensus the `break` really should be removed and checking in every script should be used. Though there are the other alternative approaches below. > > Perhaps the files should have a tag inside them and during build scripts > > which require initramfs should be conditionally enabled? > > What exactly do you mean by that? Should the scripts always be installed, > but if an initramfs is built, then the hook line is deleted? I was thinking something along the lines of having a in-comment tag for scripts which should be omitted from initramfs. Perhaps "# INITRAMFS_OMIT" and "# INITRAMFS_INCLUDE"? When building the initramfs use `grep -l` or `grep -L` to identify files which should (not) be included. > > Perhaps base-files should be split into base-files, base-files-initramfs > > and base-files-noinitramfs? > > This would be a big change from my point of view, because in base-files > are not only the preinit scripts but also many other things. We would > have to maintain this redundantly, if I understand your intention correctly This might be too big of a change for now. This may well be a bad idea right now and in the future. > > In other news 18 of the 29 [789]* files are named "79_move_config" > > inside > > target/linux. This makes me wonder how similar the job they're doing > > is. > > Perhaps these should all merge into a single file in package/base-files. > > This would make sense if the scripts were all the same, but on closer > look they are all different and are target/subtarget specific. Therefore, > from my point of view, it makes no sense to move them to the base-files. Indeed. A quick glance suggested many are pretty similar and I wonder if it would be possible to write a single script which could replace all of them. I suspect most of them started as copies and they've simply been slowly drifting apart. -- (\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/) \BS ( | ehem+sig...@m5p.com PGP 87145445 | ) / \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445 _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
