build: maintenance on the build and release path - #66
Merged
Conversation
Both PyPI publish jobs already request the `id-token: write` permission, so gh-action-pypi-publish can mint its upload credential from the GitHub OIDC token instead of a long-lived API token stored in PYPI_UPLOAD_TOKEN. Dropping the explicit user/password lets the action take that path. This requires a trusted publisher to be configured on PyPI for each project (openedx-plugin-sample and tutor-contrib-sample), pointing at this repository and the release.yml workflow: https://docs.pypi.org/trusted-publishers/adding-a-publisher/ Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tutor-contrib-sample/pyproject.toml declares `license-files = ["LICENSE*"]`, but that directory had no LICENSE, so the glob matched nothing and setuptools silently built a wheel with no `License-File:` in METADATA and no `dist-info/licenses/` directory. The published package claimed Apache-2.0 without carrying the text. Copied verbatim from backend-plugin-sample/LICENSE.txt so both distributions carry identical terms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tutor plugin sets `requires-python = ">=3.11"` but only advertised a 3.12 classifier, so PyPI under-reported what it runs on. It works on both 3.11 and 3.12, so list both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`[wheel] universal = 1` is a Python 2 era setting that marks a build as py2.py3 -- meaningless for a package that requires Python >=3.12. It was inert anyway: setuptools reads this option from `[bdist_wheel]`, not `[wheel]`, so nothing had been consuming it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Without a concurrency group GitHub never cancels superseded runs, so every push to a PR branch starts another five-job matrix while the previous five run to completion. Three pushes in a row means fifteen jobs, ten of them for commits nobody will read. The release calls this workflow via workflow_call, and in a called workflow `github.workflow` resolves to the caller's name -- so a naive `cancel-in-progress: true` would let a push to main cancel the tests of an in-flight release, and a shared group key would make concurrent releases queue behind one another. Keying non-pull_request events on `github.run_id` gives every release run a group of its own, leaving that path untouched. https://docs.github.com/en/actions/using-jobs/using-concurrency Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
npm rewrote this field on every publish, so the metadata on the registry
did not match the committed manifest:
npm warn publish npm auto-corrected some errors in your package.json
npm warn publish "repository" was changed from a string to an object
npm warn publish "repository.url" was normalized to
"git+https://github.com/openedx/sample-plugin.git"
Write it in the shape npm normalizes to, and add `directory` while we are
here -- the package lives in a subfolder, and that field is what makes the
repository link on npmjs.com point at frontend-plugin-sample rather than
the repo root.
https://docs.npmjs.com/cli/configuring-npm/package-json#repository
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The frontend package had no CI at all. Its first and only compile was the `npm run build` inside the release workflow's publish_to_npm job, which runs after the GitHub release is published and after both PyPI uploads. A syntax error in plugin.jsx would therefore be discovered only once two immutable PyPI versions and an immutable GitHub release existed for a version that has no npm package. Compiling is the only check available today -- the package defines no lint or test script -- but it is the one that would have caught that. Uses `npm ci` rather than the release job's `npm install`, so the build is pinned to the committed lockfile and fails loudly if the lockfile and package.json disagree. Aligning the release job is left for a follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Like the frontend package, the tutor plugin was never built until the release workflow needed to publish it, so a broken pyproject.toml surfaced only after the GitHub release and the backend PyPI upload had gone out. Also runs `twine check`, which validates the metadata PyPI will accept and render. The long description is read dynamically from README.md, so it is worth checking that it still renders. The checkout uses fetch-depth 0 because setuptools-scm derives the version from git tags and this project declares no fallback_version, unlike the backend. The release job sidesteps that with SETUPTOOLS_SCM_PRETEND_VERSION, so this exercises a path the release never does: building from a plain checkout, the way a contributor would locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The release only required the backend test matrix, so it would tag, publish a GitHub release, and upload to PyPI while the frontend and tutor packages were still unbuilt. Neither the GitHub release nor a PyPI upload can be withdrawn, so a package that fails to build in its own publish job leaves the release permanently half-finished. Requiring all three CI workflows means nothing gets tagged until every package this workflow publishes is known to build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`npm install` reconciles the lockfile with package.json and carries on; `npm ci` installs the lockfile exactly and fails if the two have drifted apart. For the one build that becomes a published artifact, failing loudly is what we want -- especially here, where every dependency in package.json is `"*"`, so the lockfile is the only thing pinning anything at all. `--include=dev` goes away because `npm ci` installs dev dependencies by default. This matches what frontend-ci.yml already does, so a green PR build and the release build now install the same tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
We only want `npm version` to rewrite package.json here; the tag for this
release is created by python-semantic-release.
This is a no-op today. npm gates all git work behind a single check that
stats `.git` in the package directory and does not walk up to the repo root:
const isGitDir = newversion === 'from-git' || await git.is(opts)
const doGit = gitTagVersion && isGitDir && await enforceClean(opts)
frontend-plugin-sample is a subdirectory with no `.git` of its own, so
`doGit` is already false and no commit or tag is made. Verified by
reproduction: run in a subdirectory package, npm writes the version only;
run with package.json at the git root, it also commits and tags.
The flag makes the intent explicit rather than leaving it resting on
repository layout, which would silently start committing and tagging if this
package ever moved to the repo root.
https://github.com/npm/cli/blob/latest/workspaces/libnpmversion/lib/version.js
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
=======================================
Coverage 93.04% 93.04%
=======================================
Files 16 16
Lines 503 503
Branches 18 18
=======================================
Hits 468 468
Misses 24 24
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Build and release maintenance, found by auditing the publish path end to end. Each commit stands alone.
Publishing
id-token: write, so dropping theuser/passwordpair letsgh-action-pypi-publishexchange the GitHub OIDC token instead. This also un-breaks attestations, which the action was silently ignoring:The workflow was run with the 'attestations: true' input, but an explicit password was also set, disabling Trusted Publishing.npm ciwhen publishing — every dependency infrontend-plugin-sample/package.jsonis"*", so the lockfile is the only thing pinning anything.npm installreconciles drift and carries on;npm ciinstalls the lock exactly and fails if it disagrees with the manifest. Worth having on the one build that becomes a published artifact.npm version --no-git-tag-version— a no-op today, added as intent documentation. npm gates git work behind astatfor.gitin the package directory and does not walk up to the repo root, so a subdirectory package never commits or tags. The flag stops that resting on repository layout.repositorymetadata — npm rewrote this field on every publish, so the registry metadata did not match the committed manifest. Addingdirectoryalso points the npmjs.com repository link at the subfolder.The release could publish untested code
release.ymlrequired only the backend test matrix. The frontend package had no CI at all: its first and only compile was insidepublish_to_npm, which runs after the GitHub release is published and after both PyPI uploads. A syntax error inplugin.jsxwould surface once two immutable PyPI versions and an immutable GitHub release already existed for a version with no npm package. The tutor plugin was likewise first built inside the release job.This adds
frontend-ci.ymlandtutor-ci.yml(bothpull_request+workflow_call) and makesreleaseneed all three, so nothing is tagged until every published package is known to build.Two notes on those workflows:
fetch-depth: 0. setuptools-scm reads the version from git tags andtutor-contrib-sampledeclares nofallback_version, unlike the backend, so a shallow clone breaks it. This also exercises a path the release never takes, since the release passesSETUPTOOLS_SCM_PRETEND_VERSION.Correctness
license-files = ["LICENSE*"]was declared but that directory had no LICENSE, so the glob matched nothing and setuptools built a wheel with noLicense-File:in METADATA and nodist-info/licenses/. The package claimed Apache-2.0 without carrying the text. Verified fixed by rebuilding.requires-python = ">=3.11"but only advertised 3.12.Housekeeping
github.workflowresolves to the caller's name, and withcancel-in-progress: falseGitHub keeps only one pending run per group, so non-pull_requestevents key ongithub.run_idto give each release run a group of its own.[wheel] universal = 1— a Python 2 era flag on a package requiring >=3.12, and inert anyway since setuptools reads that option from[bdist_wheel].Verification
actionlintpasses on all workflows. Both new builds were run locally first:npm ci && npm run buildcompiles, and the tutor plugin builds sdist + wheel withtwine checkPASSED on both.Follow-ups not in this PR
Upgrade Python Requirementsjob has failed every run (4/4 in August) — the sharedopenedx/.githubworkflow runsmake upgradefrom the repo root and there is no root Makefile. Deferred pending a larger change.github-actions, sofrontend-plugin-sample/package-lock.jsongets no updates.codecov.ymlsits inbackend-plugin-sample/, but Codecov reads it from the repo root,.github/, ordev/— the 100% patch target may not be applied. Unverified.