Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setuptools-scm fixes #860

Merged
merged 4 commits into from
Mar 12, 2025
Merged

setuptools-scm fixes #860

merged 4 commits into from
Mar 12, 2025

Conversation

eskultety
Copy link
Member

This is an attempt to fix one version-related aspect of our container image builds. The last 2 patches are a bit controversial, so I'd be inclined to drop them if requested, but they fix a legitimate workflow issue.
See the patch descriptions for detailed info.

FWIW I used a dummy release on my fork to experiment with this, please give it a try: https://github.com/eskultety/cachi2/releases/tag/1.2.3-alpha

Maintainers will complete the following section

  • Commit messages are descriptive enough
  • Code coverage from testing does not decrease and new code is covered
  • Docs updated (if applicable)
  • Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

'post-release' has been deprecated in v8.0.0 [1] which is already 2
years old. The new default 'guess-next-dev' [2] perfectly suffices our
needs.

[1] pypa/setuptools-scm@3bcd8c0
[2] https://setuptools-scm.readthedocs.io/en/latest/extending/#setuptools_scmversion_scheme

Signed-off-by: Erik Skultety <[email protected]>
.git_archival.txt is officially documented by setuptools-scm [1] as a
custom template file recognized by setuptools-scm that that git itself
fills out at the archive creation time. In order for git to do that, we
need to instruct it via the means of a .gitattributes file.

The order of precedence how setuptools-scm tries to infer a
project's version number [2] is:
    1) SCM itself (byt the presence of e.g. a .git directory)
    2) mercurial archive config file
    3) git archive config file, i.e. .git_archival.txt
    4) PKG-info, i.e. from an sdist

Fortunately when GitHub creates source tarballs it DOES make use of the
'git archive' functionality and so the .git_archival.txt template is
properly filled out for setuptools-scm to not fail a container build.

Resolves hermetoproject#821

[1] https://setuptools-scm.readthedocs.io/en/latest/usage/#git-archives

Signed-off-by: Erik Skultety <[email protected]>
A past commit introduced a .git_archival.txt file accompanied by a
corresponding .gitattributes file to instruct git to fill out vital
information for setuptools-scm to consume in environments where the git
repository metadata simply isn't available, e.g. builds from source
tarballs. However, there's one more legitimate (although purely
development related) usecase where setuptools-scm unfortunately
crumbles again during the build - git worktrees.

Git worktrees are shallow file system checkouts which reference the
true git objects in the main repository by essentially a symlink in the
form of a '.git' file (not a directory!). All native git actions are
worktrees aware and so everything is seamless; until one tries a
containerbuild from a worktree - due to the missing git metadata which
cannot be resolved inside a container build environment easily without
introducing nasty hacks to the Dockerfile/Containerfile, setuptools-scm
doesn't once again have means of either inferring the right version
for the project.

This patch introduces a distinct 'fallback_version' [1] string which
would only ever be applied (due to order of precedence) during
container builds when building without full git metadata or a git
archive --> git worktrees.

Note, however, that this change on its own isn't enough to make a
container build inside a git worktree pass because when setuptools-scm
uses the fallback_version configuration option, setuptools
doesn't follow with its default package detection logic and doesn't
actually put any source files/modules to the resulting sdist/wheel that
pip ends up installing. Next commit takes care of that aspect
specifically.

[1] https://setuptools-scm.readthedocs.io/en/latest/config/

Signed-off-by: Erik Skultety <[email protected]>
This is solely to workaround setuptool's weird behaviour of not
exercising its default package discovery logic when the setuptools-scm
plugin had to fall back to a static version string during its version
inference.
Due to setuptools-scm order of precedence for git metadata discovery,
this file should only ever be used on a rare occasion, e.g. doing
development builds from a git worktree.

[1] https://setuptools-scm.readthedocs.io/en/latest/usage/#file-finders-hook-makes-most-of-manifestin-unnecessary

Signed-off-by: Erik Skultety <[email protected]>
@eskultety eskultety changed the title Scm fixes setuptools-scm fixes Mar 11, 2025
Copy link
Contributor

@a-ovchinnikov a-ovchinnikov left a comment

Choose a reason for hiding this comment

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

LGTM, I don't find the second part of the PR controversial, it addresses a corner case and does not introduce much additional logic to maintain.

@eskultety
Copy link
Member Author

LGTM, I don't find the second part of the PR controversial, it addresses a corner case and does not introduce much additional logic to maintain.

The thing is, it's not a corner case, it's actually a standard workflow, e.g. I use git worktrees a lot to be able to work on several branches in parallel due to frequent interrupts. That said, adopting a MANIFEST.in file when the official setuptools docs say it's no longer needed for most cases (unless adding non-source package data) it feels like rubbing the wrong way to add it, hence I said I could be convinced to drop it. It looks like though that there might be a bug in setuptools when combined with the SCM plugin that using a manifest seems to be the only way I could find which can create a proper sdist, in turn leading to a working cachi2 install inside a development container.

Copy link
Contributor

@brunoapimentel brunoapimentel left a comment

Choose a reason for hiding this comment

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

I'm fine with the approach here.

It looks like though that there might be a bug in setuptools when combined with the SCM plugin that using a manifest seems to be the only way I could find which can create a proper sdist

Is it worth filing a bug in the setuptools-scm project?

@eskultety
Copy link
Member Author

Is it worth filing a bug in the setuptools-scm project?

Absolutely, it's on my todo list but I need to create a small reproducer first. I also think that this falls into base setuptools rather than setuptools-scm, but I need to confirm with the reproducer, so yeah, I'll do that at some point (but looking at the setuptools issues that are related to MANIFEST it doesn't seem like something that would get fixed).
Thanks for the reviews.

@eskultety eskultety added this pull request to the merge queue Mar 12, 2025
Merged via the queue into hermetoproject:main with commit d6a16c2 Mar 12, 2025
15 checks passed
@eskultety eskultety deleted the scm-fixes branch March 12, 2025 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants