Re* git remote --mirror bug?

Joakim Tjernlund <joakim.tjernlund@transmode.se> writes:

This is very unfortunate. What's more unfortunate is that, adding insult to injury, "git remote" reimplemented in C gives even worse nonsense:

    $ git remote show git.git
    fatal: * refusing to create funny ref 'refs/stash' locally

This is with my test repository sitting next to my primary git.git repository with config entries like this:

    [remote "git.git"]
        url = ../git.git/
        fetch = refs/*:refs/*

The thing is that we obviously are _not_ attempting to create anything when we say "git remote show".

Now, it happens that "stash" is purely a local matter, and propagating it with fetch (even with mirroring) may not make much sense. Even if the mirroring is for backup purposes, its value is dubious (if you are backing up because you are afraid of disk failure, you should back up properly with disk back-up tools).

So in that sense, it could be argued that it is Ok not to propagate refs/stash and that was partly the reason why we create the "stash" ref directly underneath refs/ namespace. Everything else that is propagatable has at least two level refnames, e.g. heads/master, tags/v1.5.0, and the plumbing layer has a way to enforce this, and "git remote" uses it.

But that is not an excuse to fail "git fetch" (which is the underlying mechanism "git remote" uses).

I think this patch may work the issue around for recent enough git, but I have to say that this is an interim fix. With this, now you get a less nonsense output, but it still is nonsense nevertheless:

    $ git remote show git.git
    * remote git.git
      URL: ../git.git/
      New remote branch (next fetch will store in remotes/git.git)
        refs/stash
      Tracked remote branches
        ar/sgid-bsd cb/mergetool cc/help cr/reset-parseopt db/diff...

Notice that it talks about storing refs/stash in remotes/git.git?

It won't, and it shouldn't, as we told it to mirror refs/stash to refs/stash.

As a side note, I think remote, when doing "show" that does _not_ update the local side of tracking refs, should tweak the error behaviour that it can trigger when it calls get_fetch_map(). The callee may have originally been written for "fetch" and had a hardcoded "we are fetching and storing, so our behaviour when seeing a funny ref is to error out with "refusing to create" message and that is fine" mentality. Calling such a function when it is not actually fetching, without modifying the callee's assumption to suit the error behaviour for its needs, is sloppy and needs to be fixed.

The scripted version if git-parse-remote, which is being phased out (it still is used somewhat by git-pull and git-clone) has similar logic to parse fetch refspec mapping, but it does not understand the mirroring layout (refs/*:refs/*) as you saw, so it fails way before it triggers these issues. What the C reimplementation does (or at least tries to do) is an improvement in that sense, but because it is relatively a new feature, bugs in there are to be expected.

We are going into 1.5.5-rc feature freeze tonight, and squashing these bugs now are of the highest priority. Please keep the bug reports and fixes flowing.

Thanks.

---

 builtin-check-ref-format.c |    2 +-
 git-parse-remote.sh        |    9 +++++++--
 remote.c                   |   16 +++++++++++++---
 3 files changed, 21 insertions(+), 6 deletions(-)