packit icon indicating copy to clipboard operation
packit copied to clipboard

propose-downstream: Avoid creating divergant git branches

Open gotmax23 opened this issue 3 years ago • 23 comments

Whenever possible, I like to keep my distgit branches merged. It eases maintainability and testing, and ensures that the package isn't too different between Fedora releases. I make exceptions when there are different upstream major versions or when EPEL packaging quirks would require too many conditionals.

packit propose-downstream breaks this, because it creates separate PRs with separate commits for each release, even when the branches are merged. This should be made configurable to allow using the same commit for multiple branches. I believe Pagure allows submitting PRs from the same fork branch to multiple source branches, so with this approach, packit wouldn't always need to create separate branches.

Packit could also determine when the distgit branches are perfectly merged and just do this automatically, but that can be a future improvement.

gotmax23 avatar Sep 10 '22 03:09 gotmax23

Hi @gotmax23!

That's a good suggestion.

For now, you can let Packit open a single pull request and handle the other branches yourself. But I cannot find a nice way to open a second pull-request from a Packit's fork. Maybe using a remote pull-request functionality -- e.g.: https://src.fedoraproject.org/rpms/YOUR_PACKAGE_NAME/diff/remote You can also merge the first pull-request and open other pull-requests from the merged branch.

Regarding the implementation:

  • [ ] We need to check that we don't do anything branch-specific during the process so we are able to reuse the prepared content. (Both via CLI and service.)
  • [ ] Reporting and database manipulation need to be updated.

lachmanfrantisek avatar Sep 12 '22 07:09 lachmanfrantisek

But I cannot find a nice way to open a second pull-request from a Packit's fork.

The user interface doesn't show the option, but it's possible.

See https://src.fedoraproject.org/rpms/packit/pull-request/372 and https://src.fedoraproject.org/rpms/packit/pull-request/373.

I created one with https://src.fedoraproject.org/fork/gotmax23/rpms/packit/diff/rawhide..1724_test and another with https://src.fedoraproject.org/fork/gotmax23/rpms/packit/diff/f37..1724_test

gotmax23 avatar Sep 12 '22 17:09 gotmax23

Sorry, I meant the Packit's fork when I am not a Packit user. Similarly, I am unable to create a new pull-request using your fork: Screenshot from 2022-09-12 19-45-34

lachmanfrantisek avatar Sep 12 '22 17:09 lachmanfrantisek

This issue has been marked as stale because it hasn't seen any activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are never marked as stale.)

stale[bot] avatar Nov 12 '22 19:11 stale[bot]

/me silences the user hostile stale issue bot

gotmax23 avatar Nov 12 '22 20:11 gotmax23

This would be a very nice improvement! We've kept tmt branches synced before enabling propose-downstream and it would be nice if we could return to that approach as it makes is easier to apply other changes (e.g. adjusting test config).

psss avatar Jun 09 '23 10:06 psss

But I cannot find a nice way to open a second pull-request from a Packit's fork.

Maybe it's not possible in the UI, but could it be achieved in the api backend. I did make new PRs on my own fork from the same branch, but a simple workaround is to make a new branch from the original one (should be the case to allow branch specific changes) amd then make a PR against that

I think the workflow should be similar to the copr_build and how it sends all builds at once and then splits up its monitoring:

  • git pull the latest downstream
  • go from high to low on the targets least (ordered from rawhide down) and check which branches are not divergent
  • on the highest branch in each group
    • create a PR branch from downstream
    • sync the files and create a commit
  • on all other branches in the group
    • wait for the commit to finish
    • create a PR branch from the leader branch of the group
  • wait for all branches to finish and submit PRs together. This makes it easier to retry and not diverge the commits

Edge-case: Only rawhide PR has been merged and all other branches are currently divergent. This expands on step 2 above:

  • when checking each branch to create a group, if the branch is not the highest, try-catch to git merge --ff-only {leader-branch}
  • if git merge --ff-only fails, create a new group and set current branch as leader and continue with lower branch
  • when all groups are made, continue with the logic above

(Also impact is rather medium or more because it ensures we follow fedora packaging guidelines)

LecrisUT avatar Jun 17 '23 08:06 LecrisUT

The issue with the implementation is that for now, the runs are quite separated and you can't easily report/rerun per group. Otherwise, it's easy. In the time of opening a PR, we can just run the command to create a PR multiple times with different attributes.

The easest (but a bit hackish) to implement might be to let user configure the set of branches that shares the same run. (This will allow us to know that sooner than from the git.)

lachmanfrantisek avatar Jul 06 '23 07:07 lachmanfrantisek

Do a research in general and also on using Gitlab (for CentOs Stream and probably future integration)

One example could be: do a PR just for rawhide and then use other options/tools to automate merging on other branches. Create a documentation for helping do manually the merge on other branches and add it to the PR instructions.

majamassarini avatar Oct 26 '23 08:10 majamassarini

This would be excellent, we recently adopted packit for one of our repos and found that we could no longer fast forward, while this is workable, the result is we will no longer able to compare our branches to see any fixes at a glance. In the past we would compare branches to quickly glance and easily find what was different from f39 or f37, but now that will be a little bit more effort. We certainly want to keep using packit for the reduction in day to day steps but, we would certainly prefer to maintain the fastforwardable nature if possible.

prestist avatar Jan 22 '24 15:01 prestist

Oh, I think there is a simple tool to manage this fbrnch, particularly fbrnch merge. This can take care of figuring out how to merge from the newer branch. @juhp are there other features that could help here?

LecrisUT avatar Jul 05 '24 19:07 LecrisUT

For reference if it helps, this is the code fbrnch is using for gitMergeable

juhp avatar Jul 08 '24 09:07 juhp

Notes from refinement

  • Be careful about potential rebuilds
  • Introduce new config option for fast-forwarding via PR to co-rawhide branches (on merge)
    • potential implementation: after merge to rawhide, check out the rawhide, push to Packit's fork, and open PRs (Pagure should fast-forward)

mfocko avatar Jul 18 '24 11:07 mfocko

  • potential implementation: after merge to rawhide, check out the rawhide, push to Packit's fork, and open PRs (Pagure should fast-forward)

Thinking about this, we can even do a similar thing:

  • When configuring, allow one branch to be "the source` (~rawhide) and multiple other branches to "follow".
    • Either provide another config option (something like create_pull_request_to, follow_with_branches, merged_into, use_for_branches,... better names welcome).
    • Or, support mapping between branches. Or any other syntactical sugar in config.
  • Do the sync release as usual but check the new config once we are done with PR submitting (to rawhide) and open extra PRs to other "follow" branches from the same commit.
  • This will not break the workflow, service, database, reporting,... (The only change might be to support a list of PRs in db/dashboard instead of a single one.)

Example config:

- job: pull_from_upstream
  trigger: release
  dist_git_branches:
   - fedora-rawhide
  create_pull_request_to:
   - fedora-all

lachmanfrantisek avatar Jul 18 '24 11:07 lachmanfrantisek

Example config:

- job: pull_from_upstream
  trigger: release
  dist_git_branches:
   - fedora-rawhide
  create_pull_request_to:
   - fedora-all

I like this, but it allows for "impossible" combinations:

- job: pull_from_upstream
  trigger: release
  dist_git_branches:
    - f40
    - epel9
  create_pull_request_to:
    - fedora-rawhide
    - epel8

What about something similar to what we support with targets in test jobs:

- job: pull_from_upstream
  trigger: release
  dist_git_branches:
    fedora-rawhide:
      open_pull_requests_for: [fedora-all]
    epel9: {}

nforro avatar Jul 18 '24 11:07 nforro

Sounds good. One note on fbrnch is that it does sequentially rawhide -> f40 -> f39 (epel as well, but didn't check where it branches off), and let's say f40 has diverged, f39 will try to fast-forward to f40 instead of rawhide. Having a similar approach would be useful.

Let me understand the workflow a bit:

  • First create the PRs to dist_git_branches and wait for PRs to merge
  • After all PRs are merged, create the remaining PRs from create_pull_request_to

My thought is that in this interface the latter part could be split as a separate job like sync_branches (and be independent of pull_from_upstream/propose_to_downstream).

Another interface could be:

  • Create all PRs to dist_git_branches in sequence
  • Expose an interface /packit rebase to bring the PR branches up-to-date to the PR's branch

LecrisUT avatar Jul 18 '24 11:07 LecrisUT