| Message ID | 20230120083609.8110-1-fe@dev.tdt.de |
|---|---|
| State | New |
| Headers | show |
| Series | x86: use mkfs.fat, sed, mmd and mcopy from staging_dir | expand |
On 20.01.23 09:36, Florian Eckert wrote: > During image generation, the host tools should not be used but the tools > from the staging_dir. > > - mkfs.fat > - sed > - mmd > - mcopy Why is this necessary? $STAGING_DIR_HOST/bin should already be in $PATH before the host system parts. - Felix
Hello Felix, >> During image generation, the host tools should not be used but the >> tools >> from the staging_dir. >> >> - mkfs.fat >> - sed >> - mmd >> - mcopy > Why is this necessary? $STAGING_DIR_HOST/bin should already be in > $PATH before the host system parts. I only noticed that the prefix $(STAGING_DIR_HOST) is missing for these tools on x86_64 image Makefile. If I look for this prefix in OpenWrt, it is found in some image Makefiles commands. For examples: - https://github.com/openwrt/openwrt/blob/master/target/linux/realtek/image/Makefile - https://github.com/openwrt/openwrt/blob/master/target/linux/bcm63xx/image/Makefile - https://github.com/openwrt/openwrt/blob/master/target/linux/ath25/image/Makefile If this is in the PATH through this line https://github.com/openwrt/openwrt/blob/master/Makefile, then this is not needed for the others? I only wanted to unify this with the change and make it clear that the tool from staging is used here. - Florian
On 20.01.23 12:42, Florian Eckert wrote: > > Hello Felix, > >>> During image generation, the host tools should not be used but the >>> tools >>> from the staging_dir. >>> >>> - mkfs.fat >>> - sed >>> - mmd >>> - mcopy >> Why is this necessary? $STAGING_DIR_HOST/bin should already be in >> $PATH before the host system parts. > > I only noticed that the prefix $(STAGING_DIR_HOST) is missing for these > tools on x86_64 image Makefile. > If I look for this prefix in OpenWrt, it is found in some image > Makefiles commands. > > For examples: > - > https://github.com/openwrt/openwrt/blob/master/target/linux/realtek/image/Makefile > - > https://github.com/openwrt/openwrt/blob/master/target/linux/bcm63xx/image/Makefile > - > https://github.com/openwrt/openwrt/blob/master/target/linux/ath25/image/Makefile > > > If this is in the PATH through this line > https://github.com/openwrt/openwrt/blob/master/Makefile, then this is > not needed for the others? > > I only wanted to unify this with the change and make it clear that the > tool from staging is used here. Thanks. I don't have a strong opinion one way or the other, but I think the code might be more readable without the explicit $(STAGING_DIR_HOST)/bin prefix. - Felix
On 2023-01-20 12:49, Felix Fietkau wrote: > On 20.01.23 12:42, Florian Eckert wrote: >> >> Hello Felix, >> >>>> During image generation, the host tools should not be used but the >>>> tools >>>> from the staging_dir. >>>> >>>> - mkfs.fat >>>> - sed >>>> - mmd >>>> - mcopy >>> Why is this necessary? $STAGING_DIR_HOST/bin should already be in >>> $PATH before the host system parts. >> >> I only noticed that the prefix $(STAGING_DIR_HOST) is missing for >> these >> tools on x86_64 image Makefile. >> If I look for this prefix in OpenWrt, it is found in some image >> Makefiles commands. >> >> For examples: >> - >> https://github.com/openwrt/openwrt/blob/master/target/linux/realtek/image/Makefile >> - >> https://github.com/openwrt/openwrt/blob/master/target/linux/bcm63xx/image/Makefile >> - >> https://github.com/openwrt/openwrt/blob/master/target/linux/ath25/image/Makefile >> >> >> If this is in the PATH through this line >> https://github.com/openwrt/openwrt/blob/master/Makefile, then this is >> not needed for the others? >> >> I only wanted to unify this with the change and make it clear that the >> tool from staging is used here. > Thanks. I don't have a strong opinion one way or the other, but I > think the code might be more readable without the explicit > $(STAGING_DIR_HOST)/bin prefix. Your right It works regardless of whether the prefix is there or not. But I would just like to note that it is easier to see whether the tools are now used from staging or the build host. The tool mkisofs https://github.com/openwrt/openwrt/blob/master/target/linux/x86/image/Makefile#L100, for example, is used from the build host! There is indeed a guard here https://github.com/openwrt/openwrt/blob/master/target/linux/x86/Makefile. But I am not sure if this is the case everywhere and if it is clear to everyone which tool is now being used during development. I just wanted to say that I am more in favor of explicitly select which tool is now being used. - Florian
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. On 2023-01-20 13:24, Florian Eckert wrote: > On 2023-01-20 12:49, Felix Fietkau wrote: >> On 20.01.23 12:42, Florian Eckert wrote: >>> >>> Hello Felix, >>> >>>>> During image generation, the host tools should not be used but the >>>>> tools >>>>> from the staging_dir. >>>>> >>>>> - mkfs.fat >>>>> - sed >>>>> - mmd >>>>> - mcopy >>>> Why is this necessary? $STAGING_DIR_HOST/bin should already be in >>>> $PATH before the host system parts. >>> >>> I only noticed that the prefix $(STAGING_DIR_HOST) is missing for >>> these >>> tools on x86_64 image Makefile. >>> If I look for this prefix in OpenWrt, it is found in some image >>> Makefiles commands. >>> >>> For examples: >>> - >>> https://github.com/openwrt/openwrt/blob/master/target/linux/realtek/image/Makefile >>> - >>> https://github.com/openwrt/openwrt/blob/master/target/linux/bcm63xx/image/Makefile >>> - >>> https://github.com/openwrt/openwrt/blob/master/target/linux/ath25/image/Makefile >>> >>> >>> If this is in the PATH through this line >>> https://github.com/openwrt/openwrt/blob/master/Makefile, then this is >>> not needed for the others? >>> >>> I only wanted to unify this with the change and make it clear that >>> the >>> tool from staging is used here. >> Thanks. I don't have a strong opinion one way or the other, but I >> think the code might be more readable without the explicit >> $(STAGING_DIR_HOST)/bin prefix. > > Your right It works regardless of whether the prefix is there or not. > But I would just like to note that it is easier to see whether the > tools are now used from staging or the build host. > The tool mkisofs > https://github.com/openwrt/openwrt/blob/master/target/linux/x86/image/Makefile#L100, > for example, is used from the build host! > There is indeed a guard here > https://github.com/openwrt/openwrt/blob/master/target/linux/x86/Makefile. > But I am not sure if this is the case everywhere and if it is clear to > everyone which tool is now being used during development. > I just wanted to say that I am more in favor of explicitly select > which tool is now being used. > I think the actual tool used should be in a variable, like $(STAGING_HOST_SED). This is very readable and it also makes the list of tools used explicitly known. The PATH must still be set for tools to find other staging dir tools. Actually, the host path should be unset to avoid inadvertently using the host tools instead of the one of the staging dir. I personally would prefer using a chroot-ed staging host to avoid any host interference. Regards, Bas.
diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile index 322131c2a4..66d914681d 100644 --- a/target/linux/x86/image/Makefile +++ b/target/linux/x86/image/Makefile @@ -59,7 +59,7 @@ endef define Build/grub-config rm -fR $@.boot $(INSTALL_DIR) $@.boot/boot/grub - sed \ + $(STAGING_DIR_HOST)/bin/sed \ -e 's#@SERIAL_CONFIG@#$(strip $(GRUB_SERIAL_CONFIG))#g' \ -e 's#@TERMINAL_CONFIG@#$(strip $(GRUB_TERMINAL_CONFIG))#g' \ -e 's#@ROOTPART@#root=$(ROOTPART) rootwait#g' \ @@ -91,9 +91,12 @@ define Build/iso > $@.boot/boot/grub/eltorito.img -$(CP) $(STAGING_DIR_ROOT)/boot/. $@.boot/boot/ $(if $(filter $(1),efi), - mkfs.fat -C $@.boot/boot/grub/isoboot.img -S 512 1440 - mmd -i $@.boot/boot/grub/isoboot.img ::/efi ::/efi/boot - mcopy -i $@.boot/boot/grub/isoboot.img \ + $(STAGING_DIR_HOST)/bin/mkfs.fat \ + -C $@.boot/boot/grub/isoboot.img -S 512 1440 + $(STAGING_DIR_HOST)/bin/mmd \ + -i $@.boot/boot/grub/isoboot.img ::/efi ::/efi/boot + $(STAGING_DIR_HOST)/bin/mcopy \ + -i $@.boot/boot/grub/isoboot.img \ $(STAGING_DIR_IMAGE)/grub2/iso-boot$(if $(CONFIG_x86_64),x64,ia32).efi \ ::/efi/boot/boot$(if $(CONFIG_x86_64),x64,ia32).efi )
During image generation, the host tools should not be used but the tools from the staging_dir. - mkfs.fat - sed - mmd - mcopy Signed-off-by: Florian Eckert <fe@dev.tdt.de> --- target/linux/x86/image/Makefile | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)