I think this check should be around the repository being hosted (isHosted()), not isWorkingCopyBare()? Don't we hit the same issue with bare, hosted working copies (origin pointing at file:// instead of the true remote origin)?

My setup is all about being hosted at github, so isHosted() is definitely not enough.

I'm trying to set up a non-bare hosted repo to see what that can do...

If the check is isHosted() and the logic looks like this:

`git clone file:///...`;
if (!$repository->isHosted()) {
  `git remote set-url origin ...`;
}

...that should work fine for GitHub hosting? Specifically, the clone will have a file:/// origin, and then the set-url will point it back to GitHub instead?

It will work, but will will make the next every fetch be directly from github, instead of just getting it from the local one, even if the local one is good.

Ah. What's the actual issue this is hitting, then?

That is, why do we ever need to do this? What breaks if we don't?

The original problem is that for non-bare, non-hosted repos (Which is most of what I have), the cloned workspace will be set to some old state, and the push would fail on "not-ff".

The "fix" would make it fetch from github once the initial clone is done, so it would at least be up to date; I'd prefer to have it somehow track the origin-of-origin, but I'm not sure git has that.

regarding non-bare, hosted, I found this: https://secure.phabricator.com/diffusion/P/browse/master/src/applications/differential/landing/DifferentialLandingToHostedGit.php;476b27d9c8b2b38a05ce950172d8b09a07478cf1$122-124 although I'm not sure why exactly we did that; Probably we guessed it wouldn't work?

Probably because of this error:

refusing to update checked out branch: refs/heads/master

So for the narrow context of clone-for-landing, hosted implies bare.

The reason this patch works is that while the local file:// uri is not the "right" uri, it is kept up-to-date, so when pushing to the real source, we already have the right HEAD (Barring the update delay).