[OpenWrt-Devel] [PATCH v5] build: refactor JSON info files to `profiles.json`

[Petr Štetiar]

Paul Spooren <mail at aparcar.org> [2020-03-10 18:11:21]:

> +  $$(_TARGET): $(BUILD_DIR)/json_info_files/$(call IMAGE_NAME,$(1),$(2)).json
> +  $(BUILD_DIR)/json_info_files/$(call IMAGE_NAME,$(1),$(2)).json: $(BIN_DIR)/$(call IMAGE_NAME,$(1),$(2))$$(GZ_SUFFIX)

This JSON file target is relatively long, used twice already, perhaps using
common short variable would make sense here.

>  		$(TOPDIR)/scripts/json_add_image_info.py \

You're missing here that output file argument.

> +  ROOTFS/$(1)/$(3) := \
> +	$(KDIR)/root.$(1)$$(strip \
> +		$$(if $$(FS_OPTIONS/$(1)),+fs=$$(call param_mangle,$$(FS_OPTIONS/$(1)))) \
> +	)$$(strip \
> +		$(if $(TARGET_PER_DEVICE_ROOTFS),+pkg=$$(ROOTFS_ID/$(3))) \
> +	)
> +  ifndef IB
> +    $$(ROOTFS/$(1)/$(3)): $(if $(TARGET_PER_DEVICE_ROOTFS),target-dir-$$(ROOTFS_ID/$(3)))
> +  endif
> +  $(KDIR)/tmp/$(call IMAGE_NAME,$(1),$(2)): $$(KDIR_KERNEL_IMAGE) $$(ROOTFS/$(1)/$(3))
> +	@rm -f $$@
> +	[ -f $$(word 1,$$^) -a -f $$(word 2,$$^) ]
> +	$$(call concat_cmd,$(if $(IMAGE/$(2)/$(1)),$(IMAGE/$(2)/$(1)),$(IMAGE/$(2))))
> +
> +  .IGNORE: $(BIN_DIR)/$(call IMAGE_NAME,$(1),$(2))
> +
> +  $(BIN_DIR)/$(call IMAGE_NAME,$(1),$(2)).gz: $(KDIR)/tmp/$(call IMAGE_NAME,$(1),$(2))
> +	gzip -c -9n $$^ > $$@
> +
> +  $(BIN_DIR)/$(call IMAGE_NAME,$(1),$(2)): $(KDIR)/tmp/$(call IMAGE_NAME,$(1),$(2))
> +	cp $$^ $$@
> +

Is this reordering necessary? If so, I would probably do that in separate patch for
easier review.

> diff --git a/target/imagebuilder/files/Makefile b/target/imagebuilder/files/Makefile
> index 15b3d5c35c..7d5eddaff6 100644
> --- a/target/imagebuilder/files/Makefile
> +++ b/target/imagebuilder/files/Makefile
> @@ -118,6 +118,7 @@ _call_image: staging_dir/host/.prereq-build
>  	$(MAKE) package_install
>  	$(MAKE) -s prepare_rootfs
>  	$(MAKE) -s build_image
> +	$(if $(CONFIG_JSON_OVERVIEW_IMAGE_INFO),$(_SINGLE)$(SUBMAKE) -r json_overview_image_info)
>  	$(MAKE) -s checksum

Seems like copy&paste from the `world` target. I think, that:

  $(MAKE) -s json_overview_image_info

would make more sense here. I would as well move that if somewhere else.

> world: prepare $(target/stamp-compile) $(package/stamp-compile) $(package/stamp-install) $(target/stamp-install) FORCE
>        $(_SINGLE)$(SUBMAKE) -r package/index
> +       $(if $(CONFIG_JSON_OVERVIEW_IMAGE_INFO),$(_SINGLE)$(SUBMAKE) -r json_overview_image_info)
>         $(_SINGLE)$(SUBMAKE) -r checksum

Same here, move that if and the common place to not repeat that condition two times etc.

  $(_SINGLE)$(SUBMAKE) -r json_overview_image_info

-- ynezz

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