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.

Sign up for GitHub

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

Avoid --find-links in wheel jobs #397

Merged
merged 3 commits into from
Jul 3, 2024
Merged

Avoid --find-links in wheel jobs #397

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a5d3c7
Avoid --find-links in wheel jobs
jameslamb Jun 27, 2024
633f3fb
fix typo
jameslamb Jun 27, 2024
912c195
Merge branch 'branch-24.08' into remove-find-links
jameslamb Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion ci/build_wheel_python.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@ CPP_WHEELHOUSE=$(RAPIDS_PY_WHEEL_NAME="libkvikio_${RAPIDS_PY_CUDA_SUFFIX}" rapid

cd "${package_dir}"

python -m pip wheel . -w dist -vvv --no-deps --disable-pip-version-check --find-links ${CPP_WHEELHOUSE}
# ensure 'kvikio' wheel builds always use the 'libkvikio' just built in the same CI run
#
# using env variable PIP_CONSTRAINT is necessary to ensure the constraints
# are used when creating the isolated build environment
echo "libkvikio-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo ${CPP_WHEELHOUSE}/libkvikio_*.whl)" > ./constraints.txt

PIP_CONSTRAINT="${PWD}/constraints.txt" \
python -m pip wheel . -w dist -vvv --no-deps --disable-pip-version-check
Comment on lines +20 to +27
Copy link
Member

Choose a reason for hiding this comment

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

Am curious whether just adding --no-index would be sufficient for us

Suggested change
# ensure 'kvikio' wheel builds always use the 'libkvikio' just built in the same CI run
#
# using env variable PIP_CONSTRAINT is necessary to ensure the constraints
# are used when creating the isolated build environment
echo "libkvikio-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo ${CPP_WHEELHOUSE}/libkvikio_*.whl)" > ./constraints.txt
PIP_CONSTRAINT="${PWD}/constraints.txt" \
python -m pip wheel . -w dist -vvv --no-deps --disable-pip-version-check
python -m pip wheel . -w dist -vvv --no-deps --disable-pip-version-check --no-index --find-links ${CPP_WHEELHOUSE}

Copy link
Member Author

@jameslamb jameslamb Jun 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the suggestion! I hadn't considered this.

But I wouldn't expect that to be sufficient here. We're building with build isolation, and this ${CPP_WHEELHOUSE} directory doesn't contain wheels for other build dependencies like scikit-build-core and rapids-build-backend.

So to use --no-index --find-links ${CPP_WHEELHOUSE} we'd have to also do something like pip download the other build dependencies into that local directory. I think the approach with constraints is simpler for as long as we want to use build isolation (which, per rapidsai/build-planning#69 (comment), might not be for long).

Copy link
Member

Choose a reason for hiding this comment

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

Discussed offline with James. We decided James will write up an issue for follow up and we will skip this suggestion in this PR

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for talking through it with me. Here's the issue: rapidsai/build-planning#79


mkdir -p final_dist
python -m auditwheel repair -w final_dist dist/*
Expand Down