Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/compile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ jobs:
version: 14
local_conf_header:
build-id: |
BUILD_ID = "${BUILD_ID}"
QCOM_BUILD_ID = "${BUILD_ID}"
EOF
env:
INPUTS_CACHE_DIR: ${{inputs.cache_dir}}
Expand Down
4 changes: 3 additions & 1 deletion ci/base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ local_conf_header:
# Use a weak assignment so the CI override wins regardless of how kas
# orders the merged local.conf fragments.
OS_RELEASE_FIELDS:append = " BUILD_ID"
BUILD_ID ?= "local-${DATETIME}"
QCOM_BUILD_ID ?= "local-${DATETIME}"
BUILD_ID ?= "${QCOM_BUILD_ID}"
BUILD_ID[vardepsexclude] ?= "QCOM_BUILD_ID"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get the change, this will result in builds reusing the build_id from the cache, which will produce a mismatch in os-release / uki / etc, and it goes against the main purpose of the change that added this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And some additional findings from AI:

  • DATETIME exclusion is lost. BUILD_ID[vardepsexclude] ?= "QCOM_BUILD_ID" in local.conf is parsed before the recipe, so oe-core's ?= "DATETIME" becomes a no-op. Anyone setting BUILD_ID = "${DATETIME}-tag" (the upstream-documented pattern) now rebuilds os-release and every image on each invocation. Fix is "QCOM_BUILD_ID DATETIME", or drop the flag entirely.
  • Both comments are now false. ci/base.yml lines 66-70 still say CI overrides BUILD_ID via the fragment, and compile.yml line 86 still promises os-release maps back to the S3 folder. Neither is true after this change.
  • Weak binding regression. BUILD_ID ?= "${QCOM_BUILD_ID}" means any downstream hard BUILD_ID = unconditionally disconnects CI's run id. No in-tree fragment does this today.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea behind this change is to enable the reuse of images from the sstate-cache. Something I've been cooking up for a while now in #2699

It may happen that the image in question does not correspond to the ${{ github.run_id }}-${{ github.run_attempt }} of the executed version in CI. But if this is the case, it's because the image met the conditions to be reused.

So for the execution AAAA-2 in CI we can have AAAA-1 inside the rootfs of the image. This means that the image was reused from ${{ github.run_attempt }}=1.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the idea, but while this will save build time and increase our reproducibility in our CI, it might also confuse users flashing our images for development and debugging purposes, as the runid won't necessarily match.

Unfortunately I don't really have a better solution as anything that touches the image will cause it to be created from scratch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to move this change to #2699 and leave it there while I implement the functionality.


machine: unset

Expand Down
Loading