Skip to content

Conversation

@tiran
Copy link
Collaborator

@tiran tiran commented Mar 25, 2025

Fromager now defaults to PEP 571 hooks to create sdists. This re-generates the metadata of sdists, too. Fresh metadata solves problems like core metadata violation with Hatchling < 1.26.

  • Packages with build_dir option use old tarball method by default.
  • Packages with mesonpy build backend use old tarball method. meson dist currently only works with Git or Mercurial repos.
  • Packages with build_sdist hook are not affected.
  • build_options.sdist_backend: tarball restores the previous behavior.

@mergify mergify bot added the ci label Mar 25, 2025
@tiran tiran force-pushed the sdist_backend branch 2 times, most recently from 0cedb86 to 09c01fb Compare March 25, 2025 08:53
Fromager now defaults to PEP 571 hooks to create sdists. This
re-generates the metadata of sdists, too. Fresh metadata solves problems
like core metadata violation with Hatchling < 1.26.

- Packages with `build_dir` option use old `tarball` method by default.
- Packages with `mesonpy` build backend use old `tarball` method. `meson
  dist` currently only works with Git or Mercurial repos.
- Packages with `build_sdist` hook are not affected.
- `build_options.sdist_backend: tarball` restores the previous behavior.

Signed-off-by: Christian Heimes <[email protected]>
@tiran tiran marked this pull request as ready for review March 25, 2025 10:26
@rgommers
Copy link

This re-generates the metadata of sdists, too

It sounds like you're doing sdist -> sdist? That's not a thing, no packaging standard guarantees that that will work, and I know of multiple reasons it cannot work reliably.

Copy link

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Yes this looks like it assumes sdist -> sdist should work, and that's false. There are a lot of projects that for example rely on generating a file from VCS with versioning info. That should only ever be run once, roundtripping when starting from an sdist may lose the info or change a git commit hash to "unknown".

I think this is probably going to introduce more problems than it solves. Maybe doing it only when there's a known issue with existing metadata in the sdist is better?


class SDistBackend(enum.StrEnum):
# build sdist with PEP 517 hook
PEP517 = "pep571"

Choose a reason for hiding this comment

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

🙃 typo, but PEP 571 is the manylinux2010 PEP so could be a deep feature too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ouch :)



# meson dist currently only works with Git or Mercurial repos
# affects NumPy and projects with https://mesonbuild.com/meson-python/

Choose a reason for hiding this comment

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

If Meson doesn't work with an alternative VCS, it won't be used. I don't think Fromager should worry about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need to clarify this comment or remove BUILD_BACKEND_FORCE_TARBALL again. The meson dist command expects a VCS checkout. It does not support sdist -> sdist.

@rgommers
Copy link

Other reason: a build_sdist call may do dependency detection of non-Python dependencies (shared libraries etc.), which may not be present outside of a release workflow.

@tiran tiran marked this pull request as draft March 25, 2025 11:56
@tiran
Copy link
Collaborator Author

tiran commented Mar 25, 2025

I think this is probably going to introduce more problems than it solves. Maybe doing it only when there's a known issue with existing metadata in the sdist is better?

Oh, that's too bad. Back to the drawing board!

I was able to build all the wheels in our set with the new code. I have not verified that the wheels were built correctly, though.

@rgommers
Copy link

I was able to build all the wheels in our set with the new code. I have not verified that the wheels were built correctly, though.

I'd expect most problems to be subtle, like metadata changing or a generated file going missing.

@tiran
Copy link
Collaborator Author

tiran commented Oct 28, 2025

Closing this PR because building sdist from sdist is not a supported case.

@tiran tiran closed this Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants