[PATCH 2/2] treewide: remove INITRAMFS check for preinit_main hook

Elliott Mitchell ehem+openwrt at m5p.com
Thu Sep 5 19:40:10 PDT 2024


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+sigmsg at m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445





More information about the openwrt-devel mailing list