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 patching functionality #156

Open
joxoby opened this issue Jun 29, 2020 · 5 comments
Open

Add patching functionality #156

joxoby opened this issue Jun 29, 2020 · 5 comments

Comments

@joxoby
Copy link

joxoby commented Jun 29, 2020

There's sometimes the need to apply proprietary changes to open-source repositories. One approach is to fork+commit+pull. Another approach is to pull+patch, where the patch(es) can be located either locally or remotely. Maybe this is a functionality that is worth pursuing?

I was thinking something like:

repositories:
  vcstool:
    type: git
    url: [email protected]:dirk-thomas/vcstool.git
    version: 0.2.11
    patch: patches:https://vcstool/000_add_patch_functionality.patch

Sometimes maintaining a patch is easier than a fork.

@joxoby joxoby changed the title Add patch functionalty Add patch functionality Jun 29, 2020
@joxoby joxoby changed the title Add patch functionality Add patching functionality Jun 29, 2020
@dirk-thomas
Copy link
Owner

That sounds like an interesting feature. I few thoughts:

  • It would be good to be able to separate the action from the import verb. A separate verb like patch could be added so a user can call patch on an already existing working copy.
  • It would be useful to be able to apply more than one patch. So the patch: entry could hold a list and therefore be named patches:.
  • I don't understand the patches:https:// schema. Can you clarify what you want to express with it? Why not use standard URLs like https or file?

Are you interested to contribute pull requests for this feature idea?

@joxoby
Copy link
Author

joxoby commented Jun 30, 2020

It would be good to be able to separate the action from the import verb. A separate verb like patch could be added so a user can call patch on an already existing working copy.

If patch is a separate verb (which makes sense), where would we store the patches' URLs? I'm assuming here that patch shouldn't need access to the YAML file, such that the workflow would be:

vcs import < repos.yaml
vcs patch

But even if we don't make it a separate verb to bypass that problem, it gets tricky when the user calls pull for example.

It would be useful to be able to apply more than one patch. So the patch: entry could hold a list and therefore be named patches.

Absolutely.

I don't understand the patches:https:// schema. Can you clarify what you want to express with it? Why not use standard URLs like https or file?

Some systems can implement that. However, not important, it might be a bad example. https or file are fine.

Are you interested to contribute pull requests for this feature idea?

I am. I might need to find some time for it...

@dirk-thomas
Copy link
Owner

If patch is a separate verb (which makes sense), where would we store the patches' URLs? I'm assuming here that patch shouldn't need access to the YAML file, such that the workflow would be:

vcs import < repos.yaml
vcs patch

vcs patch is certainly not sufficient as the invocation since the information about what patches to apply and where to apply them must come from somewhere. So I think it still needs a yaml file as input.

That yaml file can have a similar structure as the .repos file. Though each directory entry doesn't need type / url / version but only a patches key.

That allows you to:

  • either store the patches in the same .repos file (and vcs import could pick them up automatically or opt-in when present)
  • or use two separate yaml files (e.g. when the first one containing the repos is generated by something like rosinstall_generator and you want to manually maintain a yaml file which just contains the patches.)

it gets tricky when the user calls pull for example.

Since inherently patch modifies the working copy that will be the case. E.g. you won't be able to run the patch command twice.

@yajo
Copy link

yajo commented Feb 25, 2022

Some comments on this, on which I'm very interested:

  1. I think it's more ergonomic to add a patches key into the repositories entry, than adding a patches root key in the yaml definition. Just like the OP, but plural.
  2. A separate command is OK, but I think there should be a single command that just consumes a lock file and produces a directory structure according to what's specified in that lock file.
  3. It would be very nice to be able to add or remove patches via terminal, e.g. vcs patch [add|rm] ./the/repo https://github.com/dirk-thomas/vcstool/issues/156.patch
  4. Patches should support a hash, so you are safe to store them remotely but still get a proper failure if it changes.
  5. If the hash is not supplied, when doing vcs export it should be added automatically (or at least have a flag for it), to serve as a proper lock file.

So it would look like:

repositories:
  vcstool:
    type: git
    url: [email protected]:dirk-thomas/vcstool.git
    version: 0.2.11
    patches: 
      - ./000_add_patch_functionality.patch
      - url: https://github.com/dirk-thomas/vcstool/issues/156.patch
        sha256: asdf0978asdf0987asdf0978asdf0978

@Volksfest
Copy link

Are there any advances here?

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

No branches or pull requests

4 participants