Skip to content

feat: add pip builds #27

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

Merged
merged 2 commits into from
Jul 17, 2025
Merged

feat: add pip builds #27

merged 2 commits into from
Jul 17, 2025

Conversation

zmitchell
Copy link
Member

In the impure step of the pure build, this uses the pip download command to download binary wheels and source distributions into the $out/libexec/deps directory. Then in the pure build we simply install everything in that directory.

@zmitchell zmitchell force-pushed the push-qywrunpzuwpn branch 2 times, most recently from cb79c0d to 38551c7 Compare July 15, 2025 02:36
pip install ${quotes-app-pip-deps}/libexec/deps/*

# Install this project, skipping the deps we've just installed
pip install --no-build-isolation --no-deps .
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for reviewers: This --no-build-isolation flag is required to prevent pip from downloading a new copy of setuptools for the build. It does that by default even if there's a copy of setuptools in the environment.

@zmitchell zmitchell marked this pull request as ready for review July 15, 2025 05:31
@zmitchell zmitchell requested review from dcarley, ysndr and mkenigs July 15, 2025 22:25
source $out/libexec/venv/bin/activate

# Download wheels/source-dists of all of the dependencies
mkdir -p $out/libexec/deps
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
mkdir -p $out/libexec/deps
mkdir -p $out/deps

Is this only used by the pip install ${quotes-app-pip-deps}/libexec/deps/* command? libexec feels wrong to me. As this is only ever used by build.quotes-app-pip-pure, do we even need to follow FHS?

Copy link
Member Author

Choose a reason for hiding this comment

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

We recommend to users to use FHS, so I'm just following that convention.

Comment on lines 112 to 114
# Prevent install being a noop when run from an existing activation that
# already has the virtualenv and dependencies installed.
unset VIRTUAL_ENV
Copy link
Contributor

Choose a reason for hiding this comment

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

question nonblocking: how does VIRTUAL_ENV make it into the sandbox, and if it's not, can we drop this line for a pure build?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this, I thought I was following the convention in the quotes-app-python build but missed that it didn't unset the variable for the pure build.

@@ -0,0 +1,134 @@
version = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

question nonblocking: do we want to rename quotes-app-python -> quotes-app-poetry?

Copy link
Member Author

Choose a reason for hiding this comment

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

Eventually, but I'd also like it to be *-python-poetry to be more descriptive.

In the impure step of the pure build, this uses the `pip download`
command to download binary wheels and source distributions into the
`$out/libexec/deps` directory. Then in the pure build we simply install
everything in that directory.
@zmitchell zmitchell force-pushed the push-qywrunpzuwpn branch from 38551c7 to 556df58 Compare July 17, 2025 15:20
@zmitchell zmitchell enabled auto-merge July 17, 2025 15:39
@zmitchell zmitchell disabled auto-merge July 17, 2025 15:39
@zmitchell zmitchell merged commit 319252c into main Jul 17, 2025
3 checks passed
@zmitchell zmitchell deleted the push-qywrunpzuwpn branch July 17, 2025 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants