Re: [PATCH 03/19] t5800: document some non-functional parts of remote helpers
- From
Jeff King <peff@peff.net>
- Date
- Jun 10, 2011, 01:18 UTC
- Message-ID
- <20110610011858.GA12256@sigill.intra.peff.net>
- In-Reply-To
- <20110608192850.GF27715@elie>
On Wed, Jun 08, 2011 at 02:28:51PM -0500, Jonathan Nieder wrote:
Show 10 quoted lines
> > but we have no way of > > communicating to fast-export that this name mapping is > > happening. > > It's not just name mapping, no? E.g., I could try > > git push testgit::/path/to/repo $(git rev-parse HEAD):new > > So I think the current implementation of "export" by testgit > is just wrong. ;-)
Yeah, it's the same issue. We have no way of communicating the name of the right-hand ref at all to a helper, aside from asking fast-export to munge the refname in its output stream. But we have no way of telling fast-export that even though we asked for "foo", it needs to write the result into "bar" (and potentially _also_ into "foo", if it's a ref found on the RHS of a different refspec).
Show 7 quoted lines
> This seems like a reasonable solution. One way would be to teach > fast-export about refspecs; another way would be to set up refs in a > private namespace and then munge the stream that fast-export spits out > before passing it back to the transport-helper; another (more ugly and > hackish) way would be to set up a private repository with the real > repository set up as an alternate, put the appropriate refs there, and > point fast-export to that.
Munging the stream is a little tricky. We have to correctly parse the stream and not just do something hack-ish like sed (which is what git-remote-testgit does, but that's OK, because it's a hack-ish test script).
But we also have to take care to handle duplicates. So exporting "refs/heads/foo:refs/heads/bar" would mean that we rewrite all of refs/heads/foo into refs/heads/bar on commit lines. But two refspecs like "refs/heads/foo:refs/heads/bar refs/heads/foo:refs/heads/foo" mean we must write all of the commit lines as "foo" and then do a "reset" of bar to foo at the end (at least this is what fast-export already does). But we also need to handle criss-crosses, like:
refs/heads/foo:refs/heads/bar refs/heads/baz:refs/heads/foo
In that case we want change all commit lines for "foo" to "bar", but also all for "baz" to "foo".
So implementation-wise, the simplest way is if you can give each tip a name, and then at the end assign all of the tips to refnames (this is more or less what regular push does internally, of course).
I'm not too familiar with the fast-import protocol or what other systems' fast importers are capable of. Is it OK to just make a bunch of commits with marks, and then at the end do our resets? That would make the problem much simpler, I think.
> Anyway, I don't think this should be in the commit message for the new > test which doesn't even know about the remote helper protocol. :)
Yeah, maybe not. I wanted to stick the discussion somewhere, and since I never actually fixed this bug, there wasn't another relevant commit. :)
Show 17 quoted lines
> > --- a/t/t5800-remote-helpers.sh > > +++ b/t/t5800-remote-helpers.sh > > @@ -81,4 +81,51 @@ test_expect_success PYTHON_24 'pushing remote local repo' ' > > compare_refs clone HEAD server HEAD > > ' > > > > +test_expect_failure PYTHON_24 'fetch new branch' ' > > Side note: this repeated PYTHON_24 implementation detail as a > prerequisite feels wrong. Would it make sense to do something like > > if test_have_prereq PYTHON_24 > then > test_set_prereq REMOTE_TESTGIT > fi > > at the start and use that?
Really, I think you could just exit early if PYTHON_24 isn't set, as all of the tests require it. If we end up with other non-python helper tests, they will probably go in a separate script anyway.
Show 10 quoted lines
> > +test_expect_failure PYTHON_24 'push new branch with old:new refspec' ' > > + (cd clone && > > + git push origin new-name:new-refspec > > + ) && > > + compare_refs clone HEAD server refs/heads/new-refspec > > +' > > It would also be interesting to test tag pushes and pushes by object > name. > [examples]
Agreed.
-Peff