Fix PyPI upload by pinning version from tag#7974
Conversation
Set SETUPTOOLS_SCM_PRETEND_VERSION on both sdist and wheel build steps so the version comes directly from the git tag, bypassing dirty-tree detection from earlier build steps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
/code-review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: Fix PyPI upload by pinning version from tag
The approach here is correct — setting SETUPTOOLS_SCM_PRETEND_VERSION on both build steps forces the version from the tag and avoids +dirty suffixes that would break PyPI uploads. Good fix.
One concern worth addressing
The git reset --hard at line 46 (originally needed to prevent dirty-tree version strings) is now redundant because the env var handles that. But more importantly, keeping it means uv build --wheel runs on the clean git tree — without the frontend assets exported in "Build and export frontend files". The sdist captures those files before the reset, but the wheel does not.
If the admin-ui export is supposed to be bundled in the wheel (which it appears to be, given the explicit build step), this should be fixed either by removing the now-unnecessary git reset --hard, or by combining both builds into a single uv build call before it. See the inline comment for details.
🔬 Codegraph: unavailable
💡 Write /code-review in a comment to re-run this review.
| - name: Build the sdist | ||
| run: | | ||
| uv build --sdist | ||
| git reset --hard |
There was a problem hiding this comment.
.github/workflows/publish_package.yaml:46
Now that SETUPTOOLS_SCM_PRETEND_VERSION bypasses dirty-tree version detection, git reset --hard appears to be redundant here. More importantly, keeping it means the wheel (built in the next step) is assembled from the clean git state — without the admin-ui assets exported at line 34 (npm run prod-export-admin-ui). The sdist captures those files before the reset, but the wheel does not.
If git reset --hard is still genuinely needed for some other reason, please document why. Otherwise, consider removing it so both artifacts include the built frontend files, or build both in a single uv build invocation before the reset:
- name: Build sdist and wheel
run: uv build
env:
SETUPTOOLS_SCM_PRETEND_VERSION: ${{ github.ref_name }}There was a problem hiding this comment.
That comment is incorrect. git reset --hard only resets tracked files. The admin-ui build output in src/fides/ui-build/ is gitignored (.gitignore
has ui-build/), so git reset --hard doesn't touch it. The frontend assets persist through the reset and are included in the wheel via the
artifacts config in pyproject.toml:
[tool.hatch.build.targets.wheel]
artifacts = ["src/fides/ui-build"]
Both the sdist and the wheel get the frontend files. The git reset --hard only cleans up whatever tracked file modifications the build process
causes.
2 participants
Ticket N/A
Description Of Changes
Set
SETUPTOOLS_SCM_PRETEND_VERSIONon bothsdistandwheelbuild steps so the version comes directly from the git tag, bypassing dirty-tree detection from earlier build steps.Code Changes
Steps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works