Ensure revision when calculating changeset state
Created by: mrnugget
This fixes #14921 (closed) by making sure that the HeadRev is fetched on gitserver before attempting to get the git diff
and calculating the diff stat.
There were other options to do this:
- Adding an
git.EnsureRevision
call to the reconciler when importing changesets - Adding
git.EnsureRevision
tocomputeDiffStat
Why not (1)? It adds a special case to one method, when in fact we should always ensure that the head rev is fetched on gitserver
before calculating diff stats.
But adding two git.EnsureRevision
to (2) would've duplicated the already existing call to git.EnsureRevision
that we did in computeRev
.
The problem was that we only called out to EnsureRevision
in computeRev
when we didn't have the OID.
When importing changesets, though, we do have the OID, we just need to ensure it exists on gitserver
.
So I added those calls to computeRev
, which, I think, makes sense there, since it means we don't work with possibly outdated data, since we always ensure the newest commit is used.
I also had to fix computeGitserverRepo
which was simply using the external URL as the clone URL when constructing the gitserver.Repo
. backend.GitRepo
does the correct thing.
Trivia
This is the test script I used to push a new commit and cause the reconciler
to import the changeset, failing on
$ psql --dbname sourcegraph -c 'DELETE FROM changeset_events; DELETE FROM CHANGESETS; DELETE FROM campaigns;' && newCommit 3 && src campaign apply -f ~/tmp/wonky-diffstat.campaign.yaml -n mrnugget
Requirements:
$ newCommit() { echo "foo" >> foo${1}.txt && git add . && gc -m "foo${2}" && up }
$ cd automation-testing && git checkout thorsten/detach-reattach
And the campaign is this one:
name: debugging-1
description: hopefully fixing a bug
importChangesets:
- repository: github.com/sourcegraph/automation-testing
# https://github.com/sourcegraph/automation-testing/pull/357
externalIDs: [357]