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

Rewrite setup.py. #5271

Merged
merged 2 commits into from
Feb 4, 2020
Merged

Rewrite setup.py. #5271

merged 2 commits into from
Feb 4, 2020

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Feb 2, 2020

This is a rewritten setup.py for a more user (and maintainer) friendly experience of managing Python packages. Currently marked as WIP as I haven't test it on other platforms.

  • Remove setup_pip.py

  • Remove soft links.

  • Remove shell script.

  • Remove makefile script.

  • Define customized commands.

  • Update documents.

@hcho3
Copy link
Collaborator

hcho3 commented Feb 2, 2020

@trivialfis Yes, refactor of setup.py was long overdue. Can you make a list of goals you’d like to achieve with this refactor? Also brief summary of what’s going on inside setup.py would be nice.

@trivialfis
Copy link
Member Author

trivialfis commented Feb 2, 2020

@hcho3

For goals

  • Make the following commands work:
    From internet:
    - pip install xgboost
    - pip install --no-binary :all: xgboost

    From source tree `xgboost/python-package`:
    - python setup.py install
    - python setup.py build
    - python setup.py sdist       && pip install <sdist-name>
    - python setup.py bdist_wheel && pip install <wheel-name>
    - pip install . -e
    - python setup.py develop   # same as above
  • Make the script self-contain, instead of relying on some shell scripts with directory links.

  • Make source distribution portable. I expect source distribution working on all 3 supported platforms.

For what's inside setup.py

There are multiple commands with setuptools:

  • build_ext: Build C/C++ extension.
  • build: Build the Python package (with C++).
  • install: install the library, compile the source code by calling build_ext. This is the only place where we can accept user provided parameters.
  • develop: Install the links to original code, for the ease of rapid testing.
  • sdist: Create a source distribution.
  • bdist: Create a binary distribution.
  • bdist_wheel: Create a binary distribution with wheel format.

Each of these commands is represented as a Python module and a class with the same name. Hence we can subclass selected commands and customize it.

pip adds another layer on top of setuptools, with copying files to a temporary directory. Also internally it uses subprocess.call to invoke above commands. So quite difficult to debug ...

@trivialfis
Copy link
Member Author

trivialfis commented Feb 3, 2020

@hcho3 How do I log into Jenkins and find the libxgboost.so manually? I don't see any build step before building doc so may I assume the libxgboost.so is cached somewhere?

@trivialfis
Copy link
Member Author

I tried to use setuptools with sphinx build, but some packages uses exec function internally, resulting in some weird errors. So for building the document, we will have to install all dependencies ourselves.

@trivialfis
Copy link
Member Author

trivialfis commented Feb 3, 2020

How do I log into Jenkins and find the libxgboost.so manually? I don't see any build step before building doc so may I assume the libxgboost.so is cached somewhere?

Never mind. Found the root cause.

@trivialfis
Copy link
Member Author

@RAMitchell @hcho3 Ready for review. I will resolve the conflicts along with review comments.

Copy link
Member

@RAMitchell RAMitchell left a comment

Choose a reason for hiding this comment

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

LGTM, please test your doc changes locally.


- A recent C++ compiler supporting C++11 (g++-5.0 or higher)
- CMake 3.3 or higher (3.12 for building with CUDA)
- CMake 3.12 or higher.
Copy link
Member

Choose a reason for hiding this comment

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

Is cmake 3.12 strictly necessary?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since #5146, yes. It was done to support compiling XGBoost with OpenMP on Mac OSX.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hcho3 changed it to 3.12 for Apple Clang in CMakeLists.txt

* Remove setup_pip.py
* Remove soft links.
* Define customized commands.
* Remove shell script.
* Remove makefile script.
* Update the doc for building from source.
@trivialfis
Copy link
Member Author

trivialfis commented Feb 4, 2020

Since when it takes 1 and a half hour for appveyor to finish the tests .. ? ;-(

@trivialfis trivialfis merged commit 595a004 into dmlc:master Feb 4, 2020
@trivialfis trivialfis deleted the pip branch February 4, 2020 05:35
@hcho3 hcho3 mentioned this pull request Feb 21, 2020
12 tasks
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants