Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org]
> On Behalf Of Paul Spooren
> Sent: Sonntag, 14. Juni 2020 11:34
> To: openwrt-devel@lists.openwrt.org
> Subject: [OpenWrt-Devel] [PATCH][RFC] build: disable target name in image
> filename
> 
> The target/subtarget information in image filenames is barely of any use for
> developers or end users.
> 
> A developer reads the profile name and the target is either obvious due to
> previous work or `cd targets/ && grep -r <profile>` tells the target within
> 3ms. If no buildroot is available `cat <image> | tail -c 200` allows a look 
> at the
> attached metadata which includes the target/subtarget.
> 
> For users the information is entirely useless and maybe even harmful.
> Target names like `cortexa9` could easily be mistaken as an actual device
> name while the only relevant information would be `linksys_wrt3200acm`.
> Images are more realistically downloaded via a Wiki entry or a firmware
> wizard.
> 
> This commit therefore adds the new image option called
> CONFIG_TARGET_FILENAMES to make the target/subtarget filename part
> optional. It is disabled by default.
> 
> As the profile name `generic` appears multiple times in the x86 target as well
> as in oceton and ath25, the proposed patch on GitHub should be merged
> first:
> * treewide: use unique profile names #3082
> https://github.com/openwrt/openwrt/pull/3082
> 
> Newly produced files would look like the following:
> * openwrt-linksys_wrt3200acm-initramfs-kernel.bin
> * openwrt-linksys_wrt3200acm.manifest
> * openwrt-linksys_wrt3200acm-squashfs-factory.img
> * openwrt-linksys_wrt3200acm-squashfs-sysupgrade.bin

I just think of ar71xx and ath79, where we have the same device but different 
targets. Of course, the name won't be exactly equal, as ath79 will have e.g. 
tplink_ prefix and ar71xx won't.
For bcm63xx, we have two subtargets that build the same devices.
If we look at PR#2957, we might have a now bmips target at some point that 
features the same devices as bcm63xx.

This won't necessarily break anything, as images will still be in different 
folders (at least in /bin).

However, we couldn't tell the difference between ar71xx/ath79 or similar from 
the image name (easily) after this change, or whether it's generic or smp for 
bcm63xx. For my personal taste, this drawback is bigger that the gain we will 
get from removing the target/subtarget part.

So, unless there is overwhelming support, I tend to NAK this.

Best

Adrian

> 
> Signed-off-by: Paul Spooren <m...@aparcar.org>
> ---
> It's been a while since I made a controversial patch[0] so it feels about 
> time.
> 
> [0]: https://github.com/openwrt/openwrt/pull/2107
> 
>  include/image.mk                   | 9 +++++----
>  package/base-files/image-config.in | 9 +++++++++
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/include/image.mk b/include/image.mk index
> 984b64fb9c..c6fc467c9e 100644
> --- a/include/image.mk
> +++ b/include/image.mk
> @@ -37,11 +37,12 @@ KDIR=$(KERNEL_BUILD_DIR)
> KDIR_TMP=$(KDIR)/tmp
> DTS_DIR:=$(LINUX_DIR)/arch/$(LINUX_KARCH)/boot/dts
> 
> +IMG_PREFIX_TARGET:=$(if $(CONFIG_TARGET_FILENAMES),$(BOARD)$(if
> +$(SUBTARGET),-$(SUBTARGET))-)
>  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_PREFIX:=$(VERSION_DIST_SANITIZED)-
> $(IMG_PREFIX_VERNUM)$(IMG_PREFIX_
> +VERCODE)$(IMG_PREFIX_EXTRA)$(IMG_PREFIX_TARGET)
>  IMG_ROOTFS:=$(IMG_PREFIX)-rootfs
>  IMG_COMBINED:=$(IMG_PREFIX)-combined
>  IMG_PART_SIGNATURE:=$(shell echo
> $(SOURCE_DATE_EPOCH)$(LINUX_VERMAGIC) | mkhash md5 | cut -b1-8)
> @@ -293,7 +294,7 @@ endef
> 
>  define Image/Manifest
>       $(call opkg,$(TARGET_DIR_ORIG)) list-installed > \
> -             $(BIN_DIR)/$(IMG_PREFIX)$(if $(PROFILE_SANITIZED),-
> $(PROFILE_SANITIZED)).manifest
> +             $(BIN_DIR)/$(IMG_PREFIX)$(if
> +$(PROFILE_SANITIZED),$(PROFILE_SANITIZED)).manifest
>  endef
> 
>  define Image/gzip-ext4-padded-squashfs
> @@ -317,7 +318,7 @@ 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
> +             -C $(TARGET_DIR)/ . | gzip -9n >
> $(BIN_DIR)/$(IMG_PREFIX)$(if
> +$(PROFILE_SANITIZED),$(PROFILE_SANITIZED))-rootfs.tar.gz
>    endef
>  endif
> 
> @@ -385,7 +386,7 @@ define Device/Init
> 
>    IMAGES :=
>    ARTIFACTS :=
> -  IMAGE_PREFIX := $(IMG_PREFIX)-$(1)
> +  IMAGE_PREFIX := $(IMG_PREFIX)$(1)
>    IMAGE_NAME = $$(IMAGE_PREFIX)-$$(1)-$$(2)
>    IMAGE_SIZE :=
>    KERNEL_PREFIX = $$(IMAGE_PREFIX)
> diff --git a/package/base-files/image-config.in b/package/base-files/image-
> config.in
> index 3432db525a..5a70d51a7a 100644
> --- a/package/base-files/image-config.in
> +++ b/package/base-files/image-config.in
> @@ -264,6 +264,15 @@ if VERSIONOPT
>                       Enable this to include the revision identifier or the
> configured
>                       version code into the firmware image, SDK- and
> Image Builder archive
>                       file names
> +
> +     config TARGET_FILENAMES
> +             bool
> +             prompt "Target and subtarget in filenames"
> +             default n
> +             help
> +                     Enable this to include the target (and subtarget) in
> +                     firmware image, SDK- and Image Builder archive file
> names
> +
>  endif
> 
> 
> --
> 2.20.1
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Attachment: openpgp-digital-signature.asc
Description: PGP signature

_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to