Install Python metadata through pip #8

diegoferigo · 2021-10-18T13:06:15Z

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

Requires robotology/idyntree#922 and conda-forge/staged-recipes#16572. cc @traversaro

conda-forge-linter · 2021-10-18T13:06:19Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

diegoferigo · 2021-10-18T13:06:44Z

@conda-forge-admin please rerender

…a-forge-pinning 2021.10.18.05.45.34

traversaro · 2021-10-18T16:12:53Z

Requires robotology/idyntree#922 and conda-forge/staged-recipes#16572. cc @traversaro

While we wait for those PRs to go through, can you explain the rationale for this change? From what I understand we are mixing Python bindings compiled with setup.py and the rest of iDynTree libraries compiled via CMake, with no mechanism in place to make sure that the same CMake options are set to both builds.

diegoferigo · 2021-10-18T16:26:03Z

Requires robotology/idyntree#922 and conda-forge/staged-recipes#16572. cc @traversaro

While we wait for those PRs to go through, can you explain the rationale for this change? From what I understand we are mixing Python bindings compiled with setup.py and the rest of iDynTree libraries compiled via CMake, with no mechanism in place to make sure that the same CMake options are set to both builds.

Rationale:

Have a 1:1 match between what is deployed to PyPI and what is distributed through conda-forge.
The setup.py file becomes the entrypoint for both PyPI and conda-forge to enable / disable features.
If needed, the python files can become a subpackage and passing through this new method would be simpler to maintain.
The idyntree Python package metadata gets included in the idyntree conda-forge package, meaning that pip list would show that idyntree is installed and can be imported.
If the user installs with pip a package from PyPI that has idyntree as its dependency, the files already part of the environment but now tracked by pip will get no longer overridden.

traversaro · 2021-10-19T07:43:03Z

The setup.py file becomes the entrypoint for both PyPI and conda-forge to enable / disable features.

That was the part I was afraid of. To enable/disable features in conda-forge we have the options listed in the bld.bat/build.sh . I am a bit afraid that if we duplicate those options in yet another place (that my the way is not part of the recipe, so it can easily change without being clear in the version bump PR) this may lead to inconsistencies.

The idyntree Python package metadata gets included in the idyntree conda-forge package, meaning that pip list would show that idyntree is installed and can be imported.

If the user installs with pip a package from PyPI that has idyntree as its dependency, the files already part of the environment but now tracked by pip will get no longer overridden.

That seems the interesting feature we are looking for. If I understand well, this can be implemented simply by having something in ${SP_DIR}/idyntree-<version>.dist-info/METADATA file, where most of that METADATA is already in https://github.com/robotology/idyntree/blob/c2010240a1d56fe01e1be5ab20e3b16bcc568856/setup.py#L15 . Do you know if there is a way for generate just the METADATA that we need from setup.py, without the need to re-compile all the library?

traversaro · 2021-10-19T07:47:10Z

Some pointers for future reference:

traversaro · 2021-10-19T08:01:36Z

Related PR: conda-forge/nlopt-feedstock#37 (even if perhaps not ideal as it just installs an empty METADATA file, not fully compliant to https://packaging.python.org/specifications/recording-installed-packages/ and https://packaging.python.org/specifications/core-metadata/#core-metadata where at least a non-empty METADATA with the Metadata-Version, Name and Version fiels are required).

diegoferigo · 2021-10-19T08:21:04Z

The setup.py file becomes the entrypoint for both PyPI and conda-forge to enable / disable features.

That was the part I was afraid of. To enable/disable features in conda-forge we have the options listed in the bld.bat/build.sh . I am a bit afraid that if we duplicate those options in yet another place (that my the way is not part of the recipe, so it can easily change without being clear in the version bump PR) this may lead to inconsistencies.

Yep I totally understand the duplication concern, but I want to bring two more points to your attention because this change imo goes in the direction of simplifying rather the duplicating:

Having a 1:1 feature match between PyPI and conda-forge is fundamental to have Python code (bindings in this way) that provide the same number of features, making all downstream projects compatible regardless of the mean of installation. The simplest way is for conda-forge to pass through setuptools (instead of the other way around). Currently, setup.py contains the default CMake configuration used to package for PyPI, however diegoferigo/cmake-build-extension allows to override them from the command line, as done in this PR for BUILD_SHARED_LIBRARIES. This means that 1) we start with the same features of PyPI and 2) if we need to modify / add features, we can do it in the build.* scripts as it already happens right now. This way, we don't have to mind in most cases to keep aligned the cmake options used for Python because they are already specified in setup.py.
Passing through pip or pypa/build would be in any case necessary in the case where other Python resources that are not generated by CMake are involved. And I'm thinking of a possible future high-level Python wrappers of iDynTree classes, or similar. I think that passing through these tools would make the recipe more generic and consistent.

The idyntree Python package metadata gets included in the idyntree conda-forge package, meaning that pip list would show that idyntree is installed and can be imported.

If the user installs with pip a package from PyPI that has idyntree as its dependency, the files already part of the environment but now tracked by pip will get no longer overridden.

That seems the interesting feature we are looking for. If I understand well, this can be implemented simply by having something in ${SP_DIR}/idyntree-<version>.dist-info/METADATA file, where most of that METADATA is already in https://github.com/robotology/idyntree/blob/c2010240a1d56fe01e1be5ab20e3b16bcc568856/setup.py#L15 . Do you know if there is a way for generate just the METADATA that we need from setup.py, without the need to re-compile all the library?

This, with also the other resources you linked in later comments, is another possibility. Though, this would mean adding new logic to the recipe to automatically generate this file, which is already done for us by pip or pypa/build (maybe there's a python function somewhere that takes the setup.cfg and produces the metadata). This being said, although this seems a good option, if I were the maintainer I would prefer going towards this PR, but any solution like the autogeneration of METADATA would work.

As discussed f2f, a problem you raised is how to handle the automatic generation of the PyPI version since it's done from the git archive, and the github release tarball does not have git metadata. In this case, I think that a single sed like that writes to setup.py|cfg the conda-forge version would suffice as a workaround. Though, again, the automatic generation of METADATA has a similar complexity.

diegoferigo · 2021-10-19T08:49:08Z

Update: you can generate the METADATA passing through setup.py:

idyntree on  feature/python_cmake_component via △ v3.21.3 via 🐍 v3.8.10 🅒 /conda  
❯ python setup.py dist_info
running dist_info
writing idyntree.egg-info/PKG-INFO
writing dependency_links to idyntree.egg-info/dependency_links.txt
writing requirements to idyntree.egg-info/requires.txt
writing top-level names to idyntree.egg-info/top_level.txt
adding license file 'LICENSE.LGPL2'
adding license file 'LICENSE.LGPL3'
writing manifest file 'idyntree.egg-info/SOURCES.txt'
creating '/home/dferigo/git/idyntree/idyntree.dist-info'
adding license file "LICENSE.LGPL3" (matched pattern "LICEN[CS]E*")
adding license file "LICENSE.LGPL2" (matched pattern "LICEN[CS]E*")

idyntree on  feature/python_cmake_component via △ v3.21.3 via 🐍 v3.8.10 🅒 /conda  
❯ ls idyntree.dist-info/
LICENSE.LGPL2  LICENSE.LGPL3  METADATA  top_level.txt

diegoferigo · 2021-10-19T10:55:55Z

If your concern is to keep the CMake options consistent, I just realized that I can update this PR as follows:

Revert back to building with CMake the entire project, Python bindings included.
Use setup.py without any CMakeExtension by passing --no-cmake-extension=all option. It would install only the metadata without the need to do it manually, avoiding possible mistakes of operating in low-level.
This still needs the workaround to store the right version in setup.py|cfg, but it is fairly easy to do.

This seems the best trade off to keep a small complexity while preventing ABI-related surprises. What do you think?

Conda build output

[...]
running bdist_wheel
running build
running build_ext
installing to build/bdist.linux-x86_64/wheel
running install
running install_lib
warning: install_lib: 'build/lib.linux-x86_64-3.8' does not exist -- no Python modules to install

running install_egg_info
running egg_info
creating idyntree.egg-info
writing idyntree.egg-info/PKG-INFO
writing dependency_links to idyntree.egg-info/dependency_links.txt
writing entry points to idyntree.egg-info/entry_points.txt
writing requirements to idyntree.egg-info/requires.txt
writing top-level names to idyntree.egg-info/top_level.txt
writing manifest file 'idyntree.egg-info/SOURCES.txt'
adding license file 'LICENSE.LGPL2'
adding license file 'LICENSE.LGPL3'
writing manifest file 'idyntree.egg-info/SOURCES.txt'
Copying idyntree.egg-info to build/bdist.linux-x86_64/wheel/idyntree-4.1.1.dev21-py3.8.egg-info
running install_scripts
adding license file "LICENSE.LGPL2" (matched pattern "LICEN[CS]E*")
adding license file "LICENSE.LGPL3" (matched pattern "LICEN[CS]E*")
creating build/bdist.linux-x86_64/wheel/idyntree-4.1.1.dev21.dist-info/WHEEL
creating '$SRC_DIR/dist/tmp79b2g2ia/idyntree-4.1.1.dev21-cp38-cp38-linux_x86_64.whl' and adding 'build/bdist.linux-x86_64/wheel' to it
adding 'idyntree-4.1.1.dev21.dist-info/LICENSE.LGPL2'
adding 'idyntree-4.1.1.dev21.dist-info/LICENSE.LGPL3'
adding 'idyntree-4.1.1.dev21.dist-info/METADATA'
adding 'idyntree-4.1.1.dev21.dist-info/WHEEL'
adding 'idyntree-4.1.1.dev21.dist-info/entry_points.txt'
adding 'idyntree-4.1.1.dev21.dist-info/top_level.txt'
adding 'idyntree-4.1.1.dev21.dist-info/RECORD'
removing build/bdist.linux-x86_64/wheel
* Building wheel...
Successfully built idyntree-4.1.1.dev21-cp38-cp38-linux_x86_64.whl
Processing ./dist/idyntree-4.1.1.dev21-cp38-cp38-linux_x86_64.whl
Installing collected packages: idyntree
Successfully installed idyntree-4.1.1.dev21

Resource usage statistics from building idyntree:
   Process count: 43
   CPU time: Sys=0:00:22.3, User=0:08:52.2
   Memory: 3.1G
   Disk usage: 500.0K
   Time elapsed: 0:02:38.0

traversaro · 2021-10-19T11:33:59Z

This seems the best trade off to keep a small complexity while preventing ABI-related surprises. What do you think?

I think that is by far the best option! We also would not need a iDynTree new release.

diegoferigo · 2021-10-19T12:07:33Z

This seems the best trade off to keep a small complexity while preventing ABI-related surprises. What do you think?

I think that is by far the best option! We also would not need a iDynTree new release.

A new release should not be needed. I just pushed the first attempt of this solution, however this will work only in master since in devel we refactored the python files and we need to move the version workaround from setup.py to setup.cfg. Let's remember as soon as we do a new idyntree release.

diegoferigo · 2021-10-19T13:10:14Z

@conda-forge-admin please rerender

github-actions · 2021-10-19T13:11:35Z

Hi! This is the friendly automated conda-forge-webservice.
I tried to rerender for you, but it looks like there was nothing to do.

diegoferigo · 2021-10-19T14:48:34Z

Now linux and macOS seems happy, windows instead stills fail very likely because I made mistakes in the bat script, I'm not too familiar with that environment.

traversaro · 2021-10-19T15:47:34Z

I guess you do not need to escape the last line of the pip command.

traversaro · 2021-10-20T19:10:31Z

I see that arm64 also failed due to missing diffutils. This is more problematic as with all the NVIDIA SOC that we are starting to use on the robots, I wanted to make sure that idyntree arm64 binaries are readily available if we need them.

traversaro · 2021-10-20T19:11:35Z

I see that arm64 also failed due to missing diffutils. This is more problematic as with all the NVIDIA SOC that we are starting to use on the robots, I wanted to make sure that idyntree arm64 binaries are readily available if we need them.

Hopefully it will be fixed by conda-forge/conda-forge-pinning-feedstock#2046 .

- restore modified files before finishing - let pip find the wheel without specifying the name of the file

it returns 0 only if there's no diff

diegoferigo · 2021-10-21T10:15:08Z

@traversaro I managed to fix also the windows builds, if you want to make a more thorough review I think I'm mostly done now. No prob if you want to wait the finalization of the diffutils migration before merging.

traversaro · 2021-10-21T12:36:33Z

@traversaro I managed to fix also the windows builds, if you want to make a more thorough review I think I'm mostly done now. No prob if you want to wait the finalization of the diffutils migration before merging.

Great! Yes, thanks, I would prefer for conda-forge/diffutils-feedstock#4 to be merged to verify that CI for aarch64 is working before merging.

traversaro · 2021-11-04T08:02:44Z

@conda-forge-admin, please rerender

traversaro · 2021-11-04T08:03:08Z

I switched to cross-compilation to workaround conda-forge/status#122 .

…nda-forge-pinning 2021.11.03.23.52.26

traversaro · 2021-11-04T08:47:50Z

Cross-compilation has problems described in https://github.com/conda-forge/conda-smithy/issues/1539 .

traversaro · 2021-11-04T09:07:01Z

@conda-forge-admin, please rerender

github-actions · 2021-11-04T09:07:57Z

Hi! This is the friendly automated conda-forge-webservice.
I tried to rerender for you but ran into some issues, please ping conda-forge/core
for further assistance. You can also try re-rendering locally.

traversaro · 2021-11-04T09:11:43Z

@conda-forge-admin, please rerender

github-actions · 2021-11-04T09:12:39Z

Hi! This is the friendly automated conda-forge-webservice.
I tried to rerender for you but ran into some issues, please ping conda-forge/core
for further assistance. You can also try re-rendering locally.

traversaro · 2021-11-04T09:14:19Z

@conda-forge-admin, please rerender

…nda-forge-pinning 2021.11.03.23.52.26

github-actions · 2021-11-04T11:52:19Z

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

linter: passed
azure: passed

Thus the PR was passing and merged! Have a great day!

Install Python resources through pip

e0593e0

diegoferigo requested a review from traversaro as a code owner October 18, 2021 13:06

MNT: Re-rendered with conda-build 3.21.4, conda-smithy 3.12, and cond…

8fba95d

…a-forge-pinning 2021.10.18.05.45.34

diegoferigo mentioned this pull request Oct 18, 2021

Distribute through conda-forge robotology-legacy/gym-ignition#407

Open

10 tasks

diegoferigo added 5 commits October 19, 2021 13:59

Revert to building Python bindings with CMake

f126f14

Install only the metadata with pip

45f3b46

Add replace.py script that works like sed but multiplatform

925d7f6

Align the wheel version with the conda version

2c4c578

Update also bat script to install Python metadata

bcb06eb

Fix quoting

2c11150

Fix path to replace.py script

566dd68

diegoferigo force-pushed the feature/python_package branch from a256ee7 to 566dd68 Compare October 19, 2021 13:30

diegoferigo added 2 commits October 19, 2021 15:47

Fix quoting (again)

8137184

Check that conda version matches with python package version

87835d0

traversaro mentioned this pull request Oct 20, 2021

Add diffutils to arch migrations conda-forge/conda-forge-pinning-feedstock#2046

Merged

diegoferigo added 3 commits October 21, 2021 10:05

Update build scripts

9b41f5b

- restore modified files before finishing - let pip find the wheel without specifying the name of the file

Disable checking exit status of diff command

a7f8ce0

it returns 0 only if there's no diff

rmdir on windows fails if the folder does not exist

6cf9b69

diegoferigo changed the title ~~Install Python resources through pip~~ Install Python metadata through pip Oct 21, 2021

traversaro closed this Nov 4, 2021

traversaro reopened this Nov 4, 2021

Switch aarch64 and ppc64 build to cross-compilation

859c2fa

MNT: Re-rendered with conda-build 3.21.4, conda-smithy 3.14.3, and co…

6738c2c

…nda-forge-pinning 2021.11.03.23.52.26

Update conda-forge.yml

903310f

Update conda-forge.yml

dd94607

MNT: Re-rendered with conda-build 3.21.4, conda-smithy 3.14.3, and co…

baaaf5b

…nda-forge-pinning 2021.11.03.23.52.26

traversaro added the automerge Merge the PR when CI passes label Nov 4, 2021

github-actions bot merged commit 748c0f7 into conda-forge:master Nov 4, 2021

traversaro approved these changes Nov 4, 2021

View reviewed changes

traversaro mentioned this pull request Nov 4, 2021

Cross-compilation tests do not work if libGL is required conda-forge/conda-forge-ci-setup-feedstock#173

Closed

traversaro mentioned this pull request Feb 2, 2022

python packages installed by ROBOTOLOGY_USES_PYTHON are not listed by pip list --verbose robotology/robotology-superbuild#1012

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install Python metadata through pip #8

Install Python metadata through pip #8

diegoferigo commented Oct 18, 2021 •

edited by traversaro

Loading

conda-forge-linter commented Oct 18, 2021

diegoferigo commented Oct 18, 2021

traversaro commented Oct 18, 2021

diegoferigo commented Oct 18, 2021 •

edited

Loading

traversaro commented Oct 19, 2021

traversaro commented Oct 19, 2021

traversaro commented Oct 19, 2021

diegoferigo commented Oct 19, 2021

diegoferigo commented Oct 19, 2021

diegoferigo commented Oct 19, 2021 •

edited

Loading

traversaro commented Oct 19, 2021 •

edited

Loading

diegoferigo commented Oct 19, 2021

diegoferigo commented Oct 19, 2021

github-actions bot commented Oct 19, 2021

diegoferigo commented Oct 19, 2021 •

edited

Loading

traversaro commented Oct 19, 2021

traversaro commented Oct 20, 2021

traversaro commented Oct 20, 2021

diegoferigo commented Oct 21, 2021 •

edited

Loading

traversaro commented Oct 21, 2021

traversaro commented Nov 4, 2021

traversaro commented Nov 4, 2021

traversaro commented Nov 4, 2021

traversaro commented Nov 4, 2021

github-actions bot commented Nov 4, 2021

traversaro commented Nov 4, 2021

github-actions bot commented Nov 4, 2021

traversaro commented Nov 4, 2021

github-actions bot commented Nov 4, 2021

Install Python metadata through pip #8

Install Python metadata through pip #8

Conversation

diegoferigo commented Oct 18, 2021 • edited by traversaro Loading

conda-forge-linter commented Oct 18, 2021

diegoferigo commented Oct 18, 2021

traversaro commented Oct 18, 2021

diegoferigo commented Oct 18, 2021 • edited Loading

traversaro commented Oct 19, 2021

traversaro commented Oct 19, 2021

traversaro commented Oct 19, 2021

diegoferigo commented Oct 19, 2021

diegoferigo commented Oct 19, 2021

diegoferigo commented Oct 19, 2021 • edited Loading

traversaro commented Oct 19, 2021 • edited Loading

diegoferigo commented Oct 19, 2021

diegoferigo commented Oct 19, 2021

github-actions bot commented Oct 19, 2021

diegoferigo commented Oct 19, 2021 • edited Loading

traversaro commented Oct 19, 2021

traversaro commented Oct 20, 2021

traversaro commented Oct 20, 2021

diegoferigo commented Oct 21, 2021 • edited Loading

traversaro commented Oct 21, 2021

traversaro commented Nov 4, 2021

traversaro commented Nov 4, 2021

traversaro commented Nov 4, 2021

traversaro commented Nov 4, 2021

github-actions bot commented Nov 4, 2021

traversaro commented Nov 4, 2021

github-actions bot commented Nov 4, 2021

traversaro commented Nov 4, 2021

github-actions bot commented Nov 4, 2021

diegoferigo commented Oct 18, 2021 •

edited by traversaro

Loading

diegoferigo commented Oct 18, 2021 •

edited

Loading

diegoferigo commented Oct 19, 2021 •

edited

Loading

traversaro commented Oct 19, 2021 •

edited

Loading

diegoferigo commented Oct 19, 2021 •

edited

Loading

diegoferigo commented Oct 21, 2021 •

edited

Loading