Skip to content
Snippets Groups Projects

[Setup] Add support for development install

Merged Christophe Guillon requested to merge cguillon/aidge_core:dev/cguillon/setup-editable into dev

Context

Add capacity in setup.py to do pip editable installs (or development mode), i.e.:

  • modification to python files do not need a reinstallation,
  • C++ build can be incrementally rebuild and reinstalled from the build/ directory

In order to use editable mode even with recent versions of python one should do pip install -v --no-build-isolation -e . instead of the default install with pip install -v .

This MR allows editable mode to speedup day to day development basically with the changes:

  • moved the installation of the python binding library into the cmake build system, instead of the setup script
  • detect editable mode and in this case install the python binding library directly in the source python package

Note that due to behavior changes depending on the python/setuptools versions, the portable method to ensure incremental build on C++ files is to use the first time (pre-install build dependencies, and use --no-build-isolation):

pip install setuptools setuptools_scm[toml] cmake
pip install --no-build-isolation -v -e .

Then, when C++ files changes:

make -C build install -j $(nproc)

Modified files

Refer to the list of commits in the MR for details.

I separated them in 6 listed below, the first 5 are preparatory commits or complements to the recent merge of pip_release branch:

  • [Setup] Remove useless copy of version.txt in python package: not necessary anymore as the python version is managed by setuptools_scm
  • [CMake] Minor update for comment location: no functionnal change
  • [Setup] Allow empty AIDGE_BUILD_GEN type: allow for specifying AIDGE_BUILD_GEN= (empty) to fall back to default
  • [Setup] Remove additional build dependency on toml: prefer to limit the number of dependencies at most for setup.py
  • [CMake] Add installation of python binding: responsibility of python bindings lib installation is now in the build system
  • [Setup] Add support for editable mode: redirect installation locally in editable mode and document in README.md

Tests

Tested with:

  • normal mode on python 3.9
  • normal model on python 3.12
  • development mode on python 3.9
  • development model on python 3.12
  • normal mode on windows 3.12
  • development mode on windows 3.12
  • expected to fail: on windows: setting the Ninja backend fails, no compiler found

TODO / To be discussed

  • ninja dependency: Yes, available as a python package ninja. Let optional for the developper.
  • symlinks. Removed necessity to have symlinks has the build system installs the library itself
  • version.txt. Since changes in packaing, no need to copy version.txt anymore
Edited by Christophe Guillon

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added 1 commit

    • 3cca921f - [Setup] Add support for development install

    Compare with previous version

  • Christophe Guillon changed the description

    changed the description

  • @gregkub Can you take a look at this MR :smile:

  • This is very useful! I already use the developper mode locally, but didn't took the time to properly handle the .so file which was not at the right place... (had to add symbolic link locally)

    • Resolved by Christophe Guillon

      Yes, actually setting a symbolic link would be better than my current choice (copy) when in development mode. This would enable to alternatively just redo cmake -B build --install instead of pip install -e . (after the first pip install -e .) Though I didn't do it to support also development mode on Windows platforms. We could choose to have development mode only working on Linux platforms, or perhaps it there a way to define also a symbolic link under Windows? The same apply for the version.txt file. Actually it would be easier if the version.txt file was part of the package tree (i.e. directly in the package dir instead of the root dir). Again I did not move it, but it is an option.

      Any thought on these two points?

  • Well actually on Windows I do not know how it works currently with the standard pip install as I suppose that there is no shared objects (.so) file, but probably DLLs (.dll) file. Isn't it?

  • :warning:

    Dont merge to main directly, the main branch is only for official relase ! I'll change the branch to dev

  • Grégoire Kubler changed target branch from main to dev

    changed target branch from main to dev

  • Hi @cguillon

    Which branch to merge

    I'm glad to see other people working on the setup.py! (I've been doing work on my branch to package the project) Since I'm modifying the setup.py also, do you mind to merge your work on my branch ? It is named feat/release_pip. My job on the setup.py is mostly done on it so you can modify it without risks :)

    Ninja & editable install

    Also for now I've tested locally and your feature works but it needs to pass the CI to be merged.

    Tbh I didn't know that editable install and Ninja were a thing until now, glad to learn about that!

    For ninja do you have an idea if it is available on windows as well ? If so this could speed up the windows build that is reaaaally long.

    Edited by Grégoire Kubler
  • Hi,

    Thanks, for the first point: I would prefer in this case to wait that your own changes are merged to dev. Then I will rebase as needed. Note that I'm not merging myself (i'm an external contributor on a fork), I suppose that you will do it when ok. I mark the change as draft for now, please notify me when your work is merged to dev.

    For Ninja, I suppose it is availabale on windows, perhaps (as for cmake) there is a standard pip package which provides it, I will check (added to TODO list above).

    Otherwise, I add some questions above, here summarized, do you have an idea?

    • how does the setup script works for windows as it is actually copying ".so" and not ".dll" files?
    • do you known if it's possible to do symbolic link on windows for the libs and version files (in the case where we enable development on windows also)?
  • Christophe Guillon marked this merge request as draft

    marked this merge request as draft

  • Christophe Guillon changed the description

    changed the description

    • Resolved by Christophe Guillon

      Ok thanks! I'll look up to this story of ninja on my side.

      If there is indeed a ninja pip package then I might ask you to add ninja to the project build_depedencies in the pyproject.toml.

      about the questions

      1

      I haven't looked into that as I'm pretty noob as a windows software dev. Now that you say it, I have no clue how it can work as I wonder that nmake generates dll. I need to work on windows for other reasons this week I'll keep you updated.

      2

      1. Yes windows accepts symlinks (said chatgpt)
      New-Item -ItemType SymbolicLink -Path "LinkPath" -Target "TargetPath"
      1. @cmoineau can correct me if I'm wrong but windows is mostly user oriented and not dev oriented. Hence we don't need to enable the editable install on windows.
      Edited by Grégoire Kubler
  • Grégoire Kubler requested review from @gregkub

    requested review from @gregkub

  • Also don't forget to create an aidge merge request where the submodule hash of aidge core is updated.

  • Christophe Guillon changed the description

    changed the description

    • Resolved by Christophe Guillon

      I checked that there is indeed an official package for ninja (used in many active project, actually to have more portable/efficient builds across platforms with cmake+ninja).

      Hence we can actually add it to build dependencies in addition to cmake.

      To be tested on your side, but I suppose we could also make ninja the default instead of make as it is generally more robust to dependencies changes and faster.

  • added 2 commits

    • 84bbbf7e - [Setup] Remove cache files from package
    • 9298e5f6 - [Setup] Add support for development install

    Compare with previous version

  • Christophe Guillon changed the description

    changed the description

  • Grégoire Kubler mentioned in issue #137

    mentioned in issue #137

  • Christophe Guillon changed the description

    changed the description

  • Updated description and Tests for the windows case. I will test the symbolic links on windows soon to complete the picture. Though anyway, with the current patch linux and windows normal/editable mode should pass.

  • added 28 commits

    • 9298e5f6...956bb3f0 - 24 commits from branch eclipse/aidge:dev
    • 4ac48d01 - [Setup] Remove cache files from package
    • 733b9b23 - [Setup] Add support for development install
    • 1422bcae - [Setup] Add env var for tests
    • e87ef819 - [Setup] Use symlinks in editable mode post install

    Compare with previous version

  • Christophe Guillon marked this merge request as ready

    marked this merge request as ready

  • Christophe Guillon changed the description

    changed the description

  • Grégoire Kubler
  • Christophe Guillon marked this merge request as draft

    marked this merge request as draft

  • Christophe Guillon changed the description

    changed the description

  • Christophe Guillon added 184 commits

    added 184 commits

    • e87ef819...2ea2cd6b - 178 commits from branch eclipse/aidge:dev
    • 9ce9c50c - [Setup] Remove useless copy of version.txt in python package
    • ac291c3f - [CMake] Minor update for comment location
    • 9692ff11 - [Setup] Allow empty AIDGE_BUILD_GEN type
    • 1f76661a - [Setup] Remove additional build dependency on toml
    • dd7fdacf - [CMake] Add installation of python binding
    • 1e321715 - [Setup] Add support for editable mode

    Compare with previous version

  • Christophe Guillon marked this merge request as ready

    marked this merge request as ready

  • Christophe Guillon changed the description

    changed the description

  • Christophe Guillon resolved all threads

    resolved all threads

    • Resolved by Christophe Guillon

      @gregkub I updated the merge request, which provides now a more straightforward solution actually.

      I let you comment and optionally merge.

      Note that there are other packages (aidge_backend_cpu for instance) with python bindings for which I could report the support. I did not check if you also updated the other packages w.r.t. the pyproject.toml build. I let you tell me, thanks.

  • Thanks for your work ! I really like your MR its going to be a real quality of life improvement for the devs !

    Also thank you for the explicit readme.

    For later, if we are planning to port these modifications on other repositories, it might be interesting to move this section in the wiki and put a big link in the readme as these options will be universal.

  • @gregkub for information you may look at the RFC (request for comments) there #147 which is related to the problems you encounter with link time issues, prohibiting PYBIND+TEST without doing a double compilation, or with the link time issues you had with the test_export.py unit test.

  • added 1 commit

    • 9b54fa60 - [Setup] Add support for editable mode

    Compare with previous version

  • Christophe Guillon resolved all threads

    resolved all threads

  • mentioned in commit 1c21952e

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading