Add patching functionality

joxoby commented 4 years ago

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: git@github.com:dirk-thomas/vcstool.git
    version: 0.2.11
    patch: patches://vcstool/000_add_patch_functionality.patch

Sometimes maintaining a patch is easier than a fork.

dirk-thomas commented 4 years ago

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:// 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 commented 4 years ago

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:// 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 commented 4 years ago

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 commented 2 years ago

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

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.
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.
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
Patches should support a hash, so you are safe to store them remotely but still get a proper failure if it changes.
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: git@github.com: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 commented 2 months ago

Are there any advances here?

dirk-thomas / vcstool

Add patching functionality #156