Small item, these two patches appear largely independent. Numbering is primarily needed if there are dependencies between patches. As such there seem any need to number these.
On Thu, Sep 05, 2024 at 02:42:18PM +0200, Florian Eckert wrote: > The 'preinit' script '/lib/preinit/70_initramfs_test' [1] checks whether > the system is running in an 'initramfs'. If this is the case, the loop [2] > in which the function is called is exited via a 'break' call. All further > 'preinit_main' hooks are no longer processed. Therefore, the check whether > we are running in an initramfs is not necessary and are therefore removed. I dislike this wording. Perhaps replace the 3rd sentence with "Further 'preinit_main' hooks are ignored."? The final sentence seemed a caltrop. A better subject line might be "remove duplicate INITRAMFS checks in preinit_main hooks". The main issue is these replicate the test in 70_initramfs_test. 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'm also concerned about the test being done during *every* boot. This doesn't make sense unless there were many devices which can use both overlay and initramfs as root. Perhaps the after initramfs scripts should be split from preinit hooks? Perhaps the files should have a tag inside them and during build scripts which require initramfs should be conditionally enabled? Perhaps base-files should be split into base-files, base-files-initramfs and base-files-noinitramfs? 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. The patch basically looks reasonable, just there are these concerning bits lurking slightly deeper. -- (\___(\___(\______ --=> 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
