Re: [PATCH v5 06/10] fast-export: add new --refspec option

From: Jeff King <peff@peff.net>
Date: Nov 9, 2013, 00:06 UTC
Message-ID: <20131109000653.GB22906@sigill.intra.peff.net>
In-Reply-To: <xmqqwqkimpas.fsf@gitster.dls.corp.google.com>

On Fri, Nov 08, 2013 at 03:44:43PM -0800, Junio C Hamano wrote:

Show 11 quoted lines

> > The "^origin" is not a refspec, and finding the refspec in the
> > dot-expression would involve parsing it into two components. I think you
> > can come up with a workable system by parsing the arguments as revision
> > specifiers and then applying some rules. E.g....
> 
> I was thinking about this a bit more today.  It is more or less
> trivial to actually teach the setup_revisions() infrastructure to
> optionally allow A:B to mean "We want a revision A, but with an
> extra twist", and leave that "extra twist" information in the
> rev_cmdline machinery.  After all, rev_cmdline was introduced for
> doing exactly this kind of thing.

I had wondered at first whether it would be a problem to be syntactically ambiguous with <ref>:<path>. But I don't think so. Doing:

   git fast-export HEAD:Documentation

does not produce any output currently. And the fast-import stream cannot actually represent trees outside of commits. Something like:

  git fast-export HEAD:Makefile

could produce a blob entry, but it currently does nothing. So I don't think it's a big deal, as nobody is clamoring for the feature. And certainly "git push" suffers from the same problem, and you can work around it with:

  git push $(git rev-parse HEAD:Makefile):refs/tag/makefile-blob

Show 13 quoted lines

> Earlier I said that all the existing transport commands take refspec
> as proper operands, not a value to a dashed-option, but I can imagine
> that we may in the future want to update "git push" in a way similar
> to what Felipe did to "git fast-export" so that it allows something
> like this:
> 
>     $ git push mothership \
>     > --refspec refs/heads/*:refs/remotes/satellite/* master
> 
> which would mean "I am pushing out 'master', but anything I push out
> to the mothership from my refs/heads/ hierarchy should be used to
> update the refs/remotes/satellite/ hierarchy over there".  The same
> thing can be done in the reverse direction for "git fetch".

Yes, though I would expect the "--refspec" bit would end up in config (else why would you not just apply it directly to master). And you would want to be able to override it, like:

  git push mothership \
    --refspec refs/heads/*:refs/remotes/satellite/* master:refs/other/master

So I think even with such wildcard magic, you'd still want refspecs, both for "push" and for "fast-export".

Show 5 quoted lines

> But such a wildcard refspec cannot be supported naturally by
> extending the setup_revisions(); what the wildcarded refspec expands
> to will depend on what other things are on the command line (in this
> case, 'master').  So I stopped there (I'll attach a toy patch at the
> end, but I'll discard it once I send this message out).

Yes, I think applying such a refspec would be the duty of whoever expands "master" into "master:master". And that is not a job for setup_revisions, which should be dealing mostly with the syntax and telling us "I got just 'master'" or "I got 'master:foo'". The interpretation is up to the caller.

> If we were to go that route, however, I would be strongly against
> calling that option --refspec; perhaps calling it --refmap would
> avoid confusion.

Agreed.

-Peff