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

Ignore CATKIN_IGNORE paths only when build_type is catkin (same for AMENT_IGNORE) #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

janstrohbeck
Copy link

My use case: I have a shared source space in which both ROS1 and ROS2 packages reside (the migration to ROS2 is ongoing). Now I want to build the ROS1 packages using catkin_tools, same as before, and the ROS2 packages using colcon. To do this, I place CATKIN_IGNORE files in the ROS2 packages, and COLCON_IGNORE files in the ROS1 packages.

Problem: colcon-ros ignores the ROS2 packages due to an existing CATKIN_IGNORE.

I think colcon should ignore ROS2 packages only when there is a COLCON_IGNORE file present (or AMENT_IGNORE), and respect CATKIN_IGNORE only when the build_type is catkin, as indicated by the package.xml. This PR implements this behavior.

@janstrohbeck
Copy link
Author

Not sure why the CI fails, test/test_flake8.py does not report errors for me locally. Is the DeprecationWarning causing the failure?

@cottsay
Copy link
Member

cottsay commented Dec 9, 2021

FWIW, the current behavior prevents the parsing of the manifest in a directory blocked by a *_IGNORE file. If I had a WIP package that had an invalid manifest, this change would result in a parsing warning even if I dropped a COLCON_IGNORE file in the directory.

I'm not sure that's a common situation, but I thought I'd mention it.

@wodtko
Copy link

wodtko commented Jan 3, 2022

By the change, CATKIN_IGNORE or AMENT_IGNORE files are only considered, if the build type matches catkin/ament.
The handling of COLCON_IGNORE files is not altered.

@DavidATapia
Copy link

DavidATapia commented Jan 20, 2022

The author's outlined use-case seems in some ways like it should really be the expected behavior for colcon build -- and not instead just assume that colcon will be used to compile both ROS1 and ROS2 source packages which are co-mingled in the same workspace.

If colcon's expected behavior in this case seems ambiguous, then I think it makes sense to at least allow for a CLI way to remove colcon-ros to permit distinct ROS1 & ROS2 package exclusion, depending on the running build system. colcon-ros's inclusion is defined here: https://github.com/colcon/colcon-common-extensions/blob/master/setup.cfg#L46

If this PR isn't going to be merged, then is there a command-line way to remove colcon-ros package? Will the removal of this package have other unintended negative effects (besides ignoring the *_IGNORE directive)?

@dirk-thomas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants