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

add config ini to ament_mypy site package #182

Merged

Conversation

Arnatious
Copy link
Contributor

fixes issue @jacobperron pointed out in sros discussion by adding ini to MANIFEST.in

Signed-off-by: Ted Kern ted.kern@canonical.com

Signed-off-by: Ted Kern <ted.kern@canonical.com>
@jacobperron
Copy link
Contributor

CI is failing Build Status:

--- stderr: ament_mypy
19:10:05 error: Error: setup script specifies an absolute path:
19:10:05 
19:10:05     /home/jenkins-agent/workspace/ci_linux/ws/build/ament_mypy/ament_mypy.egg-info/PKG-INFO
19:10:05 
19:10:05 setup() arguments must *always* be /-separated paths relative to the
19:10:05 setup.py directory, *never* absolute paths.

I'm not sure why.

Signed-off-by: Ted Kern <ted.kern@canonical.com>
@Arnatious
Copy link
Contributor Author

That is weird, doesn't grok with the changes made.

@Arnatious
Copy link
Contributor Author

The generation is handled automatically, and the path is provided as relative in the manifest, I'm not sure where this error is coming from.

writing manifest file '/home/jenkins-agent/workspace/ci_linux/ws/build/ament_mypy/ament_mypy.egg-info/SOURCES.txt'
reading manifest file '/home/jenkins-agent/workspace/ci_linux/ws/build/ament_mypy/ament_mypy.egg-info/SOURCES.txt'
error: Error: setup script specifies an absolute path:
reading manifest template 'MANIFEST.in'
writing manifest file '/home/jenkins-agent/workspace/ci_linux/ws/build/ament_mypy/ament_mypy.egg-info/SOURCES.txt'
copying ament_mypy/__init__.py -> /home/jenkins-agent/workspace/ci_linux/ws/build/ament_mypy/build/lib/ament_mypy

    /home/jenkins-agent/workspace/ci_linux/ws/build/ament_mypy/ament_mypy.egg-info/PKG-INFO

setup() arguments must *always* be /-separated paths relative to the
setup.py directory, *never* absolute paths.

@dirk-thomas
Copy link
Contributor

Please use package_data instead of a MANIFEST.in file like in other packages, e.g.

package_data={'': [
'configuration/ament_code_style.cfg',
]},

Signed-off-by: Ted Kern <ted.kern@canonical.com>
@Arnatious
Copy link
Contributor Author

Done. There a reason to prefer it/that it caused this issue? Documentation for modern python seems to prefer the MANIFEST.in method.

@dirk-thomas
Copy link
Contributor

Interesting, I was under the impression of the opposite to be the case.

Atm no ROS 2 package uses a MANIFEST.in file. So I think it is good to keep the same pattern. If there is a good reason to change I am more than happy to do so if that is what is the recommended way. Please provide some references if you have them at hand.

@dirk-thomas dirk-thomas added the bug Something isn't working label Aug 20, 2019
@dirk-thomas dirk-thomas merged commit cc10008 into ament:master Aug 20, 2019
@Arnatious
Copy link
Contributor Author

Arnatious commented Aug 20, 2019

@dirk-thomas

@dirk-thomas
Copy link
Contributor

The sampleproject is indeed a credible reference. I am open to transition to MANIFEST.in and include_package_data=True. It would need to be ensured that all our toolchains work fine with it (e.g. colcon, symlink installs, Debian packaging, if we want to apply this to distro-agnostic Python packages stdeb).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants