Inconsistencies in include/image.mk

[Adrian Schmutzler]

Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Philip Prindeville
> Sent: Donnerstag, 3. Juni 2021 07:23
> To: OpenWrt Development List <openwrt-devel at lists.openwrt.org>
> Subject: Inconsistencies in include/image.mk
> 
> Hi,
> 
> I was looking at this with Paul this afternoon, and noticed a few issues worth
> fixing in include/image.mk after reviewing the DEVICE_PACKAGES PR he sent
> out.

I've not checked it in detail, but it looks to me like you are mixing TARGET and DEVICE variables here.

IIRC IMG_PREFIX is a target-based, device-independent variable, so it should not be set based on a device-dependent PROFILE_something variable.

Best

Adrian

> 
> The first part is how IMG_PREFIX is derived:
> 
> IMG_PREFIX_EXTRA:=$(if $(EXTRA_IMAGE_NAME),$(call
> sanitize,$(EXTRA_IMAGE_NAME))-) IMG_PREFIX_VERNUM:=$(if
> $(CONFIG_VERSION_FILENAMES),$(call sanitize,$(VERSION_NUMBER))-)
> IMG_PREFIX_VERCODE:=$(if $(CONFIG_VERSION_CODE_FILENAMES),$(call
> sanitize,$(VERSION_CODE))-)
> 
> IMG_PREFIX:=$(VERSION_DIST_SANITIZED)-
> $(IMG_PREFIX_VERNUM)$(IMG_PREFIX_VERCODE)$(IMG_PREFIX_EXTRA)$
> (BOARD)$(if $(SUBTARGET),-$(SUBTARGET)) IMG_ROOTFS:=$(IMG_PREFIX)-
> rootfs IMG_COMBINED:=$(IMG_PREFIX)-combined
> 
> But we don't append -$(PROFILE_SANITIZED) to it (if set), adding it later
> here:
> 
> define Image/Manifest
> 	$(call opkg,$(TARGET_DIR_ORIG)) list-installed > \
> 		$(BIN_DIR)/$(IMG_PREFIX)$(if $(PROFILE_SANITIZED),-
> $(PROFILE_SANITIZED)).manifest
> endef
> 
> And here:
> 
> ifdef CONFIG_TARGET_ROOTFS_TARGZ
>   define Image/Build/targz
> 	$(TAR) -cp --numeric-owner --owner=0 --group=0 --mode=a-s --
> sort=name \
> 		$(if $(SOURCE_DATE_EPOCH),--
> mtime="@$(SOURCE_DATE_EPOCH)") \
> 		-C $(TARGET_DIR)/ . | gzip -9n >
> $(BIN_DIR)/$(IMG_PREFIX)$(if $(PROFILE_SANITIZED),-
> $(PROFILE_SANITIZED))-rootfs.tar.gz
>   endef
> endif
> 
> Note that we're also adding the suffix -rootfs, but not using the
> $(IMG_ROOTFS) definition made up at the top.
> 
> Conversely, here:
> 
> ifdef CONFIG_TARGET_ROOTFS_CPIOGZ
>   define Image/Build/cpiogz
> 	( cd $(TARGET_DIR); find . | $(STAGING_DIR_HOST)/bin/cpio -o -H
> newc -R 0:0 | gzip -9n >$(BIN_DIR)/$(IMG_ROOTFS).cpio.gz )
>   endef
> endif
> 
> We use $(IMG_ROOTFS) which doesn't include $(PROFILE_SANITIZED)... but
> we do use $(PROFILE_SANITIZED) in Image/Build/targz.  I don't see why the
> two cases aren't handled in the same way.
> 
> Either $(PROFILE_SANITIZED) is important enough to be included in
> everything (including cpio.gz images and manifests) or it's not.
> 
> Creating a PR to address this:
> 
> https://github.com/openwrt/openwrt/pull/4223
> 
> I don't know how to get good coverage of this change with CI/CD since that
> seems to only happen with PR's for "packages", but not "openwrt".
> 
> Any suggestions?
> 
> Thanks,
> 
> -Philip
> 
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20210603/4d73c1a0/attachment.sig>