fetch: delay user information post committing of transaction
In Git 2.50 and earlier, we would display failure codes and error
message as part of the status display:
$ git fetch . v1.0.0:refs/heads/foo
error: cannot update ref 'refs/heads/foo': trying to write non-commit object
f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'
From .
! [new tag] v1.0.0 -> foo (unable to update local ref)
With the addition of batched updates, this information is no longer
shown to the user:
$ git fetch . v1.0.0:refs/heads/foo
From .
* [new tag] v1.0.0 -> foo
error: cannot update ref 'refs/heads/foo': trying to write non-commit object
f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo'
Since reference updates are batched and processed together at the end,
information around the outcome is not available during individual
reference parsing.
To overcome this, collate and delay the output to the end. Introduce
`ref_update_display_info` which will hold individual update's
information and also whether the update failed or succeeded. This
finally allows us to iterate over all such updates and print them to the
user. While this brings back the functionality, it does change the order
of the output. Modify the tests to reflect this.
Using an strmap does add some overhead to 'git-fetch(1)', but from
benchmarking this seems to be not too bad:
Benchmark 1: fetch: many refs (refformat = files, refcount = 1000, revision = master)
Time (mean ± σ): 51.9 ms ± 2.5 ms [User: 15.6 ms, System: 36.9 ms]
Range (min … max): 47.4 ms … 58.3 ms 41 runs
Benchmark 2: fetch: many refs (refformat = files, refcount = 1000, revision = HEAD)
Time (mean ± σ): 53.0 ms ± 1.8 ms [User: 17.6 ms, System: 36.0 ms]
Range (min … max): 49.4 ms … 57.6 ms 40 runs
Summary
fetch: many refs (refformat = files, refcount = 1000, revision = master) ran
1.02 ± 0.06 times faster than fetch: many refs (refformat = files, refcount = 1000, revision = HEAD)
Another approach would be to move the status printing logic to be
handled post the transaction being committed. That however would require
adding an iterator to the ref transaction that tracks both the outcome
(success/failure) and the original refspec information for each update,
which is more involved infrastructure work compared to the strmap
approach here.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>