[OpenWrt-Devel] [PATCH 01/13] build metadata: Allow to build a subset of profiles in a single build
Felix Fietkau
nbd at openwrt.org
Tue Jan 19 05:04:19 EST 2016
On 2016-01-19 03:50, openwrt at daniel.thecshore.com wrote:
> From: Daniel Dickinson <openwrt at daniel.thecshore.com>
>
> Certain platforms have large numbers of possible images, and it can be
> desirable to build neither all images nor only a single image,
> therefore this patch makes selecting target profiles a menu instead of a
> single choice, which allows the user to build a specific subset of all
> possible images for a target.
>
> This patch contains only the build machinery changes to support
> selecting multiple images and the .config generator, but does
> not include profile changes that are needed to make selecting
> multiple profiles possible in menuconfig.
>
> This is done to isolate the changes as much as possible, although
> obviously this patch touches a greaty many files due to changing
> the image generation logic (which is necessary in order to
> eliminate the single-valued PROFILE variable, which gets in the
> way of allowing multiple images to be built at once.
>
> Signed-off-by: Daniel Dickinson <openwrt at daniel.thecshore.com>
> ---
> diff --git a/include/image.mk b/include/image.mk
> index 4eee4ad..bb46bb9 100644
> --- a/include/image.mk
> +++ b/include/image.mk
> @@ -192,7 +191,7 @@ ifneq ($(CONFIG_NAND_SUPPORT),)
> # $(1) board name
> # $(2) ubinize-image options (e.g. --uboot-env and/or --kernel kernelimage)
> # $(3) rootfstype (e.g. squashfs or ubifs)
> -# $(4) options to pass-through to ubinize (i.e. $($(PROFILE)_UBI_OPTS)))
> +# $(4) options to pass-through to ubinize (i.e. $(<profile>_UBI_OPTS)))
> define Image/Build/UbinizeImage
> sh $(TOPDIR)/scripts/ubinize-image.sh $(2) \
> "$(KDIR)/root.$(3)" \
> @@ -203,12 +202,13 @@ ifneq ($(CONFIG_NAND_SUPPORT),)
> endif
>
> ifneq ($(CONFIG_TARGET_ROOTFS_UBIFS),)
> +# $(1) profile
> define Image/mkfs/ubifs/generate
> $(CP) ./ubinize$(1).cfg $(KDIR)
> ( cd $(KDIR); \
> $(STAGING_DIR_HOST)/bin/ubinize \
> - $(if $($(PROFILE)_UBI_OPTS), \
> - $(shell echo $($(PROFILE)_UBI_OPTS)), \
> + $(if $($(1)_UBI_OPTS), \
> + $(shell echo $($(1)_UBI_OPTS)), \
> $(shell echo $(UBI_OPTS)) \
> ) \
> -o $(KDIR)/root$(1).ubi \
> @@ -216,12 +216,13 @@ ifneq ($(CONFIG_TARGET_ROOTFS_UBIFS),)
> )
> endef
>
> +# $(1) profile
> define Image/mkfs/ubifs
>
> - ifneq ($($(PROFILE)_UBIFS_OPTS)$(UBIFS_OPTS),)
> + ifneq ($($(1)_UBIFS_OPTS)$(UBIFS_OPTS),)
> $(STAGING_DIR_HOST)/bin/mkfs.ubifs \
> - $(if $($(PROFILE)_UBIFS_OPTS), \
> - $(shell echo $($(PROFILE)_UBIFS_OPTS)), \
> + $(if $($(1)_UBIFS_OPTS), \
> + $(shell echo $($(1)_UBIFS_OPTS)), \
> $(shell echo $(UBIFS_OPTS)) \
> ) \
> $(if $(CONFIG_TARGET_UBIFS_FREE_SPACE_FIXUP),--space-fixup) \
> @@ -233,13 +234,13 @@ ifneq ($(CONFIG_TARGET_ROOTFS_UBIFS),)
> -o $(KDIR)/root.ubifs \
> -d $(TARGET_DIR)
> endif
> - $(call Image/Build,ubifs)
> + $(call Image/Build,ubifs,$(1))
>
> - ifneq ($($(PROFILE)_UBI_OPTS)$(UBI_OPTS),)
> + ifneq ($($(1)_UBI_OPTS)$(UBI_OPTS),)
> $(if $(wildcard ./ubinize.cfg),$(call Image/mkfs/ubifs/generate,))
> $(if $(wildcard ./ubinize-overlay.cfg),$(call Image/mkfs/ubifs/generate,-overlay))
> endif
> - $(if $(wildcard ./ubinize.cfg),$(call Image/Build,ubi))
> + $(if $(wildcard ./ubinize.cfg),$(call Image/Build,ubi,$(1)))
> endef
> endif
>
Please don't add extra abstractions to this code. The
$(PROFILE)_UBI_OPTS stuff is legacy crap and any target still using it
should just stay that way and not get multi-profile selection until it
is converted to the new code.
> @@ -248,7 +249,8 @@ define Image/mkfs/cpiogz
> endef
>
> define Image/mkfs/targz
> - $(TAR) -czpf $(BIN_DIR)/$(IMG_PREFIX)$(if $(PROFILE_SANITIZED),-$(PROFILE_SANITIZED))-rootfs.tar.gz --numeric-owner --owner=0 --group=0 --sort=name -C $(TARGET_DIR)/ .
> +# Move to Image/Build to eliminate profile from mkfs
> + touch $(KDIR)/root.targz
> endef
>
> E2SIZE=$(shell echo $$(($(CONFIG_TARGET_ROOTFS_PARTSIZE)*1024*1024)))
> @@ -293,6 +295,11 @@ define BuildImage/mkfs
>
> endef
>
> +# $(1) profile
> +define Image/Build/targz
> + $(TAR) -czpf $(BIN_DIR)/$(IMG_PREFIX)$(if $(call sanitize,$(1)),-$(call sanitize,$(1)))-rootfs.tar.gz --numeric-owner --owner=0 --group=0 --sort=name -C $(TARGET_DIR)/ .
> +endef
> +
> # Build commands that can be called from Device/* templates
> define Build/uImage
> mkimage -A $(LINUX_KARCH) \
> @@ -547,6 +554,12 @@ define Device
>
> endef
>
> +# $(1) macro
> +# $(2) macro parameter
> +define BuildForProfiles
> + $(foreach profile,$(PROFILES_BUILD),$(call $(1),$(2),$(profile)))
> +endef
Please don't call all the legacy Image/* code once per profile.
This will generate unnecessary duplicate files and extra build time.
If you rework targets to just stop using $(PROFILE) and check the
profile enabled/disabled variables directly, you don't need this at all.
> @@ -565,29 +578,28 @@ define BuildImage
>
> image_prepare: compile
> mkdir -p $(KDIR)/tmp
> - $(call Image/Prepare)
> + $(call BuildForProfiles,Image/Prepare)
> else
> image_prepare:
> mkdir -p $(KDIR)/tmp
> endif
>
> mkfs_prepare: image_prepare
> - $(call Image/mkfs/prepare)
> + $(call BuildForProfiles,Image/mkfs/prepare)
>
> kernel_prepare: mkfs_prepare
> - $(call Image/BuildKernel)
> - $(if $(CONFIG_TARGET_ROOTFS_INITRAMFS),$(if $(IB),,$(call Image/BuildKernel/Initramfs)))
> - $(call Image/InstallKernel)
> + $(call BuildForProfiles,Image/BuildKernel)
> + $(if $(CONFIG_TARGET_ROOTFS_INITRAMFS),$(if $(IB),,$(call BuildForProfiles,Image/BuildKernel/Initramfs)))
> + $(call BuildForProfiles,Image/InstallKernel)
>
> $(foreach device,$(TARGET_DEVICES),$(call Device,$(device)))
> $(foreach fs,$(TARGET_FILESYSTEMS) $(fs-subtypes-y),$(call BuildImage/mkfs,$(fs)))
>
> install: kernel_prepare
> $(foreach fs,$(TARGET_FILESYSTEMS),
> - $(call Image/Build,$(fs))
> + $(call BuildForProfiles,Image/Build,$(fs))
> )
> - $(call Image/mkfs/ubifs)
> + $(foreach profile,$(PROFILES_BUILD),$(call Image/mkfs/ubifs,$(profile)))
> $(call Image/Checksum,md5sum --binary,md5sums)
> $(call Image/Checksum,openssl dgst -sha256,sha256sums)
> -
> endef
All of the above can be removed, as explained earlier.
> diff --git a/include/target.mk b/include/target.mk
> index f129298..92e52db 100644
> --- a/include/target.mk
> +++ b/include/target.mk
> @@ -70,6 +70,11 @@ define Profile
> DUMPINFO += \
> echo "Target-Profile: $(1)"; \
> echo "Target-Profile-Name: $(NAME)"; \
> + echo "Target-Profile-Type: $(if $(filter All,$(1)),all,$(if $(filter Minimal,$(1)),minimal,normal))"; \
> + echo "Target-Profile-Skip: $(if $(filter Default,$(1)),$(if $(PROFILE_SKIP_DEFAULT),1,0),$(if $(PROFILE_SKIP),1,0))"; \
> + echo "Target-Profile-Skip-Single: $(if $(filter All,$(1)),$(if $(PROFILE_SKIP_SINGLE),1,0),0)"; \
> + echo "Target-Profile-Default: $(if $(filter All,$(1)),1,$(if $(filter Default,$(1)),1,0))"; \
> + echo "Target-Profile-Build-If-All: $(if $(filter All,$(1)),0,$(if $(filter Minimal,$(1)),0,1))"; \
Please get rid of the magic flag setting based on the profile name. Make
these explicit by setting variables in the appropriate profiles.
Also, the types and flags are rather confusing here. From the metadata
processing point of view, there should be nothing special about profiles
named 'Default' and 'Minimal' aside from a few simple distinctions:
There should be one profile that is enable by default
There should be a distinction between device profiles and profiles that
build all (or most) devices.
If you write the code to clearly handle the above, this should make the
rest of your patch less confusing.
Now that I read your patches again, I get the sense that I disagree with
part of your changes on a conceptual level. I think giving the
impression that selecting multiple entries in the profile list will
'Build multiple profiles' seems wrong to me, making the description
'Build all profiles' even more wrong. The code is not doing that,
because that would imply having a different package selection for every
profile build, and the build system can't do that.
I think a cleaner description would be building for all devices or a
subset of the devices on the list, and just not feed the impression that
it has that much to do with 'profiles' at all.
Essentially, this is nothing but a simple way to make an image that is
compatible with a specific set of devices, and we should present it to
the user that way.
- Felix
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list