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

Feature reques: Add an exclude tag to repository files for self-documenting exclusions (Linked to #19) #21

Open
jfrascon opened this issue Feb 18, 2025 · 3 comments
Assignees

Comments

@jfrascon
Copy link

jfrascon commented Feb 18, 2025

Hi @ErickKramer,

A few hours ago, I noticed that you merged the pull request implementing feature request #19 . I was about to comment on it, but you were faster in merging than I was in providing feedback.

Looking at your example:

go run main.go import -i test/nested_example.repos -f -l -r -x dependencies.repos /tmp/output
Importing from test/nested_example.repos
--------------------
=== /tmp/output/naoqi_driver2 ===
Successfully cloned git repository 'https://github.com/ros-naoqi/naoqi_driver2' with version 'main'

I see that the exclusion of a repository is specified directly in the command-line execution, which works well. However, I believe there is an area for improvement, if I may share my perspective.

Proposal: Embed the exclusion inside the repository file

These .repos files are often part of a project. Let’s consider the following scenario:

Imagine the following repository file (deps.repos) is part of my team's project:

repositories:
  twist_mux:
    type: git
    url: https://github.com/ros-teleop/twist_mux.git
    version: humble
    exclude: [twist_mux_deps.repos]

My team wants to clone the twist_mux repository but avoid cloning twist_msgs, which is listed in the twist_mux_deps.repos file, since twist_mux_msgs can be installed via rosdep. It is important to note that twist_mux_deps.repos is part of the cloned twist_mux repository itself.

With the current approach, each team member must run:

rv import -i deps.repos -r -x twist_mux/twist_mux_deps.repos

This works, but the exclusion is not self-documented within the repository file. As a result, we need to either document this in a README file or create a script to automate the import process.

Suggested Improvement

By allowing the exclude field inside the repository file itself, the exclusion would be self-contained and explicitly defined, reducing the need for additional documentation or scripting.

Additionally, I believe that adding the exclude tag would not interfere with the vcs tool, since vcs does not recognize this tag and would simply ignore it. This means that .repos files used with ripvcs would remain fully compatible with vcs.

I hope my explanation is clear, and I’d love to hear your thoughts on this suggestion.

Best regards,

@ErickKramer ErickKramer self-assigned this Feb 18, 2025
@ErickKramer
Copy link
Owner

Hi @jfrascon,
Thanks for your suggestion. That should be straightforward to implement and as you said, it should not make the .repos file incompatible between ripvcs and vcs.

I will add it to my backlog and bring it forth as soon as possible/

@jfrascon
Copy link
Author

Great, thanks @ErickKramer for your effort!

@Tacha-S
Copy link
Contributor

Tacha-S commented Feb 20, 2025

I think this feature is really great.

In deeply chained dependencies like a.repos -> b.repos -> c.repos -> d.repos ..., if I want to exclude something defined in c.repos, I have to be mindful of the exclusion not only when executing c.repos but also when executing b.repos and a.repos. I have always found this to be quite cumbersome.

I’m looking forward to seeing this proposal implemented.

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

No branches or pull requests

3 participants