-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
CI Use lock files for Windows builds #23379
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, the script works well, although it seems that some dependencies have been released this since PR was opened:
diff --git a/build_tools/azure/py38_pip_openblas_32bit_lock.txt b/build_tools/azure/py38_pip_openblas_32bit_lock.txt
index 5dd3a5c04..b30f7c2c4 100644
--- a/build_tools/azure/py38_pip_openblas_32bit_lock.txt
+++ b/build_tools/azure/py38_pip_openblas_32bit_lock.txt
@@ -6,7 +6,7 @@
#
attrs==21.4.0
# via pytest
-cython==0.29.28
+cython==0.29.30
# via -r build_tools/azure/py38_pip_openblas_32bit_requirements.txt
execnet==1.9.0
# via pytest-xdist
@@ -20,7 +20,7 @@ numpy==1.22.3
# scipy
packaging==21.3
# via pytest
-pillow==9.1.0
+pillow==9.1.1
# via -r build_tools/azure/py38_pip_openblas_32bit_requirements.txt
pluggy==1.0.0
# via pytest
@@ -39,7 +39,7 @@ pytest-forked==1.4.0
# via pytest-xdist
pytest-xdist==2.5.0
# via -r build_tools/azure/py38_pip_openblas_32bit_requirements.txt
-scipy==1.8.0
+scipy==1.8.1
# via -r build_tools/azure/py38_pip_openblas_32bit_requirements.txt
threadpoolctl==3.1.0
# via -r build_tools/azure/py38_pip_openblas_32bit_requirements.txt
I reran the script and pushed a commit to update the Windows build lock file. |
And everything went fine :) |
@lesteve Could you solve the conflict since I merged the circle ci PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
8b34fcb
to
b45b6d1
Compare
|
||
@click.command() | ||
@click.option( | ||
"--select-build", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this quite convenient when only generating a subset of all the possible lock files, regenerating all the lock files can take a while (~4 minutes on my machine)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does add click
as direct dependency. click
is already installed because is a dependency for pip-tools
. Let's add it into the list of dependencies?
scikit-learn/build_tools/update_environments_and_lock_files.py
Lines 25 to 29 in ef2ef31
To run this script you need: | |
- conda-lock. The version should match the one used in the CI in | |
build_tools/azure/install.sh | |
- pip-tools | |
- jinja2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
click
is a indirect dependency right? conda-lock
(and pip-tools
actually as you said) depends on click
, so I thought it was not necessary to mention it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jinja2
was included in the list as a indirect dependency from conda-lock
. (I'm trying to be consistent with what goes in the list)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed jinja2
from the list since it is a dependency of conda-lock
|
Merging this one! |
Reference Issues/PRs
part of #22425
What does this implement/fix? Explain your changes.
This uses lock files for Windows builds in the CI
Any other comments?
For the Windows 32bit build, the assumption is that you can pin versions on a Linux 64bit machine. There is no cross-compile support for pip-compile (see https://github.com/jazzband/pip-tools#cross-environment-usage-of-requirementsinrequirementstxt-and-pip-compile for more details)