FR-223: Add option for changing the target repository - #224
Conversation
…iegroup#223) Adds a --tb-repo/tb-repo flag, mirroring the existing --bp-repo, so the backport pull request can be opened against a repository other than the one targeted by the original PR. This unblocks workflows like GraalVM's community LTS repos, where backports of oracle/graal PRs need to land in a separate downstream repo (e.g. graalvm/graalvm-community-jdk25u) rather than in a fork of the same repository. Unlike --bp-repo, which only changes where the branch is pushed, --tb-repo changes where the repo is cloned from and where the PR is ultimately opened. Because the original PR's commits generally don't exist in the history of an unrelated target repository, the runner now adds an "upstream" remote back to the original PR's repo and fetches the PR ref from there before cherry-picking, rather than assuming origin. Also made addRemote idempotent (update the URL if the remote already exists) since the working folder is reused across multiple target branches, which would otherwise make --tb-repo (and pre-existing --bp-repo) usage with multiple target branches fail on the second iteration.
…artifact Correct the README note for the 'backport to a different target repository' mode (kiegroup#223): the required permissions differ depending on whether --bp-repo is also used. With --tb-repo alone the branch is pushed straight to the target repo (needs push access there); combined with --bp-repo the branch goes to the fork and only PR-open ability is needed on the target repo. Also drop the unrelated allowScripts entry that had leaked into the bundled package.json inside dist/cli/index.js during an earlier rebuild.
Coverage report
Test suite run success228 tests passing in 19 suites. Report generated by 🧪jest coverage report action from 8bf1c82 |
lampajr
left a comment
lampajr
left a comment
There was a problem hiding this comment.
Hi @synecdoche, first of all thanks a lot for opening the PR ❤️
Overall looks great to me, I just noticed one possible pitfall.
I tried using --tb-repo without the --no-squash and it fails to cherry-pick when backporting a merged PR:
node dist/cli/index.js -tb master --tb-repo zakkak/graalvm-community-jdk25u \
-pr https://github.com/oracle/graal/pull/13276 -a $(gh auth token) \
--bp-repo synecdoche/graalvm-community-jdk25u -f /tmp/test-2 -d
[INFO ] Fetching upstream pull/13276/head:pr/13276
[INFO ] Cherry picking efcf7228fe0616970cb399685553f57b3aa60160
[ERROR] fatal: remote error: upload-pack: not our ref efcf7228fe0616970cb399685553f57b3aa60160
fatal: bad object efcf7228fe0616970cb399685553f57b3aa60160
I believe this happens because in --tb-repo mode we clone the target repo and fetch only pull/<N>/head from the original repo (upstream). But for a merged PR the commit we cherry-pick is the merge/squash commit (merge_commit_sha), which lives on the original repo's base branch - it is not reachable from pull/<N>/head, so it never gets downloaded.
It gets worse because the clone is a partial clone (--filter=blob:none), which makes origin (the tb-repo) the promisor remote. When cherry-pick looks for the missing commit, git lazily asks origin for it - not upstream - and origin doesn't have it either, hence upload-pack: not our ref → bad object.
This means:
- works: --no-squash (individual commits are on pull/N/head) and open PRs (head sha is on pull/N/head)
- fails: the default merged/squashed case - the most common scenario
The current tests don't catch this because they mock fetch/cherryPick, so commit reachability is never exercised.
We could Instead of fetching pull//head, fetch the actual commit SHAs we're about to cherry-pick directly from upstream git fetch upstream <sha> for each commit in the PR. There might be other better solution I did not think of, I am open to suggestion here 😄
3 participants
Thank you for submitting this pull request
fixes #223
Description
Adds a new flag,
--tb-repo, which allows a different destination repository for the backport as described in issue 223How Has This Been Tested?
This was tested with the following command:
The output of that command was:
Checklist
Merge criteria:
First time here?
This project follows git conventional commits pattern, therefore the commits should have the following format:
Where the type must be one of
[build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test]How to prepare for a new release?
There is no need to manually update
package.jsonversion andCHANGELOG.mdinformation. This process has been automated in Prepare Release Github workflow.Therefore whenever enough changes are merged into the
mainbranch, one of the maintainers will trigger this workflow that will automatically updateversionandchangelogbased on the commits on the git tree.More details can be found in package release section of the README.