Skip to content

gitserver: Allow updating ref with new commits

Administrator requested to merge git/repeatable-commit-from-patch into master

Created by: mrnugget

Before this change it wasn't possible to use createCommitFromPatch twice with the same TargetRef and different patches. When trying to push the new commit to the codehost (tested with GitHub) it would report that everything's up to date.

The reason was that we created ambigious refs. Example: with a TargetRef of A the call to git update-ref -- A <commitsha> created a ref called A. But when pushing to the codehost with git push A:refs/heads/A we also implicitly created a second ref, called refs/heads/A, making A as a ref name ambigious.

When the endpoint was called again with a different diff a new commit was created and the A ref was updated, but since the name was ambigious the refs/heads/A ref was used, which was correctly reported as being up to date with the remote host.

This could be confirmed with git show-ref and git show:

  $ git show-ref sourcegraph/thorstens-test-ref-1
  f9cfe6d7e04f5127c9d3d763dda7c1bdbcc2ec1b refs/heads/sourcegraph/thorstens-test-ref-1

  $ git show sourcegraph/thorstens-test-ref-1
  warning: refname 'sourcegraph/thorstens-test-ref-1' is ambiguous.
  commit c8791c8a194c99dc312371e520e19908db7bc5e8
  Author: Sourcegraph Bot <[email protected]>
  Date:   Tue Jan 7 13:10:34 2020 +0000
      foobar
  diff --git a/file1.txt b/file1.txt
  index 1e4ff02..83aa92b 100644
  --- a/file1.txt
  +++ b/file1.txt
  @@ -1 +1 @@
  -this is file 1
  +FOOOOOBAR4444

The commands report a different commit SHA and git show prints a warning.

What this change here does is

  1. avoid ambigous ref name by ensuring that the ref has a refs/heads/ prefix when Push: true
  2. pushing cmtHash (instead of the ref) to the full ref name on the codehost before updating the local ref
  3. updating the local full ref name to the now-pushed commit

That allows us to now use createCommitFromPatch with the same TargetRef to push changed diffs to the codehost.

Merge request reports

Loading