Page MenuHomePhabricator

If the base commit for `arc patch` does not exist locally, try to fetch it
ClosedPublic

Authored by alexmv on May 18 2017, 4:33 PM.

Details

Summary

If the commit does not exist locally, aborting still leaves
the user checked out on the branch. In nearly all cases, all that is
necessary is a fetch -- but the branch must also be cleaned up. This
leads to the pattern of:

arc patch D12345
[...base commit does not exist...]
^C
git checkout master
git branch -D arcpatch-D12345
git fetch
arc patch D12345

Solve this common problem by simply trying to fetch once if the commit does not
exist locally.

Test Plan

Ran arc patch on a recent diff.

Diff Detail

Repository
rARC Arcanist
Branch
up-fetch-first (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 17081
Build 22843: Run Core Tests
Build 22842: arc lint + arc unit

Event Timeline

I can't really come up with any good reason not to bring this upstream. Two weak reasons are:

  • Users may not want to fetch --all for some reason.

You might want to use execPassthru() or similar (without --quiet) to show users status and errors.

This revision is now accepted and ready to land.May 18 2017, 4:52 PM

Er, my other reason was:

  • Weird --tags / --no-tags expectations?

But we'll hit both of these issues, if they do exist, on the pathway through T11091, and it may help to know about them earlier.

My theory for not execPassthru was if it fails, those failures are likely irrelevant to the matter at hand (maybe they have an old, invalid, remote still hanging out), and spewing them to the console is just going to cause confusion.

Oh, I didn't catch that you're completely ignoring the error. I suspect we don't want to do that in the long run (it leads to us getting bug reports like "arc patch said it fetched, but the commit is definitely in the remote! your software is BAD and has made my life AWFUL"), but this seems reasonable for now since I think T11091 is somewhere on the horizon.

See, e.g., T12721 for a somewhat similar case where silent local misconfiguration led to a support request.

  • Users may not want to fetch --all for some reason.

I think I would have some reasons not to want to fetch --all, but I'm probably not a typical user.
None-the-less, is there maybe a way to fetch the commits without changing the refs in .git/refs/remotes/?

None-the-less, is there maybe a way to fetch the commits without changing the refs in .git/refs/remotes/?

We could do something like git fetch <remote> '+refs/*:refs/murky.chasm/*' -- I'm not sure how that interacts with --all. I'm not overjoyed about the prospect of doing this.

Also I think it probably becomes infinitely bad very quickly if you have the working copy configured as a remote.