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

[Paul Spooren]

Hi Petr,

On 02.03.20 23:12, Petr Štetiar wrote:
> Paul Spooren <mail at aparcar.org> [2020-03-02 16:19:05]:
>
>> -  .IGNORE: $(BIN_DIR)/$(call IMAGE_NAME,$(1),$(2))
>>
>> [...]
>>
>>     $(BIN_DIR)/$(call IMAGE_NAME,$(1),$(2)): $(KDIR)/tmp/$(call
>> IMAGE_NAME,$(1),$(2))
>> -       cp $$^ $$@
>> +       -cp $$^ $$@
>>
>> The prefixed dash ignores a failure.
> This change seems like a band-aid as I really don't see a reason to touch the
> existing code just because you've put the JSON generation in that place
> initially.

How about I create an independent patch like the following?

[ -f $$^ ] && cp $$^ $$@ || true

It seems to be bad style ignoring cp errors, what if the destination is 
no longer writable or storage full? This way we can remove the .IGNORE.

> I would probably try to make that JSON file Make target, something like:
>
>   $(JJSON_FILENAME): whatever-prerequisites-that-json-needs
>   	json-generator.py > $$@
>
> Then add this target as prerequisite to another Make target, ideally one which
> should produce some output, not ignoring the errors. If thats not possible I
> would probably change the target/prerequisites dependency somehow etc.

I tried that, but couldn't figure out the prerequisite part. I lost hope 
for now going this path, I kindly ask you to help me if you say this is 
the only clean way.

As I see parallel file writes as the core problem I split up the 
temporary JSON files to a image instead of profile base. This means 
sysupgrade and factory images can be created in parallel because two 
independent JSON files are created. The previous approach would "extend" 
an existing file which is prone to parallel file writing trouble.

>> In scripts/json_add_image_info.py
>>
>> +image_file = bin_dir / getenv("IMAGE_NAME")
>> +if not image_file.is_file():
>> +    print("Skip JSON creation for non existing image ", image_file)
>> +    quit()
>>
>> This way the Python scripts detects an image wasn't copied and quits.
> Another band-aid, but I understand, that fixing that properly would be a huge
> task. I think, that the .IGNORE is there to workaround the abused image
> generation code for stuff like DTBs etc. and probably few more corner cases
> I'm not aware about and that commit a1b725bba534 ("build: ignore errors on
> copying firmware binaries from $(KDIR) to $(BIN_DIR)") is missing commit
> description so who knows :)
See above.
>> Is that what you're looking for?
> It doesn't hurt (and is probably necessary), but I was providing you with the
> error which was related to the previous code/JSON parsing which you're saying
> should be fixed now:
>
>   json.decoder.JSONDecodeError: Extra data: line 35 column 2 (char 823)

I think the problem is now clear and (from my point of view) solved via 
independent JSON files.

Best,
Paul


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