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

Fix stdeb for python 3 #648

Merged
merged 7 commits into from
Jul 17, 2021
Merged

Fix stdeb for python 3 #648

merged 7 commits into from
Jul 17, 2021

Conversation

timonegk
Copy link
Member

This is my attempt to fix #594. The following changes are done:

  • replace dependency catkin-pkg with catkin-pkg-modules (> 0.2.9 can be dropped because catkin-pkg-modules was introduced with 0.3.0)
  • Depends and Conflicts (i. e., the python 2 settings) were removed from stdeb.cfg because no further python 2 packages will be built.
  • python3-osrf-pycommon was added as a deb dependency (I don't know why it was missing)

I tested the changes on five different systems:

  • Debian 9 Stretch (python 3.5.3)
  • Debian 10 Buster (python 3.7.3)
  • Ubuntu 16.04 Xenial Xerus (python 3.5.2)
  • Ubuntu 18.04 Bionic Beaver (python 3.6.9)
  • Ubuntu 20.04 Focal Fossa (python 3.8.5)

Once all build dependencies were installed, I used the command python3 setup.py --command-packages=stdeb.command sdist_dsc --with-python2=False bdist_deb to create the deb file. It could be installed on all systems and catkin commands could be executed without error.
The mentioned systems cover all the systems in Suite, assuming that the non-LTS Ubuntu releases behave like either the predecessor or the successor.

Before the release with these changes, the trivial fix in osrf/osrf_pycommon#71 should be merged. Currently, python3-osrf-pycommon and python-osrf-pycommon don't conflict in all cases, resulting in annoying apt errors on installing python3-osrf-pycommon when python-osrf-pycommon was already installed.

Before the release, the version in setup.py an the changelog should obviously be updated.

@timonegk
Copy link
Member Author

There is still a problem, we cannot require catkin-pkg-modules in setup.py because that will break catkin_tools on systems where no apt packages for ros are available (catkin-pkg-modules is not a pip package). Therefore we have to use catkin-pkg >= 0.3.0 in setup.py. This will now break on systems where only python3-catkin-pkg-modules is installed, because catikn-pkg is not available there. But python3-catkin-pkg can not be a requirement because it conflicts with ros. The best option might be to remove catkin-pkg(-modules) completely from setup.py – or find a way to depend on one of the two depending on pypi/deb installation...

@timonegk
Copy link
Member Author

The cleanest solution (in my opinion) would be to create a pypi package catkin-pkg-modules that contains no code but depends on catkin-pkg, essentially a meta package or wrapper around catkin-pkg. Then, catkin-pkg-modules could be the dependency for deb and pypi/pip.

@timonegk
Copy link
Member Author

Okay, I found another way, as ros_buildfarm is doing it.

@timonegk timonegk force-pushed the fix/stdeb branch 3 times, most recently from c9885c2 to 5b63124 Compare February 11, 2021 00:01
@timonegk
Copy link
Member Author

Sorry for the spam, now everything works with pip and with stdeb. Yay!

@gavanderhoorn

This comment has been minimized.

@timonegk
Copy link
Member Author

As you found out, the build failure for ros-melodic-catkin is completely unrelated, this repository has no connection to ros-melodic-catkin.

@mikepurvis
Copy link
Member

Looks sane to me; thanks for the thorough effort at testing this!

@timonegk timonegk requested a review from wjwwood March 1, 2021 22:03
@timonegk
Copy link
Member Author

timonegk commented Mar 1, 2021

I just added the changelog. Maybe you'll manage to take a look at it in the next few days.

stdeb.cfg Outdated
Conflicts3: python-catkin-tools
Suite: xenial yakkety zesty artful bionic cosmic disco eoan focal stretch buster
Suite3: xenial yakkety zesty artful bionic cosmic disco eoan focal stretch buster
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this line change is necessary. The separate Suite3 config field is only needed when the release suites differ between Python 2 and Python 3.

I'm pretty certain that stdeb defaults to unstable if no suites are configured although I haven't confirmed that. Leaving this line as it was while adding the No-Python2 field below should be sufficient to halt all further python2 releases of this package.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you seem to be right. Thank you, I changed it back.

@timonegk timonegk merged commit 11da5e0 into master Jul 17, 2021
@timonegk timonegk deleted the fix/stdeb branch July 17, 2021 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python 3 release - will be 0.7.0
5 participants