Inconsistencies in include/image.mk

Philip Prindeville philipp_subx at redfish-solutions.com
Wed Jun 2 22:23:16 PDT 2021


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.

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





More information about the openwrt-devel mailing list