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

Fix perspective-python sdist and add sdist tests to CI #2817

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

tomjakubowski
Copy link
Contributor

@tomjakubowski tomjakubowski commented Oct 31, 2024

This updates the perspective-python build to create an sdist tarball by
hand, not going through maturin sdist. This bypasses some issues we
have seen with Maturin mispackaging the perspective Cargo workspace in
the sdist.

Maturin is still used as the build-backend to build a wheel from the
sdist. CI still uses maturin build to build wheels. Local
development builds still use maturin develop.

The sdist now includes the .data directory so that Maturin will place it
in the wheel correctly. Building an sdist with PSP_BUILD_SDIST=1 will
now error if it appears the jupyterlab plugin was not built into the
.data directory.

Most of the work is in generating a PKG-INFO file. Its output matches
what's in the 3.1.2 sdist on pypi, minus a correction to fix the
Home-page field miscapitalized by Maturin and some unimportant
whitespace changes in Require-Dist.

This updates the perspective-python build to create an sdist tarball by
hand, not going through `maturin sdist`. This bypasses some issues we
have seen with Maturin mispackaging the perspective Cargo workspace in
the sdist.

Maturin is still used as the `build-backend` to build a wheel from the
sdist.  CI still uses `maturin build` to build wheels.  Local
development builds still use `maturin develop`.

The sdist now includes the .data directory so that Maturin will place it
in the wheel correctly.  Building an sdist with PSP_BUILD_SDIST=1 will
now error if it appears the jupyterlab plugin was not built into the
.data directory.

Most of the work is in generating a PKG-INFO file.  Its output matches
what's in the 3.1.2 sdist on pypi, minus a correction to fix the
`Home-page` field miscapitalized by Maturin and some unimportant
whitespace changes in `Require-Dist`.

Signed-off-by: Tom Jakubowski <tom@prospective.dev>
@tomjakubowski tomjakubowski force-pushed the pack-prebuilt-sdist-minimal-ci branch from 50a472d to 1953d56 Compare October 31, 2024 02:33
@tomjakubowski
Copy link
Contributor Author

This is #2811 plus another commit which adds a CI job to test the sdist. See my commentary in the commit message and on #2811 for some caveats

@tomjakubowski tomjakubowski mentioned this pull request Oct 31, 2024
7 tasks
@tomjakubowski tomjakubowski force-pushed the pack-prebuilt-sdist-minimal-ci branch from 1953d56 to 105ec64 Compare October 31, 2024 02:38
@tomjakubowski tomjakubowski reopened this Oct 31, 2024
@tomjakubowski tomjakubowski force-pushed the pack-prebuilt-sdist-minimal-ci branch 2 times, most recently from a62bc6b to 4043309 Compare October 31, 2024 04:18
caveat: a ~/.cargo/config.toml file is created when testing the sdist
which patches the perspective-python build so that it uses
perspective-server and perspective-client from the local git checkout.
This is so we can test unreleased changes to those crates together,
without including the client/server source in the sdist.

Builders who download sdist artifacts produced in CI between releases
may find they don't build or work correctly, since those sdists will
contain dependencies on release versions of perspective-server and
perspective-client published to crates.io.

Signed-off-by: Tom Jakubowski <tom@prospective.dev>
@tomjakubowski tomjakubowski force-pushed the pack-prebuilt-sdist-minimal-ci branch from 4043309 to 08a2d2d Compare October 31, 2024 04:19
@texodus texodus changed the title Generate sdist by hand, test sdist in CI Fix perspective-python sdist and add sdist tests to CI Oct 31, 2024
@@ -765,7 +828,7 @@ jobs:
publish:
needs:
[
build_and_test_juptyerlab,
build_and_test_jupyterlab,
Copy link
Member

Choose a reason for hiding this comment

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

🤯

Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good!

@texodus texodus merged commit b5b3d7b into finos:master Oct 31, 2024
13 checks passed
@texodus texodus added the bug Concrete, reproducible bugs label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Concrete, reproducible bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants