Bug 256057

Summary: ports/ declines multi-branch pushes without ability to override via --push-options
Product: Services Reporter: Jan Beich <jbeich>
Component: Core InfrastructureAssignee: Mathieu Arnold <mat>
Status: Closed Works As Intended    
Severity: Affects Only Me CC: lwhsu, portmgr
Priority: ---    
Version: unspecified   
Hardware: Any   
OS: Any   

Description Jan Beich freebsd_committer 2021-05-21 15:56:41 UTC
I often need to push both to main and a quarterly branch. To avoid desync with hashes I use --atomic but this no longer works.

If this is intentional to avoid foot-shooting, please, add an escape hatch.

$ cd /path/to/ports/latest
$ git log --no-abbrev --pretty=fuller @{u}..
commit 1307312bf475a3457d54c8ea2b3ff871519db22a
Author:     Jan Beich <jbeich@FreeBSD.org>
AuthorDate: Fri May 21 10:43:36 2021 +0000
Commit:     Jan Beich <jbeich@FreeBSD.org>
CommitDate: Fri May 21 15:53:55 2021 +0000

    benchmarks/glmark2: backport --fullscreen fix for WAYLAND option
    
    $ glmark2-wayland --fullscreen
    Error: eglCreateWindowSurface failed with error: 0x300b
    Error: eglCreateWindowSurface failed with error: 0x300b
    Error: CanvasGeneric: Invalid EGL state
    Error: main: Could not initialize canvas

$ cd /path/to/ports/quarterly
$ git cherry-pick -x main
$ git log --no-abbrev --pretty=fuller @{u}..
commit 7023f3d11c3c36d5dd0d08a697dff45534f09541 (HEAD -> 2021Q2)
Author:     Jan Beich <jbeich@FreeBSD.org>
AuthorDate: Fri May 21 10:43:36 2021 +0000
Commit:     Jan Beich <jbeich@FreeBSD.org>
CommitDate: Fri May 21 15:54:30 2021 +0000

    benchmarks/glmark2: backport --fullscreen fix for WAYLAND option
    
    $ glmark2-wayland --fullscreen
    Error: eglCreateWindowSurface failed with error: 0x300b
    Error: eglCreateWindowSurface failed with error: 0x300b
    Error: CanvasGeneric: Invalid EGL state
    Error: main: Could not initialize canvas
    
    (cherry picked from commit 1307312bf475a3457d54c8ea2b3ff871519db22a)

$ git branch -r --contains 1307312bf475a3457d54c8ea2b3ff871519db22a
$ git branch --contains 1307312bf475a3457d54c8ea2b3ff871519db22a
* main

$ git push --atomic freebsd main 2021Q2
Enumerating objects: 33, done.
Counting objects: 100% (29/29), done.
Delta compression using up to 8 threads
Compressing objects: 100% (15/15), done.
Writing objects: 100% (15/15), 1.98 KiB | 1.98 MiB/s, done.
Total 15 (delta 10), reused 0 (delta 0), pack-reused 0
remote:
remote: ================================================================
remote: jbeich, you are pushing a commit to 2021Q2 which is
remote: a cherry-pick, but the commit hash is not part of the main branch.
remote:
remote: This is most probably because you did the cherry-pick before pushing
remote: the changes to main and you ended up having to rebase your work,
remote: which changed the commit hashes.
remote:
remote: If you are trying to push both main and 2021Q2 at the
remote: same time, you will have to push main first, and then 2021Q2.
remote: ================================================================
remote:
To gitrepo.FreeBSD.org:ports.git
 ! [remote rejected]           2021Q2 -> 2021Q2 (pre-receive hook declined)
 ! [remote rejected]           main -> main (pre-receive hook declined)
error: failed to push some refs to 'gitrepo.FreeBSD.org:ports.git'
Comment 1 Li-Wen Hsu freebsd_committer 2021-05-21 16:13:13 UTC
I think this is due to the newly deployed ports hook on the server side.  It checks if the comment of the cherry-pick’d commit in the quarterly branch has the commit hash in the main branch.  I guess because of both commits are in the staging (receiving) area but hook only finds the hash in the final repository causes this.  Maybe also checking the staging area can be a solution.
Comment 2 Mathieu Arnold freebsd_committer 2021-05-24 09:18:09 UTC
Checking in the staging area is more complicated, it would make the hook way much complicated.
The workflow is:

1) push to the main branch
2) cherry-pick to quarterly
3) push to quarterly

Using --atomic is nice, but if the push to main fails (which it will more often than not) you will have to re-do the cherry-picks anyway.
Comment 3 Jan Beich freebsd_committer 2021-05-24 21:37:20 UTC
(In reply to Mathieu Arnold from comment #2)
Please, provide --push-options=async-cherry or similar. When pushing a security fix the last thing I want to remember is a silly dance around push hooks. No need to replicate how awful Subversion was.
Comment 4 Jan Beich freebsd_committer 2021-05-24 21:57:52 UTC
For example, when committer != author push (pre-receive) hook suggests how to override:

  $ git push freebsd
  Enumerating objects: 5, done.
  Counting objects: 100% (5/5), done.
  Delta compression using up to 8 threads
  Compressing objects: 100% (3/3), done.
  Writing objects: 100% (3/3), 296 bytes | 296.00 KiB/s, done.
  Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
  remote:
  remote: ================================================================
  remote: jbeich, you are pushing a commit which author and committer are different:
  remote:
  remote: author: foo <foo@bar>
  remote: commit: 987e6796c6daeee9c8516fa623dcd93896bb1421
  remote: subject: test
  remote:
  remote: Please check the author name and email are correct and then use:
  remote:         git push --push-option=confirm-author
  remote: ================================================================
  To gitrepo-dev.FreeBSD.org:ports.git
   ! [remote rejected]           main -> main (pre-receive hook declined)
  error: failed to push some refs to 'gitrepo-dev.FreeBSD.org:ports.git'

A similar override can be provided for cherry-picks. For consistency it should probably start with "confirm-" prefix. Now, the option name is a bit of bikeshed: confirm-atomic, confirm-atomic-branches, confirm-multi-branch, confirm-multi-cherry, ... confirm-footshooting.
Comment 5 Li-Wen Hsu freebsd_committer 2021-05-25 00:47:07 UTC
We will discuss how to do so, but the complexity will add some more difficulty to it.
Comment 6 Mathieu Arnold freebsd_committer 2021-05-25 08:26:57 UTC
So, to provide more context, git *-receive hooks gets one `old-hash new-hash ref` at a time on stdin, and they process each ref as they come.  If you push multiple refs, there is no guarantee that refs/heads/main would arrive before refs/heads/xxxxQy.

To keep things simple and readable, the hooks are stateless.  To do what you ask, hooks would need to become stateful, and some, but not all, checks, would have to be deferred to after all refs have been processed, which would defeat the KISS principle into witch they were written.

Note that you still can push everything at once, instead of doing this:

git push --atomic freebsd main 2021Q2

do this:

git push freebsd main; git push freebsd 2021Q2
Comment 7 Jan Beich freebsd_committer 2021-06-05 20:06:52 UTC
(In reply to Mathieu Arnold from comment #6)
> no guarantee that refs/heads/main would arrive before refs/heads/xxxxQy.

How does this affect --push-option?

> git push freebsd main; git push freebsd 2021Q2

Duplication like "git push freebsd" makes it error-prone. I've almost pushed all changes from my staging branch by accident.
Comment 8 Mathieu Arnold freebsd_committer 2021-06-07 06:57:20 UTC
(In reply to Jan Beich from comment #7)
> (In reply to Mathieu Arnold from comment #6)
> > no guarantee that refs/heads/main would arrive before refs/heads/xxxxQy.

> How does this affect --push-option?

Well, if we get the quarterly branch before main, we are going to check that the commits exists before we get the commits.

> > git push freebsd main; git push freebsd 2021Q2

> Duplication like "git push freebsd" makes it error-prone. I've almost pushed all changes from my staging branch by accident.

If your shell does not support history, or you don't trust it, maybe create either a shell or a git alias to do both push at once.