git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] clone: always fetch remote HEAD

From
Jeff King <peff@peff.net>
Date
Jun 3, 2011, 18:10 UTC
Message-ID
<20110603181052.GA17538@sigill.intra.peff.net>
In-Reply-To
<BANLkTim03_3DLdDkc3QgFrcUa0Fqhhqnbw@mail.gmail.com>
On Fri, Jun 03, 2011 at 12:36:50AM -0500, Sverre Rabbelier wrote:
Show 11 quoted lines
> On Fri, Jun 3, 2011 at 00:18, Jeff King <peff@peff.net> wrote:
> > So I guess it doesn't like us asking for HEAD. But the fact that it
> > sends weird data to fast-import instead of saying "hey, HEAD doesn't
> > exist" has me confused. I'm not sure if this is something one should not
> > be doing with remote helpers, or if the testgit helper is simply buggy
> > or incomplete.
> 
> Definitely the latter, quite possibly the former. I don't know if
> asking for "HEAD" makes much sense in a remote-helper context though.
> In Mercurial it does (e.g., tip), and in svn sort of, but I don't know
> about other vcs-es. Perhaps it should be guarded by a capability?

I did some more digging. I don't think the problem is with HEAD at all, but rather with asking for more than one ref at all.

If we want to fetch multiple refs, and the remote helper has the import capability, we end up in transport-helper.c:fetch_with_import. This will generate an "import %s" line for every ref we are interested in.

Meanwhile, the testgit remote helper will read each import line and call do_import. This ends up in the git_remote_helper library's exporter.export_repo function, which dumps fast-import data to stdout, including some "feature" lines.

On the second import line, we have already sent some fast-import data, but now we send more "feature" lines, which causes fast-import to complain.

So which of the three (git's transport-helper code, the testgit remote helper, or the remote helper library) is at fault?

Is it git itself? The fetch_with_import has to be able to ask for multiple refs, right? Or should it simply say "import" once, and then pick the refs it wants out of the alternate namespace?

Or is it the fault of testgit for calling exporter.export_repo twice? It seems to ignore the ref argument it gets to import, and just dumps the whole repo, albeit into an alternate namespace. Should we then be doing something per-ref with the result? Or since our particular import imports everything, should we simply ignore further imports after the first one?

Or is it the fault of the exporter code, which should not assume it is the first thing to generate fast-import data on stdout? I don't think this is the case, since there would be no point to exporting again, right? We didn't actually restrict the export to a particular ref in the first place.

So I'm confused about who is supposed to be doing what with remote helpers and "import". And I wasn't able to find any helpful documentation or other examples. From reading the transport-helper code, my guess is:

  1. We send the helper a bunch of import lines, one per ref. The helper
     can use that ref to generate fast-import data for just that ref. Or
     it can ignore it and just import everything (like testgit does).
     The result goes into an alternate namespace, like refs/testgit/.
  2. Git reads the sha1 values from the alternate namespace into its
     list of to-fetch refs. This puts us in the same state we would be
     in for other transports (which usually fill in the sha1 earlier,
     but in this case, we don't know it until the import is done).
  3. Git does the usual write-out of remote sha1 values into their
     matching local counterparts.

Does that make sense? If so, then I think the right fix is for testgit ti ignore all imports after the first one (since the first one will have done all available refs). And the patch is:

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index df9d512..7330753 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -111,13 +111,19 @@ def update_local_repo(repo):
     return repo
 
 
+did_import = False
 def do_import(repo, args):
     """Exports a fast-import stream from testgit for git to import.
     """
+    global did_import
 
     if len(args) != 1:
         die("Import needs exactly one ref")
 
+    if did_import:
+	return
+    did_import = True
+
     if not repo.gitdir:
         die("Need gitdir to import")
 

-Peff
Previous: Junio C HamanoNext: Sverre Rabbelier
Message 15 of 65 in “git clone (ssh://) skips detached HEAD”
  1. Dmitry IvankovJun 1, 2011
  2. Jeff KingJun 1, 2011
  3. Dmitry IvankovJun 1, 2011
  4. Junio C HamanoJun 1, 2011
  5. Jeff KingJun 1, 2011
  6. Jeff KingJun 1, 2011
  7. Jeff KingJun 3, 2011
  8. 1/3 t: add tests for cloning remotes with detached HEADJeff King, Jun 3, 2011
  9. 2/3 consider only branches in guess_remote_headJeff King, Jun 3, 2011
  10. 3/3 clone: always fetch remote HEADJeff King, Jun 3, 2011
  11. Sverre RabbelierJun 3, 2011
  12. Jeff KingJun 3, 2011
  13. Jeff KingJun 3, 2011
  14. Junio C HamanoJun 3, 2011
  15. Jeff KingJun 3, 2011
  16. Sverre RabbelierJun 4, 2011
  17. Jeff KingJun 6, 2011
  18. Junio C HamanoJun 6, 2011
  19. Junio C HamanoJun 6, 2011
  20. Sverre RabbelierJun 6, 2011
  21. Junio C HamanoJun 6, 2011
  22. Jeff KingJun 6, 2011
  23. Sverre RabbelierJun 6, 2011
  24. Jeff KingJun 7, 2011
  25. Sverre RabbelierJun 7, 2011
  26. 0/8 minor import/export remote helper fixesJeff King, Jun 7, 2011
  27. 1/8 transport-helper: fix minor leak in push_refs_with_exportJeff King, Jun 7, 2011
  28. 2/8 git-remote-testgit: exit gracefully after pushJeff King, Jun 7, 2011
  29. Sverre RabbelierJun 7, 2011
  30. 3/8 t5800: factor out some ref testsJeff King, Jun 7, 2011
  31. Sverre RabbelierJun 7, 2011
  32. 4/8 t5800: document some non-functional parts of remote helpersJeff King, Jun 7, 2011
  33. Sverre RabbelierJun 7, 2011
  34. Jeff KingJun 7, 2011
  35. Sverre RabbelierJun 7, 2011
  36. Jeff KingJun 7, 2011
  37. Sverre RabbelierJun 7, 2011
  38. Jeff KingJun 7, 2011
  39. Junio C HamanoJun 8, 2011
  40. Jeff KingJun 9, 2011
  41. Junio C HamanoJun 9, 2011
  42. Jeff KingJun 9, 2011
  43. Sverre RabbelierJun 9, 2011
  44. 5/8 teach remote-testgit to import non-HEAD refsJeff King, Jun 7, 2011
  45. Junio C HamanoJun 8, 2011
  46. Jeff KingJun 9, 2011
  47. 6/8 teach remote-testgit to import multiple refsJeff King, Jun 7, 2011
  48. 7/8 transport-helper: don't feed bogus refs to export pushJeff King, Jun 7, 2011
  49. Sverre RabbelierJun 7, 2011
  50. 8/8 git_remote_helpers: push all refs during a non-local exportJeff King, Jun 7, 2011
  51. Sverre RabbelierJun 7, 2011
  52. Jeff KingJun 7, 2011
  53. Sverre RabbelierJun 7, 2011
  54. Junio C HamanoJun 6, 2011
  55. Jeff KingJun 6, 2011
  56. Jeff KingJun 7, 2011
  57. 1/2 make copy_ref globally availableJeff King, Jun 7, 2011
  58. 2/2 clone: always fetch remote HEADJeff King, Jun 7, 2011
  59. Junio C HamanoJun 7, 2011
  60. Junio C HamanoJun 3, 2011
  61. Jeff KingJun 3, 2011
  62. Jakub NarebskiJun 1, 2011
  63. Jeff KingJun 1, 2011
  64. Jakub NarebskiJun 2, 2011
  65. Jeff KingJun 3, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.