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

[Paul Spooren]

On Wed Mar 11, 2020 at 2:12 AM PST, Petr Štetiar wrote:
> 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.

$(JSON_INFO_FILE) introduced

>
> >  		$(TOPDIR)/scripts/json_add_image_info.py \
>
> You're missing here that output file argument.

Now uses $$@

>
> > +  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.

Now again in old order and still works

>
> > 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.

ACK, c&p indeed

> I would as well move that if somewhere else.

Where should I move it? Into the $(BIN_DIR)/profiles.json target?

>
> > 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

Updated, same question regarding the IF

Thanks,
Paul

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