search

colcon / colcon-argcomplete

Completion for colcon command lines using argcomplete
http://colcon.readthedocs.io
Apache License 2.0
2 stars 3 forks source link

fix condition to use dst_prefix during deb_dist #25

Closed dirk-thomas closed 5 years ago

dirk-thomas commented 5 years ago

Follow up of #24.

The condition if not os.path.exists(src_base): was never True and therefore the dst_prefix was never used which resulted in the hooks to be installed into /usr/... instead of the local deb_dist directory.

With the updated condition ros_release_python deb3 doesn't try to write to /usr/... anymore and the hooks are part of the Debian package.

cottsay commented 5 years ago

This doesn't work for me:

cottsay@cottsay-box-bionic:/tmp$ git clone https://github.com/ros-infrastructure/ros_release_python
Cloning into 'ros_release_python'...
remote: Enumerating objects: 204, done.
remote: Total 204 (delta 0), reused 0 (delta 0), pack-reused 204
Receiving objects: 100% (204/204), 36.91 KiB | 2.17 MiB/s, done.
Resolving deltas: 100% (77/77), done.
cottsay@cottsay-box-bionic:/tmp$ git clone https://github.com/colcon/colcon-argcomplete -b fix-deb-dist
Cloning into 'colcon-argcomplete'...
remote: Enumerating objects: 61, done.
remote: Counting objects: 100% (61/61), done.
remote: Compressing objects: 100% (44/44), done.
remote: Total 165 (delta 24), reused 32 (delta 13), pack-reused 104
Receiving objects: 100% (165/165), 48.83 KiB | 5.42 MiB/s, done.
Resolving deltas: 100% (67/67), done.
cottsay@cottsay-box-bionic:/tmp$ cd colcon-argcomplete/
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete$ ../ros_release_python/scripts/ros_release_python deb3

== Determine package name and version
The Python package 'colcon-argcomplete' requires at least setuptools version 40.5.0

Command [python3 setup.py --name --version] returned non-zero exit status 1
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete$
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete$ BUILD_DEBIAN_PACKAGE=1 ../ros_release_python/scripts/ros_release_python deb3

== Determine package name and version
Traceback (most recent call last):
  File "setup.py", line 150, in <module>
    setup(cmdclass=cmdclass)
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 128, in setup
    _install_setup_requires(attrs)
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 121, in _install_setup_requires
    dist.parse_config_files(ignore_option_errors=True)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 494, in parse_config_files
    ignore_option_errors=ignore_option_errors)
  File "/usr/lib/python3/dist-packages/setuptools/config.py", line 110, in parse_configuration
    options.parse()
  File "/usr/lib/python3/dist-packages/setuptools/config.py", line 380, in parse
    self.section_prefix, section_name))
distutils.errors.DistutilsOptionError: Unsupported distribution option section: [options.data_files]

Command [python3 setup.py --name --version] returned non-zero exit status 1
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete$ python3 -c "import setuptools; print(setuptools.__version__)"
39.0.1
cottsay@cottsay-box-bionic:~$
cottsay commented 5 years ago

If I install the latest setuptools, this patch works, but this is what I was trying to describe when I said that I needed to manually patch setup.cfg when using older setuptools in the other thread.

cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete$ pip3 install setuptools
Collecting setuptools
  Using cached https://files.pythonhosted.org/packages/37/06/754589caf971b0d2d48f151c2586f62902d93dc908e2fd9b9b9f6aa3c9dd/setuptools-40.6.3-py2.py3-none-any.whl
Installing collected packages: setuptools
Successfully installed setuptools-40.6.3
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete$ python3 -c "import setuptools; print(setuptools.__version__)"
40.6.3

...after that, the above commands build as expected. As does my original fix in #23, which also works when packaging colcon-argcomplete for anything other than Debian.

cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete$ python3 -c "import setuptools; print(setuptools.__version__)"
40.6.3
cottsay@cottsay-box-bionic:/tmp$ git clone https://github.com/cottsay/colcon-argcomplete -b use_root_arg colcon-argcomplete-root-arg
Cloning into 'colcon-argcomplete-root-arg'...
remote: Enumerating objects: 64, done.
remote: Counting objects: 100% (64/64), done.
remote: Compressing objects: 100% (45/45), done.
remote: Total 168 (delta 26), reused 35 (delta 15), pack-reused 104
Receiving objects: 100% (168/168), 49.13 KiB | 3.78 MiB/s, done.
Resolving deltas: 100% (69/69), done.
cottsay@cottsay-box-bionic:/tmp$ cd colcon-argcomplete-root-arg/
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete-root-arg$ ../ros_release_python/scripts/ros_release_python deb3

== Determine package name and version
Package: colcon-argcomplete
Version: 0.3.0

...<SNIP>...

-- Skip uploading package to apt repositories
# dput -u -c ../ros_release_python/resources/dput.cf ros-bootstrap deb_dist/python3-colcon-argcomplete_0.3.0-1_source.changes
# dput -u -c ../ros_release_python/resources/dput.cf ros-bootstrap deb_dist/python3-colcon-argcomplete_0.3.0-1_amd64.changes
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete-root-arg$ echo $?
0
cottsay@cottsay-box-bionic:/tmp/colcon-argcomplete-root-arg$
dirk-thomas commented 5 years ago

This doesn't work for me:

What version of stdeb do you have installed?

If you invoke ros_release_python deb3 but get The Python package 'colcon-argcomplete' requires at least setuptools version 40.5.0 it sounds like your stdeb version doesn't set the environment variable as specified in the stdeb.cfg file.

The same for the second error. The CustomSdistDebCommand is applying the patch so the options.data_files section should be commented out.

As does my original fix in #23, which also works when packaging colcon-argcomplete for anything other than Debian.

It doesn't for me as mentioned in https://github.com/colcon/colcon-argcomplete/pull/23#issuecomment-452508849

cottsay commented 5 years ago

What version of stdeb do you have installed?

cottsay@cottsay-box-bionic:~$ python -c "import stdeb; print(stdeb.__version__)"
0.8.5
cottsay@cottsay-box-bionic:~$ python3 -c "import stdeb; print(stdeb.__version__)"
0.8.5
cottsay@cottsay-box-bionic:~$

It doesn't appear to make any difference if I use the pip version or the version that comes with Bionic or Cosmic - it's 0.8.5 in all cases.

python3 setup.py --name --version is invoked right at the beginning of ros_release_python, so sdist_dsc hasn't even been invoked yet, so I'm not sure how the patch could have been applied already.

Even if I hack around get_name_and_version, it just fails in the same way later, even though ros_release_python sets the environment variable specified in stdeb.cfg automatically. It seems like a chicken-and-egg - the call to sdist_dsc, which is supposed to patch the setup.cfg, can't run because setup.cfg hasn't been patched yet to remove the unsupported section.

...
-- Building sdist_dsc and bdist_deb package...
# python3 setup.py --command-packages=stdeb.command sdist_dsc --source python3-colcon-argcomplete --with-python3 true --with-python2 false --force-x-python3-version --debian-version 1 bdist_deb [with BUILD_DEBIAN_PACKAGE=1]
Traceback (most recent call last):
  File "setup.py", line 150, in <module>
    setup(cmdclass=cmdclass)
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 139, in setup
    _install_setup_requires(attrs)
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 132, in _install_setup_requires
    dist.parse_config_files(ignore_option_errors=True)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 495, in parse_config_files
    ignore_option_errors=ignore_option_errors)
  File "/usr/lib/python3/dist-packages/setuptools/config.py", line 110, in parse_configuration
    options.parse()
  File "/usr/lib/python3/dist-packages/setuptools/config.py", line 398, in parse
    self.section_prefix, section_name))
distutils.errors.DistutilsOptionError: Unsupported distribution option section: [options.data_files]
...

I simply can't get this to work without upgrading setuptools via pip.

I also can't reproduce your error in https://github.com/colcon/colcon-argcomplete/pull/23#issuecomment-452508849 no matter what I do. Stdeb always appears to invoke setup.py install with the --root argument.

dirk-thomas commented 5 years ago

I simply can't get this to work without upgrading setuptools via pip.

That is totally expected. You do need the lastest version in order to create a wheel which supports the section in the configuration file.

We can't just only use that approach because even when you have the latest version installed via pip when building the Debian package it will only use the older version of setuptools that is installed as a Debian package. That is why all the extra hoops depending on the environment variable are in the setup file.

cottsay commented 5 years ago

Hmm...I'd guess I didn't expect that it would be normal for multiple versions of setuptools to be involved, but I see how it is happening now.

dirk-thomas commented 5 years ago

we should still revisit the --root issue sometime so everyone else doesn't need to patch around this to get this package to install.

Since I don't see the behavior you are describing please re-check that all documented ways to install the package works for you (after this has been merged and a new patch release been made. If any of them don't work for you please open a new ticket describing exactly the steps as well as the environment and how it fails.