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

[Petr Štetiar]

Paul Spooren <mail at aparcar.org> [2020-03-01 14:10:16]:

> On 01.03.20 02:34, Petr Štetiar wrote:
> > Paul Spooren <mail at aparcar.org> [2020-02-29 16:48:50]:
> > 
> > FYI:
> > 
> >   $ grep JSON .config
> >   CONFIG_JSON_OVERVIEW_IMAGE_INFO=y
> > 
> >   $ cat bin/targets/imx6/generic/profiles.json
> >   {}
> 
> This problem occurs also fox x86, the problem is that the image function is
> not properly called. Maybe because IMX6 only offer a default target but no
> profiles, resulting in an empty profiles.json file - I think. I started
> (based on Lynxis draft) reworking the x86 so it creates also JSON files[0].

This doesn't make much sense to me. It's now possible to create separate JSON
files[1], but not the amalgamated one? There is something wrong with this
reasoning.

> I'd be in favor reworking (unifying) the target specific code instead of
> extending the script logic to handle corner cases. At least if reworking is
> something what should be done anyway.

Sure, patches are always welcome and unification is great and needed, but I
think, that fixing that INPUT_DIR variable of yours would be lower hanging
fruit.

> I tried adding things like `|| exit 1` but make stubbornly keeps going. Can
> you help me out here please?

Looking at the log I see (ignored):

 /openwrt.git/scripts/json_add_image_info.py: line 1: borken: command not found
 Makefile:235: recipe for target '/openwrt.git/bin/targets/imx6/generic/openwrt-imx6-apalis-squashfs.sysupgrade.bin' failed
 make[5]: [/openwrt.git/bin/targets/imx6/generic/openwrt-imx6-apalis-squashfs.sysupgrade.bin] Error 127 (ignored)

Which leads to following in the include/image.mk:

 .IGNORE: $(BIN_DIR)/$(call IMAGE_NAME,$(1),$(2))

> > > +output_json = {}
> > > +
> > > +assert target_dir, "Target directory required"
> > > +
> > > +for json_file in input_dir.glob("*.json"):
> > > +    profile_info = json.loads(json_file.read_text())
> > > +    if not output_json:
> > > +        output_json = {
> > > +            "metadata_version": 1,
> > > +            "target": profile_info["target"],
> > > +            "version_commit": profile_info["version_commit"],
> > > +            "version_number": profile_info["version_number"],
> > > +            "profiles": {},
> > > +        }
> > I'm not a Pythonista, but perhaps you want to init the output_json dict just a
> > few lines above and get rid of that unnecesary if.
>
> The `profile_info` variable is only available after reading the first JSON
> profile and therefore in the loop.

Indeed, making me confused, hiding unhandled corner case as well, like for
example now, where you're producing `{}` instead of expected or rather correct
`{'profiles': {}, 'metadata_version': 1}`.

I would write it as following:

output_json = {
    "profiles": {},
    "metadata_version": 1,
}

def add_target_info(ojs, js):
    if 'target' in ojs:
        return

    ojs["target"] = js["target"]
    ojs["version_commit"] = js["version_commit"]
    ojs["version_number"] = js["version_number"]

def add_profile_info(ojs, js):
    profile = {
        "images": js["images"],
        "titles": js["titles"],
        "supported_devices": js["supported_devices"]
    }
    ojs["profiles"][js["id"]] = profile

for json_file in Path(os.getcwd()).glob("*.json"):
    js = json.loads(json_file.read_text())
    add_target_info(output_json, js)
    add_profile_info(output_json, js)

print(json.dumps(output_json, separators=(",", ":")))

> json.dumps(output_json, sort_keys=True, indent="  ")

BTW I've just noticed that, why do you need to produce human readable JSON?

1. https://downloads.openwrt.org/snapshots/targets/imx6/generic/openwrt-imx6-apalis.json

-- ynezz

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