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

Fix folder removal #763

Closed
wants to merge 2 commits into from

Conversation

rhaschke
Copy link
Contributor

Trying to keep a single folder from a larger source repository, I ran into several issues:

  1. It's not easy to keep a single folder, but delete all others
  2. Deleting folders as described in the docs doesn't work:
    UPSTREAM_WORKSPACE='github:ros-controls/ros_control#melodic-devel -rqt_controller_manager'
    The folder should be specified with the repository prefix: -ros_control/rqt_controller_manager. However, this is clumsy.

This PR

  1. remembers the last imported repository name
  2. tries to delete a folder from that repo (thus matching the docs)
  3. adds syntax +folder to keep a specific folder

Example run: https://github.com/rhaschke/test/runs/4654479819

Remember last imported repository and consider file to-be-removed
w.r.t. this repo as indicated by docs.
... to source space.
This facilitates keeping a few folders from a large imported source repository,
e.g. github.com:ros-planning/moveit#master +moveit_core -moveit
@mathias-luedtke
Copy link
Member

Deleting folders as described in the docs doesn't work:

Oh, the documentation needs to be fixed!

remembers the last imported repository name

This only works with directly specified repositories, not with rosinstall files (which are recommended for larger setups)

tries to delete a folder from that repo (thus matching the docs)

This adds ambiguity, especially, if you mix instructions.

if [ -e "${sourcespace:?}/$file" ]; then break; fi
done
ici_log "Moving '${sourcespace:?}/$file' to '${sourcespace:?}'"
ici_guard mv "${sourcespace:?}/$file" "${sourcespace:?}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not copy parent folders..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which parent folder you are referring to? I'm assuming that you would move folders below $sourcespace only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repo/sub1/sub2/abc would end up in ${sourcespace}/abc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Does that harm?
Of course, your syntax proposal that filters folders within a repo parent folder, will be much better. No risk for conflicts ;-)

@rhaschke
Copy link
Contributor Author

I'm not quite sure how to interpret your feedback.
I guess you prefer keeping fully-specified paths only and fixing the docs appropriately?
What do you think about introducing +folder syntax to facilitate keeping a few folders only? Do you have other suggestions for this use case?

@mathias-luedtke
Copy link
Member

Trying to keep a single folder from a larger source repository,

I was told that this could be done easily with SVN (which is still supported by Github).
But it would still make sense to come up with a better filter syntax.

Something like:
github:ros-controls/ros_control#melodic-devel(+rqt_controller_manager,-rqt_controller_manager/CHANGELOG.rst)

@rhaschke
Copy link
Contributor Author

I was told that this could be done easily with SVN (which is still supported by Github).

You can use git-svn to clone an svn repo with git, but not the other way around as far as I know.

But it would still make sense to come up with a better filter syntax, e.g.
github:ros-controls/ros_control#melodic-devel(+rqt_controller_manager,-rqt_controller_manager/CHANGELOG.rst)

That would clarify that the filter applies to the specific repo, which is good.
Do you plan to drop the generic -folder option then? This was much more generic, allowing to remove any file/folder from the source space.

@mathias-luedtke
Copy link
Member

You can use git-svn to clone an svn repo with git, but not the other way around as far as I know.

Apparently, it works the other way around as well.
I will investigate ;)

Do you plan to drop the generic -folder option then?

No, the old options should stay (used with rosinstall).
The new syntax would be special for all cases in which the target folder is known (vcs, path).

@rhaschke
Copy link
Contributor Author

Closing as Mathias will suggest a more specific approach.

@rhaschke rhaschke closed this Dec 29, 2021
@rhaschke rhaschke deleted the fix-folder-removal branch December 29, 2021 13:39
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

Successfully merging this pull request may close these issues.

2 participants