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

[ci] ask pip to always install local artifact but not download package from PyPI #6574

Merged
merged 11 commits into from
Jul 29, 2024

Conversation

StrikerRUS
Copy link
Collaborator

@StrikerRUS StrikerRUS commented Jul 26, 2024

Initially I was trying to use the following pip flag:

--no-index

Ignore package index (only looking at --find-links URLs instead).

https://pip.pypa.io/en/stable/cli/pip_install/#cmdoption-no-index

to prohibit pip to install LightGBM from PyPI like here:

Successfully built lightgbm-4.5.0.tar.gz
--- installing lightgbm ---
Looking in links: .
Collecting lightgbm
  Downloading lightgbm-4.5.0-py3-none-manylinux_2_28_x86_64.whl.metadata (17 kB)
Downloading lightgbm-4.5.0-py3-none-manylinux_2_28_x86_64.whl (3.6 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.6/3.6 MB 97.6 MB/s eta 0:00:00
Installing collected packages: lightgbm
Successfully installed lightgbm-4.5.0

But unfortunately that flag cannot be used without --no-build-isolation flag because pip needs setuptools during installation:

--- installing lightgbm ---
Looking in links: .
Processing ./lightgbm-4.5.0.tar.gz
  Installing build dependencies: started
  Installing build dependencies: finished with status 'error'
  error: subprocess-exited-with-error
  
  × pip subprocess to install build dependencies did not run successfully.
  │ exit code: 1
  ╰─> [3 lines of output]
      Looking in links: .
      ERROR: Could not find a version that satisfies the requirement setuptools (from versions: none)
      ERROR: No matching distribution found for setuptools
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× pip subprocess to install build dependencies did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

So I found that being more precise with *.tar.gz/*.whl extension is enough to prevent pip from downloading wheel file from PyPI.

@@ -362,9 +362,10 @@ if test "${INSTALL}" = true; then
# ref for use of '--find-links': https://stackoverflow.com/a/52481267/3986677
pip install \
${PIP_INSTALL_ARGS} \
--ignore-installed \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

--ignore-installed was added in #6526 for the following reason:

It also proposes remove the pip uninstall lightgbm in that script with passing --ignore-installed to pip install... that way the locally-built version overwrites the already-installed one every time, without generating a log message like "WARNING: Skipping lightgbm as it is not installed."

But it seems that the right flag is --force-reinstall for that.
--ignore-installed may mix old and new lightgbm installations which can result in corrupted installation. Refer to the detailed explanation here: https://stackoverflow.com/a/51916623.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right, --force-reinstall is preferable now that we are also using --no-deps here (#6256).

I'd previously not wanted to use it because it applies to all dependencies, not just to the package being installed, and that led to an existing numpy or scipy being force-reinstalled too.

@StrikerRUS StrikerRUS changed the title [ci] ignore index in Python builds [ci] ask pip to always install local artifact but not download from PyPI Jul 26, 2024
@StrikerRUS StrikerRUS changed the title [ci] ask pip to always install local artifact but not download from PyPI [ci] ask pip to always install local artifact but not download package from PyPI Jul 26, 2024
@StrikerRUS StrikerRUS marked this pull request as ready for review July 26, 2024 23:31
Copy link
Collaborator

@jameslamb jameslamb 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 this. I tested locally to be sure sh build-python.sh install --precompile works as well, and it did.

I agree, this is an improvement! Not having the ability to silently fall back to installing from PyPI is really important for ensuring that CI tests what we think it's testing.

Funny enough, I recently worked through similar issues over in RAPIDS:

Never thought about how build-python.sh here could be in need of similar changes.

Thanks very much!!

@jameslamb jameslamb merged commit e52d8fb into master Jul 29, 2024
45 checks passed
@jameslamb jameslamb deleted the ci/pip-no-index branch July 29, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants