Hello Elliott,
On 2024-09-06 04:40, Elliott Mitchell wrote:
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.
You're right sorry, I summarized it because it makes changes in the same
corner.
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.
I'm not a native speaker. Thanks for the hint. I will definitely adjust
the wording if we agree on how to proceed now.
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.
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.
I have update the wiki page [1] with currently used preinit script
that I have found to get an overview what is going on.
I hope it is complete!
If I have seen it correctly there are some scripts that need an
additional `[ "$INITRAMFS" = "1" ] ||` check if we remove the 'break' in
'70_initramfs_test'. This means that all scripts that have a
number greater than '70' and hook into the 'preinit_main' need
this check. See my change in the wiki `INITRAMFS check 'NO'`
Perhaps the after initramfs scripts should be split from preinit hooks?
If I got the whole picture, we already have a hook 'preinit_mount_root'
for scripts that are executed if we are not run in an initramfs.
Therefore we can move all other scripts to 'preinit_mount_root' and thus
drop the INITRAMFS check? So we only need this check for
'80_mount_root'.
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?
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
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.
Best regards
Florian
[1] https://openwrt.org/docs/techref/preinit_mount#overview
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
