cweagans / composer-patches

Simple patches plugin for Composer
https://www.cweagans.net/project/composer-patches
BSD 3-Clause "New" or "Revised" License
1.53k stars 240 forks source link

Apply patches on-the-fly via forked repository with a commit checkpoint #575

Closed emircanerkul closed 6 months ago

emircanerkul commented 6 months ago

Verification

Is your feature request related to a problem?

No.

Describe your proposed solution(s)

Scenario;

I'm using

https://git.drupalcode.org/project/tfa/-/tree/2.x

And I have a forked issue repository for that project

https://git.drupalcode.org/issue/tfa-3386547/-/tree/3386547-remove-sms-from

I can create PR and diff files manually and use them like that. Mostly in the Drupal ecosystem these PR patches are uploaded to issue to get a static patch hosted in drupal.org and used like that. But to eliminate these steps I'm suggesting this proposal.

Structure can be something like that.

"extra": {
        "patches": {
            "drupal/tfa": [
                {
                    "description": "This is the description of the patch",
                    "repo": "https://git.drupalcode.org/issue/tfa-3386547/-/tree/3386547-remove-sms-from",
                    "branch": "3386547-remove-sms-from",
                    "sha": "f866513e2c02c1f7e028445c0908085d8d1d75ad",
                    "extra": {},
                }
            ]
        }
}

Describe alternatives

No response

Additional context

No response

github-actions[bot] commented 6 months ago

👋 Thanks for the idea! Please remember that this is an open source project - feature requests may or may not be implemented, and if they are, the timeline is unknown. If you need a guaranteed implementation or timeline, sponsorships are welcome!

emircanerkul commented 6 months ago

More example: https://www.drupal.org/project/group/issues/3378653

cweagans commented 6 months ago

This is a neat idea! I probably won't build this into the core plugin, but Composer Patches is now extensible and this is something that you can definitely do in a third party plugin. I think you could do this just by implementing a Downloader (see https://docs.cweagans.net/composer-patches/getting-started/terminology/ and https://docs.cweagans.net/composer-patches/api/capabilities/ ). You may also need to subscribe to PluginEvents::POST_DISCOVER_DOWNLOADERS to re-order the Downloaders so that your custom downloader is executed first though.

As an alternative, all Gitlab merge requests can generate a patch out of the box. For the tfa issue you mentioned above, https://git.drupalcode.org/project/tfa/-/merge_requests/37 is the merge request, and https://git.drupalcode.org/project/tfa/-/merge_requests/37.patch is a URL where a patch can be downloaded from.

In past versions of this plugin, that would have been a pretty dangerous thing to do since the contents of that patch could change at any time. In version 2.x of Composer Patches, a sha256 of your patch is stored alongside the rest of the patch information, so it's significantly less risk now. It's still a little fragile (since a change to that issue fork could break your CI), but it would work.

If you want to be really specific about the changes that you're making to your project, you can also get patches from specific commit hashes on Gitlab (e.g. https://git.drupalcode.org/project/tfa/-/commit/d10a8107998372b9f4e9f46e22506c88a81d2a47.patch). You'd have to apply each of the patches from the MR in order, but then you could be confident that you're getting exactly the right changes in your project.

Now that I think about it, you could probably use the Gitlab API to automate that. E.g: https://git.drupalcode.org/api/v4/projects/57554/merge_requests/37/commits will give you JSON describing the commits on a MR. You could write a Resolver plugin that will take a URL to a Gitlab merge request, find the numeric ID of the project (I looked in the network pane of my browser inspector when loading the Gitlab project page to find that 57554 = tfa, but there's probably a better way to do that), build that URL and get the commits, and then only apply the patches that happened before a particular time (that timestamp could be encoded in the extra section of a patch definition). If you wanted to, that could be a more generic plugin that does Github PRs too.

If you end up writing a plugin that does any of this, I'm happy to help when/if you get stuck + I'd be happy to list your plugin(s) on a docs page that points people to third party plugins!

Closing this for now, but feel free to reply if you need any additional details!

emircanerkul commented 6 months ago

@cweagans thank you for the detailed answer.

Yes thanks to 2.0 with patch sha256 file validation now using the PR patch URL directly is safe, but anyone can push to that PR, which will throw an error in the next composer package upgrade. (This can be good actually, notifying there is new code probably would be helpful)

Using commit hash patches yes that looks like a good solution/workaround also I think it is the most flexible one. Regarding security, I think no one can delete or force update commits/tree in issue repos.

Regarding implementation my idea was;

-> clone fork repo, checkout commit/branch, create diff, apply diff

Using gitlab api flow would be

-> Fetch api, (assuming commits are hierarchical) loop commits starting from specified commit -> get patches via API -> apply patches.

Using GitLab API flow looks easier to implement if 2.0 has some preprocess hooks. (but have to make sure created PR is undeletable) Looks like we have PatchEvents::PRE_PATCH_APPLY

"extra": {
        "patches": {
            "drupal/tfa": {
                    "Sample MR": "https://git.drupalcode.org/api/v4/projects/57554/merge_requests/37/commits#d10a8107998372b9f4e9f46e22506c88a81d2a47,
                }
            }
        }
}

Will turned into

"extra": {
        "patches": {
            "drupal/tfa": {
                    "Remove the rest of the SMS settings and test the update hook": "https://git.drupalcode.org/project/tfa/-/commit/f866513e2c02c1f7e028445c0908085d8d1d75ad.diff",
                    "Remove the sms setting from user data": "https://git.drupalcode.org/project/tfa/-/commit/d10a8107998372b9f4e9f46e22506c88a81d2a47.diff",
                }
        }
}

But maybe no need for all of these; https://docs.gitlab.com/ee/api/repositories.html#compare-branches-tags-or-commits

cweagans commented 6 months ago

You can write a resolver that hooks in directly to the patch process. No need to use the event. You'd be able to point at the merge request directly and then your resolver could go track down the right commits :)