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 default_mainline input #372

Closed
wants to merge 3 commits into from
Closed

Add default_mainline input #372

wants to merge 3 commits into from

Conversation

korthout
Copy link
Owner

@korthout korthout commented Aug 6, 2023

Adds a new default_mainline input, which can be used to support cherry-picking merge commits.

Note that the only usable values at this time are:

  • default_mainline: '' (disabled)
  • default_mainline: 1 (use parent 1 as mainline)

This does not yet provide support for skipping merge commits, nor for using parent numbers 2+ as the default mainline.

closes #341

tdonohue added a commit to tdonohue/dspace-angular that referenced this pull request Aug 8, 2023
tdonohue added a commit to DSpace/dspace-angular that referenced this pull request Aug 8, 2023
@tdonohue
Copy link

tdonohue commented Aug 8, 2023

@korthout : Thanks for this quick work! I gave this a test by enabling this early release here https://github.com/DSpace/dspace-angular/pull/2424/files

I then tried merging a PR with a merge commit: https://github.com/DSpace/dspace-angular/pull/2408/commits

But, unfortunately, it still failed to be ported to the other branch: DSpace/dspace-angular#2408 (comment)

In the logs of the Action, I see the --mainline=1 flag was used. Here's the relevant logs:

Retrieve pull request data for #2408
Check whether pull request 2408 is merged
Determining target branches...
...
Found target branches in labels: dspace-7_x
Found target branches in `target_branches` input: 
Exclude pull request's headref from target branches: ProcessDetailComponent-test-improvemen
Determined target branches: dspace-7_x
Fetching all the commits from the pull request: 3
git fetch --depth=3 origin refs/pull/2408/head
From https://github.com/DSpace/dspace-angular
 * branch            refs/pull/2408/head -> FETCH_HEAD
Determining first and last commit shas, so we can cherry-pick the commit range
Retrieving the commits from pull request 2408
Found commits: 03e1f677b610498b6dbb54b1f3e6310bef5a[14](https://github.com/DSpace/dspace-angular/actions/runs/5801382709/job/15725533319#step:3:15)07,63e6e79750c71adaef0cc14671a8eb555e05d94e
...
Backporting to target branch 'dspace-7_x...'
git fetch --depth=1 origin dspace-7_x
From https://github.com/DSpace/dspace-angular
 * branch            dspace-7_x -> FETCH_HEAD
 * [new branch]      dspace-7_x -> origin/dspace-7_x
Start backport to backport-[24](https://github.com/DSpace/dspace-angular/actions/runs/5801382709/job/15725533319#step:3:25)08-to-dspace-7_x
git switch -c backport-2408-to-dspace-7_x origin/dspace-7_x
Switched to a new branch 'backport-2408-to-dspace-7_x'
git cherry-pick -x --mainline=1 03e1f677b610498b6dbb54b1f3e6[31](https://github.com/DSpace/dspace-angular/actions/runs/5801382709/job/15725533319#step:3:32)0bef5a1[40](https://github.com/DSpace/dspace-angular/actions/runs/5801382709/job/15725533319#step:3:41)7 63e6e79750c71adaef0cc1[46](https://github.com/DSpace/dspace-angular/actions/runs/5801382709/job/15725533319#step:3:47)71a8eb555e05d94e
error: could not apply 63e6e79... Merge branch 'main' into ProcessDetailComponent-test-improvemen
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
git cherry-pick --abort
Backport failed for `dspace-7_x`, because it was unable to cherry-pick the commit(s).

It appears (to me) that the main issue on our end is that --mainline=1 isn't exactly what we want.

In this scenario, the PR in question was for our main (active development) branch, and we wanted to auto-backport it to our dspace-7_x (maintenance) branch.

  • The first commit cherry-picked successfully.
  • But, the merge commit (second commit) failed with a merge conflict.

When I attempted both cherry picks manually using --mainline=1, I see the same merge conflict with the second commit. Unfortunately, that conflict appears to imply that it is attempting to cherry-pick unrelated changes from main to this new PR against our dspace-7_x branch.

Unfortunately, this isn't the behavior we need on our end. We need a way to essentially --skip the merge commit in this scenario (as it literally adds no new/unique code to the PR, it's just a "resync" of the PR branch with main).

That said, this solution might still work for others who require --mainline=1. It just doesn't fit with our scenario.

tdonohue added a commit to DSpace/dspace-angular that referenced this pull request Aug 8, 2023
tdonohue added a commit to DSpace/dspace-angular that referenced this pull request Aug 8, 2023
…t-action

Revert "Enable early release of backport-action to test korthout/backport-action#372"
@korthout
Copy link
Owner Author

korthout commented Aug 9, 2023

Thanks @tdonohue for that comprehensive feedback. It gives invaluable insights and I greatly appreciate it.

Unfortunately, this isn't the behavior we need on our end.

I guess this holds for all use cases where a merge commit updates the pull request from the base. I see no reason why any user might want to take some of the merged changes from the base along in the backport.

I'll close this pull request so I don't have to support an unused feature indefinitely. If any user wants this behavior, please reach out and let me know. I'm happy to support it if there is a need for it.

We need a way to essentially --skip the merge commit in this scenario

Yup, this is likely more useful to most. Initially, I wrote that this might be a fit for default_mainline: 0 but I no longer think that this fits. Instead, I'd rather introduce a new merge-commit input option with fail and skip values. I'll continue this in a separate issue.

@korthout korthout closed this Aug 9, 2023
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.

Feature request: allow specifying mainline
2 participants