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 logic for excluding group workaround dependencies #1040

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Apr 15, 2024

This represents the first step in the journey to proper support for group dependencies in ros_buildfarm. Specifically, it sets the mechanism by which we disable the existing "pre-resolved" group dependencies present in the package manifests in the source repositories.

I chose the name DISABLE_GROUPS_WORKAROUND to act as the "feature flag" for a tool to use when expressing that it has support for group dependencies and doesn't want the pre-resolved dependencies to be present. I'm open to suggestions for a better name.

An example of how the package manifests would be modified for this change:

-  <build_depend>rmw_connextdds</build_depend>
+  <build_depend condition="$DISABLE_GROUPS_WORKAROUND != 1">rmw_connextdds</build_depend>

All manifests which pre-resolve groups would need to change, which (I believe) includes all manifests containing a <group_depend> element.

At present, build_file.include_group_dependencies is never True in production, so this flag should never be activated and no change in behavior is expected to any supported platforms or distributions.

@nuclearsandwich
Copy link
Contributor

DISABLE_GROUPS_WORKAROUND

I don't think the conditional value needs to change otherwise the only reasonable value would grow long enough to be worthy of a Fiona Apple album title.

But a thing I think could be helpful: have a tracking issue that gives a scanty summary of the status quo, how we're planning to change it, and open to track progress on that would also give us a url to link in an XML comment explaining what this is for (and thus helping people decide when to use it).

If instead this is "buildfarm only" then using BUILDFARM in the condition name seems reasonable.
Also, we could consider adding the issue number as an ID to the string on the very remote chance that we have multiple GROUPS_WORKAROUNDs that we need to disable over time.

@cottsay
Copy link
Member Author

cottsay commented Apr 23, 2024

a tracking issue that gives a scanty summary of the status quo, how we're planning to change it, and open to track progress on that would also give us a url to link in an XML comment explaining what this is for

Done: ros-infrastructure/catkin_pkg#369

@cottsay cottsay merged commit d52a968 into master Jun 5, 2024
32 checks passed
@cottsay cottsay deleted the cottsay/group-workarounds branch June 5, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants