-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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 config-settings handling in pip install #9115
Conversation
For some reason, the doc build is broken :( |
if not build_config.use_system_libxgboost: | ||
copy_with_logging(libxgboost, lib_path, logger=logger) |
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 manually tested this change by:
- Create a new Conda environment.
- Build XGBoost from the source using CMake:
conda activate my_env
cmake .. -GNinja -DCMAKE_INSTALL_PREFIX="$CONDA_PREFIX" -DCMAKE_INSTALL_LIBDIR="lib"
- Install libxgboost into the Conda env:
ninja install
- Install the Python package with
use_system_libxgboost
flag:
pip install -v . --config-settings use_system_libxgboost=True
- Inspect
$CONDA_PREFIX
to ensure that only one copy oflibxgboost.so
is installed in the Conda environment.
$ find . -name libxgboost.so
./lib/libxgboost.so
I filed readthedocs/readthedocs.org#10285 to notify RTD about the broken doc 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.
LGTM, assuming all CI tests pass.
@trivialfis Can we ignore the RTD error for now? The RTD won't deploy the latest bug fix until next Tuesday. |
Yup, let's skip the unrelated errors. @rongou Have you seen this error before https://buildkite.com/xgboost/xgboost-ci/builds/2386 ? |
@trivialfis no, but will take a look. |
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.
Feel free to merge it.
Also test
--config-settings
in the CI so that we know it is not broken.