镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Stamp ESP-IDF component versions from git and add registry metadata - #24710

Open
rascani wants to merge 1 commit into
esp-idf-consumer-cifrom
esp-idf-release-metadata
Open

rascani wants to merge 1 commit into
esp-idf-consumer-cifrom
esp-idf-release-metadata

Conversation

@rascani

@rascani rascani commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

The component's version was version.txt plus a hand-bumped packaging
revision (1.6.03), which nothing tied to what was being packaged. The
generator now derives it from git and refuses stamps the checkout cannot
back: a plain X.Y.Z needs HEAD at the vX.Y.Z tag, X.Y.Z-rc.N needs the
vX.Y.Z-rcN tag, and an X.Y.Z
N respin needs HEAD after vX.Y.Z on
release/X.Y with no changes since the tag outside examples/espressif/,
.ci/, .github/ and docs/. Those three also refuse a checkout with changes
to what the component ships: to tracked files, or untracked files where
the generator globs sources, the flatbuffers submodule's headers
included. Builds from any other commit pass --allow-dev-stamp and get
X.Y.Z-dev., which is never published. An ExecuTorch
tree that is not the top of its own git checkout gets no stamp, so a
copy inside another repository is never stamped from that repository's
history. examples/espressif/test_version_stamp.sh checks these guards in
a scratch repository, and CI runs it first.

idf_component.yml gains the registry's repository, documentation (at the
packaged commit), issues, discussion and tags fields, and its license now
lists every license in the vendored code: ExecuTorch's BSD-3-Clause,
FlatBuffers' Apache-2.0, and the LLVM (Apache-2.0 WITH LLVM-exception,
NCSA) and musl (MIT) code in a few runtime and c10 headers. The component
ships LLVM's LICENSE.TXT, whose legacy NCSA text array_ref.h refers to,
next to FlatBuffers' license. sbom.yml with one sub-manifest each for
FlatBuffers and c10 describes the vendored code for esp-idf-sbom, and CI
validates them along with compote manifest lint.

The component ships examples/hello_executorch, a project that runs the
smoke model with the model and kernel bundle made at generation time by
the component's own gen_kernels.py. The generator therefore needs
ExecuTorch installed from the exact commit it packages, and checks that
before it starts. The example depends on exactly this component version,
since its bundle is made for it, through override_path. CI builds it
that way and runs it on the ESP32-S3 emulator, and the consumer test
also builds it as a registry user gets it, copied out of the packed
archive without override_path so the pin resolves from the mirror. The
component's README, which becomes its registry page, tells projects to
depend on one exact version and commit dependencies.lock, since a bundle
only fits the version it was made for.

Authored with Claude Code.

@pytorch-bot

pytorch-bot Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24710

Note: Links to docs will display an error until the docs builds have been completed.

❌ 4 New Failures, 1 Unrelated Failure

As of commit bf96e77 with merge base 60a8b5f (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@rascani
rascani added this pull request to stack #24713 November 5, 2026 23:48
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@rascani
rascani force-pushed the esp-idf-release-metadata branch from 2381b16 to 7672819 Compare November 6, 2026 19:19
@rascani
rascani force-pushed the esp-idf-release-metadata branch from 7672819 to cac1874 Compare November 6, 2026 20:40
@rascani
rascani requested a review from JakeStevens November 6, 2026 20:48
@rascani
rascani marked this pull request as ready for review November 6, 2026 20:48
@rascani
rascani requested a review from larryliu0820 as a code owner November 6, 2026 20:48
@rascani
rascani force-pushed the esp-idf-release-metadata branch from cac1874 to ed688e0 Compare November 6, 2026 21:15
@rascani

rascani commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@claude review effort=max

@claude

claude Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Claude finished @rascani's task in 46m 26s —— View job


Request Changes: the git-checked version stamps, registry/SBOM metadata and hello_executorch example are close, but the ~N respin rules contradict each other and gen_kernels.py, and the NCSA-licensed array_ref.h would be published without its license text.

Detailed review of #24710 (effort=max)

Summary

This PR replaces the hard-coded X.Y.Z~3 component version with stamps checked against git: a release at vX.Y.Z, -rc.N at vX.Y.Z-rcN, ~N respins on release/X.Y, and -dev.<commit date>. It also adds registry metadata and esp-idf-sbom manifests, ships examples/hello_executorch with its model and kernel bundle, and tests the stamp guards in a scratch repo.

Nothing blocks on its own. The main problems:

  • The ~N respin rules are inconsistent. A respin is refused for changes that never reach the component, and the generator rejects the release wheel that gen_kernels.py accepts.
  • The component declares NCSA-licensed code but doesn't ship the NCSA text.

All 11 changed files were reviewed. I couldn't run test_version_stamp.sh or the generator in this run, so claims about git behavior come from reading the code and git's documented semantics.

Code Quality

  • The respin guard refuses changes that never reach the component (build_idf_component.sh:160). git diff --name-only "v$ET_VERSION" HEAD -- . ':(exclude)examples/espressif' refuses a ~N for any change outside examples/espressif/. That includes .github/, .ci/, docs/ and backends the component doesn't ship. Two cases:

    • Once any CI or docs fix lands on release/X.Y after the tag, no commit after it can be stamped ~N.
    • A packaging fix that also updates its own test in .ci/scripts/test_espressif_idf_component.sh is refused.

    At least exclude .github/, .ci/ and docs/. An allow-list of what the generator reads would also need the Python that exports the example and makes its bundle (exir/, codegen/). Fix this →

  • The installed-executorch check rejects the release wheel for ~N (build_idf_component.sh:205-213). Every stamp requires executorch.version.git_version == HEAD. versions_match (idf_component_template/tools/gen_kernels.py:73-78) only requires an exact SHA for -dev. and -rc., and accepts any X.Y.Z install for X.Y.Z and X.Y.Z~N. A real respin is a later commit than the release, so it can only be generated with executorch built from source at the respin commit. It can never use the X.Y.Z wheel its users export with. That wheel would work for everything the generator does with it (export the smoke model, run gen_kernels.py), because the respin guard keeps everything outside examples/espressif/ identical to the tag. With two definitions of "matching executorch", one will drift; use versions_match's rule here too. Fix this →

  • NCSA-licensed code ships without its license text (build_idf_component.sh:461-472, :483-490).

    • The NOTICE table (:472), sbom.yml (:486) and idf_component.yml.in:15 declare NCSA for array_ref.h, but only flatbuffers-LICENSE.txt ships (:461).
    • runtime/core/array_ref.h:13-14 says "distributed under the University of Illinois Open Source License. See LICENSE.TXT for details". There is no LICENSE.TXT in the component, and the root LICENSE that :457 ships has no NCSA text.
    • NCSA requires source redistributions to keep the copyright notice, conditions and disclaimer, and sbom.yml's copyright: has no University of Illinois entry.

    The gap already exists in the repo, but this PR is what publishes the component with NCSA declared. Ship LLVM's legacy NCSA LICENSE.TXT (with its UIUC copyright line) under THIRD_PARTY_LICENSES/, reference it in the LLVM row, and add the copyright to sbom.yml. The musl MIT text is already inline in BFloat16-math.h. function_ref.h and llvmMathExtras.h only cite https://llvm.org/LICENSE.txt, so shipping that file too is cheap. Worth checking with whoever signs off on OSS licensing. Fix this →

  • The untracked-file guard misses the flatbuffers submodule and repeats the copy roots by hand (build_idf_component.sh:180-182). third-party/flatbuffers/include/flatbuffers is copied whole (cp -r, :404), but neither check sees untracked files in it:

    • it isn't in the list;
    • ls-files --others doesn't descend into submodules;
    • status --untracked-files=no ignores untracked content in submodules.

    The list also repeats the find/copy_headers/cp -r roots of steps 2–4 by hand, so the next root added there will be missed. Define the roots once and use them for both copying and this guard. Fix this →

  • VERSION marks release stamps that passed the guard as dirty (build_idf_component.sh:612, :617). ET_DIRTY runs git status --porcelain, which includes untracked files. A release, RC or respin stamp that passed the guard at :175-187 is still recorded as executorch_commit: <sha> (tree had uncommitted changes) in two cases:

    • any untracked file exists anywhere in the checkout (the notes.txt case at test_version_stamp.sh:89-91, which is checked only with --version);
    • --out is inside the checkout and not gitignored.

    The published metadata then says the release was built from a dirty tree. Use the guard's rule. Fix this →

  • The ~N branch and tag checks are weaker than the header says (build_idf_component.sh:145-159). Fix this →

    • Any local branch named release/X.Y that contains HEAD satisfies "HEAD must be on release/X.Y". That includes an unpushed commit on the real branch, or a feature branch created with git checkout -b release/1.6. The only remote checked is a hard-coded origin, which in a fork workflow is the fork. So a respin can be generated from a commit that isn't on the public release branch. Check only the remote-tracking branch, or drop the check and document what a respin guarantees.
    • HEAD at vX.Y.Z itself passes, because merge-base --is-ancestor is true for equal commits and the diff is empty. The release can then be republished unchanged as X.Y.Z~1, and every user who moves to it has to regenerate their bundle for nothing. Require HEAD to differ from vX.Y.Z^{commit}.
  • --version-suffix -dev.YYYYMMDD is a second, unchecked way to make a dev stamp (build_idf_component.sh:166-170). Only the format is checked, so any date stamps any commit: -dev.20991231 sorts after every real dev build. --allow-dev-stamp already covers dev builds. Drop the explicit form (and test_version_stamp.sh:66), or check the date against HEAD's commit date. Fix this →

  • The stamp can come from an enclosing repository (build_idf_component.sh:106). git -C "$ET_ROOT" uses the nearest enclosing repo. An ExecuTorch tree that isn't a checkout itself but sits inside one (vendored into a monorepo, or a tarball extracted under a project repo) gets stamped with the outer repo's HEAD, tags, commit date and status. It should fail with "generate from a git checkout" instead. setup.py's git rev-parse HEAD finds the same outer repo, so the SHA check at :210 passes too. Require git rev-parse --show-toplevel to equal $ET_ROOT (compare pwd -P paths). The no-git test (test_version_stamp.sh:112-125) has the same problem in reverse: it fails when TMPDIR is inside a repo, so set GIT_CEILING_DIRECTORIES. Fix this →

  • The example build re-implements consumer() (.ci/scripts/test_espressif_idf_component.sh:290-293). Lines :291-292 repeat the body of consumer() (:107-113). project="${example}" consumer example esp32s3 build is the same call and matches :257. Fix this →

  • documentation: points at main (idf_component_template/idf_component.yml.in:12). A published component pinned to an exact X.Y.Z links to tree/main/examples/espressif, which describes whatever main does when someone reads it (stamp flags, layout). Point it at the release's docs, substituting the tag the way the version is substituted. Consider also stamping repository_info.commit_sha with $ET_SHA so the registry links to the source commit. Fix this →

  • The SBOM sub-manifests are missing fields (build_idf_component.sh:481-521). Fix this →

    • sbom_c10.yml has no version, cpe or purl. The SBOM can't say which PyTorch revision include/c10 and include/torch came from, and esp-idf-sbom's vulnerability check has nothing to match against.
    • sbom_flatbuffers.yml has no copyright:.
    • FLATBUFFERS_VERSION (:481-482) isn't checked. If the defines in base.h change format, the manifest gets an empty or partial version. Fail unless it is X.Y.Z.

Testing

  • test_version_stamp.sh leaves several guards untested. Fix this →

    • No test depends on the ancestry half of the branch check (build_idf_component.sh:147). The only "off a release branch" case (:74) has no release/1.6 ref at all, and every other ~1 case has HEAD on it. Deleting && git merge-base --is-ancestor HEAD "$ref" keeps the suite green. Add a ~1 from a commit that isn't on an existing release/1.6.
    • No case reaches the missing-tag error (build_idf_component.sh:154-156) or the catch-all (:171-172). The ~0 case (:73) only matches --version-suffix, which the catch-all message also contains. Match ~N with N >= 1 instead, and add an unknown suffix such as -beta.1.
    • The scratch repo inherits the developer's git config. commit.gpgSign, tag.gpgSign or core.hooksPath can make the setup's git commit/git tag prompt or fail locally while CI stays green. Export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 next to the identity variables (:20-22).
  • The shipped example is only built inside the generated tree (.ci/scripts/test_espressif_idf_component.sh:289-293). CI builds examples/hello_executorch in place, through override_path: "../../../". Registry users get it through idf.py create-project-from-example, which copies the example out of the component, so the exact == pin has to resolve from a registry. No test covers that path. test_espressif_idf_consumer.sh already packs the component and serves it from a local mirror (:71-76); creating and building the example from that mirror would cover it. Fix this →

Recommendation

Request Changes. Nothing blocks on its own, but three fixes should land before the first registry publish:

  1. Limit the respin diff to paths that can reach the component.
  2. Use gen_kernels.py's compatibility rule in the generator's executorch check.
  3. Ship the NCSA text.

The rest are minor.

Specific Comments

  • build_idf_component.sh:20 — the header shows --version-suffix ~N unquoted. After a pushd, bash expands ~1 to a directory-stack entry, and zsh fails with "not enough directory stack entries". Show '~1'; the generator: line (:192) already quotes it.
  • build_idf_component.sh:20-22 — "packaging-only respin" undersells it. examples/espressif/ holds shipped runtime code (platform/esp_pal.cpp, idf_component_template/src/et_model.cpp, the public include/et_model.h), so a respin can change the PAL and the C API. The same wording is in idf_component.yml.in:4-5 and the template README (:32-33).
  • build_idf_component.sh:24 — --allow-dev-stamp reads like a permission switch, but it selects the stamp and conflicts with --version-suffix. --dev-stamp says what it does.
  • build_idf_component.sh:35-37 — reading build_variables.bzl needs only a plain Python (bzl_list execs the file), and --version needs no Python at all. Only step 6 needs the executorch package from this commit.
  • build_idf_component.sh:177 — unlike the untracked-file (:184-185) and respin (:162-163) errors, this one doesn't say what is dirty. Print the git status output.
  • build_idf_component.sh:479-480 — "name, version and url come from idf_component.yml" is only true for sbom.yml; the two sub-manifests set their own.
  • build_idf_component.sh:637-638 — "made with this component's tools" is true of the kernel bundle, but the model is exported with the checkout's export_smoke_model.py and split_bundled_program.py.
  • build_idf_component.sh:451-452 — the README is copied verbatim, so its pin snippet (README :29) says "==X.Y.Z" even though the generator knows the version. Substitute @ET_COMPONENT_VERSION@ as the manifests do.
  • idf_component.yml.in:4-5 — the comment describes only release and ~N versions, but generated manifests can also be -rc.N or -dev.YYYYMMDD.
  • idf_component_template/README.md:16 — the regenerate command has no stamp option, so it now fails anywhere except at a release tag. Point to the generator: line in VERSION, which records the exact command.
  • idf_component_template/README.md:32-34 — with dependencies.lock committed (:25), a newer version only breaks a range-pinned build once the lock is re-solved (e.g. idf.py update-dependencies). It doesn't break the moment the version is published.
  • idf_component_template/README.md:117-122 — "Updating" describes re-running the generator, which fits a vendored copy. Registry users update by bumping the == pin and regenerating their bundle.
  • examples/hello_executorch — this user-facing example reuses the smoke test's main.c, so it prints ET_COMPONENT_SMOKE: PASS and walks smoke_models. It also has no README.
  • .ci/scripts/test_espressif_idf_component.sh:70 — test_version_stamp.sh needs only git and bash, but runs after the toolchain setup (:42) and the model exports (:47-64). Run it first so a stamp regression fails in seconds.
  • test_version_stamp.sh:61 — the --clean case uses expect_version for output that isn't a version. expect_output would fit both uses.

  • Load /pr-review skill
  • Gather PR diff and context (11 files, +473/−49, one commit ed688e0 on top of esp-idf-consumer-ci)
  • Independent review panel (3 reviewers)
  • Consolidate findings
  • Fact-check findings against the code
  • Post final review

The component's version was version.txt plus a hand-bumped packaging
revision (1.6.0~3), which nothing tied to what was being packaged. The
generator now derives it from git and refuses stamps the checkout cannot
back: a plain X.Y.Z needs HEAD at the vX.Y.Z tag, X.Y.Z-rc.N needs the
vX.Y.Z-rcN tag, and an X.Y.Z~N respin needs HEAD after vX.Y.Z on
release/X.Y with no changes since the tag outside examples/espressif/,
.ci/, .github/ and docs/. Those three also refuse a checkout with changes
to what the component ships: to tracked files, or untracked files where
the generator globs sources, the flatbuffers submodule's headers
included. Builds from any other commit pass --allow-dev-stamp and get
X.Y.Z-dev.<UTC commit date>, which is never published. An ExecuTorch
tree that is not the top of its own git checkout gets no stamp, so a
copy inside another repository is never stamped from that repository's
history. examples/espressif/test_version_stamp.sh checks these guards in
a scratch repository, and CI runs it first.

idf_component.yml gains the registry's repository, documentation (at the
packaged commit), issues, discussion and tags fields, and its license now
lists every license in the vendored code: ExecuTorch's BSD-3-Clause,
FlatBuffers' Apache-2.0, and the LLVM (Apache-2.0 WITH LLVM-exception,
NCSA) and musl (MIT) code in a few runtime and c10 headers. The component
ships LLVM's LICENSE.TXT, whose legacy NCSA text array_ref.h refers to,
next to FlatBuffers' license. sbom.yml with one sub-manifest each for
FlatBuffers and c10 describes the vendored code for esp-idf-sbom, and CI
validates them along with compote manifest lint.

The component ships examples/hello_executorch, a project that runs the
smoke model with the model and kernel bundle made at generation time by
the component's own gen_kernels.py. The generator therefore needs
ExecuTorch installed from the exact commit it packages, and checks that
before it starts. The example depends on exactly this component version,
since its bundle is made for it, through override_path. CI builds it
that way and runs it on the ESP32-S3 emulator, and the consumer test
also builds it as a registry user gets it, copied out of the packed
archive without override_path so the pin resolves from the mirror. The
component's README, which becomes its registry page, tells projects to
depend on one exact version and commit dependencies.lock, since a bundle
only fits the version it was made for.

Authored with Claude Code.

This branch was previously deployed

1 inactive deployment
cadence — bf96e77e Deployed Nov 7, 2026 by rascani via hifi-op-test / hifi4 #37196
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant