Add build_id placeholder to contents.xml

[vkraleti]

Axiom requires a unique identifier (build_id) to track published artifacts. Since values like github.run_id or build path are not
accessible at the ptool repository level, add a placeholder field in contents.xml.

The actual build_id can be injected during the final build stage in meta-qcom by passing "BUILD_ID=" to make.

Initial change to fix qualcomm-linux/meta-qcom#1081

[lool]

Changes look good, thanks! Two requests:

1. could you mention the filename fix in the PR description

2. could you add a --build-id or similar flag to gen_contents.py to set the output value? This would avoid having to sed this in qcom-deb-images and meta-qcom, and that would also give us the opportunity to replace @BUILD_ID@ with an empty string when unset, which would give a somewhat file to use with Axiom if people run this by hand

Thanks!

[lool]

Changes look good, thanks! Two requests:

1. could you mention the filename fix in the PR description
2. could you add a --build-id or similar flag to gen_contents.py to set the output value? This would avoid having to sed this in qcom-deb-images and meta-qcom, and that would also give us the opportunity to replace @BUILD_ID@ with an empty string when unset, which would give a somewhat file to use with Axiom if people run this by hand

Thanks!

Actually just realized that you might also want a BUILD_ID option in the Makefile so that "make BUILD_ID=foo" generates the files with the right id (we still want the gen_contents --build-id flag though)

[vkraleti]

Changes look good, thanks! Two requests:

1. could you mention the filename fix in the PR description
2. could you add a --build-id or similar flag to gen_contents.py to set the output value? This would avoid having to sed this in qcom-deb-images and meta-qcom, and that would also give us the opportunity to replace @BUILD_ID@ with an empty string when unset, which would give a somewhat file to use with Axiom if people run this by hand

Thanks!

Actually just realized that you might also want a BUILD_ID option in the Makefile so that "make BUILD_ID=foo" generates the files with the right id (we still want the gen_contents --build-id flag though)

Sure, I'll enhance both gen_contents.py script & makefile to accept BUILD_ID.

I moved the filenames fix to #39 So that it can merge independently.

[lumag]

Changes look good, thanks! Two requests:

1. could you mention the filename fix in the PR description
2. could you add a --build-id or similar flag to gen_contents.py to set the output value? This would avoid having to sed this in qcom-deb-images and meta-qcom, and that would also give us the opportunity to replace @BUILD_ID@ with an empty string when unset, which would give a somewhat file to use with Axiom if people run this by hand

Thanks!

Actually just realized that you might also want a BUILD_ID option in the Makefile so that "make BUILD_ID=foo" generates the files with the right id (we still want the gen_contents --build-id flag though)

Thinking about it... Maybe it's not a correct idea. The build-id comes from the firmware / rootfs / final build stage. Inside qcom-ptool we don't know this ID (and I don't think we should be reruning ptool generation each time the ID changes).
So, I'd prefer if we don't have a way to specify the ID here. Instead it should be added to an empty tag at the place when it is actually determined.

[lool]

2. could you add a --build-id or similar flag to gen_contents.py to set the output value? This would avoid having to sed this in qcom-deb-images and meta-qcom, and that would also give us the opportunity to replace @BUILD_ID@ with an empty string when unset, which would give a somewhat file to use with Axiom if people run this by hand

Actually just realized that you might also want a BUILD_ID option in the Makefile so that "make BUILD_ID=foo" generates the files with the right id (we still want the gen_contents --build-id flag though)

Thinking about it... Maybe it's not a correct idea. The build-id comes from the firmware / rootfs / final build stage. Inside qcom-ptool we don't know this ID (and I don't think we should be reruning ptool generation each time the ID changes). So, I'd prefer if we don't have a way to specify the ID here. Instead it should be added to an empty tag at the place when it is actually determined.

(I don't understand!) ptool is where we the script that generates contents.xml lies, and build-id is in the contents.xml. The flag can be empty or not specified, and the default behavior can be the one you describe, but if we don't have the flag, that means we have to post-process the xml in meta-qcom and qcom-deb-images instead of simply passing the build-id from these while we generate the contents.xml. It's not hard, but it feels unneeded when we are doing xml parsing to adjust a few things in this script already?!

[lumag]

2. could you add a --build-id or similar flag to gen_contents.py to set the output value? This would avoid having to sed this in qcom-deb-images and meta-qcom, and that would also give us the opportunity to replace @BUILD_ID@ with an empty string when unset, which would give a somewhat file to use with Axiom if people run this by hand

Actually just realized that you might also want a BUILD_ID option in the Makefile so that "make BUILD_ID=foo" generates the files with the right id (we still want the gen_contents --build-id flag though)

Thinking about it... Maybe it's not a correct idea. The build-id comes from the firmware / rootfs / final build stage. Inside qcom-ptool we don't know this ID (and I don't think we should be reruning ptool generation each time the ID changes). So, I'd prefer if we don't have a way to specify the ID here. Instead it should be added to an empty tag at the place when it is actually determined.

(I don't understand!) ptool is where we the script that generates contents.xml lies, and build-id is in the contents.xml. The flag can be empty or not specified, and the default behavior can be the one you describe, but if we don't have the flag, that means we have to post-process the xml in meta-qcom and qcom-deb-images instead of simply passing the build-id from these while we generate the contents.xml. It's not hard, but it feels unneeded when we are doing xml parsing to adjust a few things in this script already?!

Well... we might be generating contents.xml too early. I'm thinking from the OE point of view. There partitions and contents.xml files are generated via the recipe. The generated files should not contain a fixed build ID. Later, when the code combines rootfs, boot image, firmware into the archive, then it's the time to set the build ID.

[lool]

@lumag Agreed, perhaps contents.xml should indeed be generated quite late as for the build-id to be meaningful and related to the integrated use case. In the Debian case, we generate rootfs, then filesystem / disk images, then flat images and it's at this third stage that we call ptool, just before sending these flat images for testing to LAVA and in the future Axiom.

I guess Yocto is building software, then building images, with ptool generated being just an input to the image; perhaps the ptool/contents.xml generation should be closer to image generation in Yocto/OE world too?

[lumag]

@lumag Agreed, perhaps contents.xml should indeed be generated quite late as for the build-id to be meaningful and related to the integrated use case. In the Debian case, we generate rootfs, then filesystem / disk images, then flat images and it's at this third stage that we call ptool, just before sending these flat images for testing to LAVA and in the future Axiom.

I guess Yocto is building software, then building images, with ptool generated being just an input to the image; perhaps the ptool/contents.xml generation should be closer to image generation in Yocto/OE world too?

No, it's a low-level artifact, so it should also be generated pretty early. I don't see a need to regenerate partition, GPT or contents files if the only change is the build ID. We are doing some preprocessing on the files anyway, when generating the final archive, so why not fix the build ID at that point too?

[lool]

@lumag Agreed, perhaps contents.xml should indeed be generated quite late as for the build-id to be meaningful and related to the integrated use case. In the Debian case, we generate rootfs, then filesystem / disk images, then flat images and it's at this third stage that we call ptool, just before sending these flat images for testing to LAVA and in the future Axiom.
I guess Yocto is building software, then building images, with ptool generated being just an input to the image; perhaps the ptool/contents.xml generation should be closer to image generation in Yocto/OE world too?

No, it's a low-level artifact, so it should also be generated pretty early. I don't see a need to regenerate partition, GPT or contents files if the only change is the build ID. We are doing some preprocessing on the files anyway, when generating the final archive, so why not fix the build ID at that point too?

Different user experiences / distro experiences might customize e.g. the list of partitions; I think that would call for running ptool later.

Again, I could also post-process the files, I just don't see what we would lose by having this ability to set the build-id if we know it when calling gen_contents.py; it's de factor where we are worrying about what a well formed file looks like.

[ricardosalveti]

Latest changes from @vkraleti works with both cases.

[vkraleti]

@lumag @lool with this PR, make can read an optional BUILD_ID to add to contents.xml and if not passed @BUILD_ID@ placeholder gets added to contents.xml. I hope this addresses both cases you are referring to.

[lool]

LGTM, suggested a small improvement to Makefile

[lool]

I checked out the proposed branch, and built with make and make BUILD_ID=foobar and I could see a build_id element being created with foobar in the second case, and with @BUILD_ID@ in the first case

Small things:

1. Makefile uses BUILD_ID but gen_contents.py uses BUILDID, they should either be the same or more different :)

2. I don't think "@buildid@" is nice in the resulting contents.xml when it's unset; I believe if that's meant to make the job of post-processing easier, these tools can call make BUILD_ID=@BUILDID@ and achieve the same results. I believe we should default to either not setting the build_id element at all when -b isn't set, or default to an empty build_id

3. how could we make the Makefile BUILD_ID setting a bit more visible to end-users? Should there be something at the top of Makefile like:

# optional build_id for Axiom contents.xml files
BUILD_ID ?=

[vkraleti]

@lool acted on your comments, can you check now?

[lool]

Looks great @vkraleti, thanks for the polish!

I've rested with make (empty <build_id/>) and make BUILD_ID=foobar, and both worked as we discussed.