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

Re: [PATCH v2 08/12] initial_ref_transaction_commit(): function for initial ref creation

From
Jeff King <peff@peff.net>
Date
Jun 15, 2015, 18:57 UTC
Message-ID
<20150615185746.GB10080@peff.net>
In-Reply-To
<xmqqy4jk95pw.fsf@gitster.dls.corp.google.com>
On Mon, Jun 15, 2015 at 11:39:07AM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
> > It would seem to make sense to add a test here that there are no
> > existing references, because that is how the function *should* be
> > used. But in fact, the "testgit" remote helper appears to call it
> > *after* having set up refs/remotes/<name>/HEAD and
> > refs/remotes/<name>/master, so we can't be so strict.
> 
> Is "testgit" so hard to fix (or so sacred to break) that we want to
> sacrifice the quality of this part that is nearer to the core?

I do not think "testgit" is sacred; it is there to serve the needs of git and not the other way around. But we do have to stop and think whether what "testgit" is doing is representative of what a real remote-helper might do.

It has been a while since I touched the remote-helper code (and it has always been under-documented and confusing, from my recollection), but I think it is not "testgit" that is making the refs, but rather that "import"-style helpers will feed a data stream to git-fast-import (actually the transport code does it on behalf of the remote).

So "fast-import" creates the refs, and then "clone" later wants to recreate them. IMHO this is not ideal[1]; we should have fast-import create the objects and report the ref tips, which can then be relayed to clone. But with the way the code works now, this is not about fixing "testgit", but rather would break any "import"-style helper.

Take all of that with a grain of salt, though. I might be totally wrong about how this part of the code works.

-Peff
[1] This may also be buggy for regular fetches. E.g., do we correctly
    respect "--force" (or lack thereof) and "+"-refspecs when importing?
    I am not sure, as I think we may pass enough information to the
    helper to handle this itself. But certainly if the responsibility
    for updating refs was always in the caller (clone or fetch), then it
    would follow the regular code path and Just Work.
Previous: Junio C HamanoNext: Junio C Hamano
Message 18 of 30 in “Improve "refs" module encapsulation”
  1. 00/12 Improve "refs" module encapsulationMichael Haggerty, Jun 13, 2015
  2. 01/12 delete_ref(): move declaration to refs.hMichael Haggerty, Jun 13, 2015
  3. 02/12 remove_branches(): remove temporaryMichael Haggerty, Jun 13, 2015
  4. 03/12 delete_ref(): handle special case more explicitlyMichael Haggerty, Jun 13, 2015
  5. Junio C HamanoJun 15, 2015
  6. 04/12 delete_refs(): new function for the refs APIMichael Haggerty, Jun 13, 2015
  7. 05/12 delete_refs(): improve error messageMichael Haggerty, Jun 13, 2015
  8. Junio C HamanoJun 15, 2015
  9. Michael HaggertyJun 19, 2015
  10. 06/12 prune_remote(): use delete_refs()Michael Haggerty, Jun 13, 2015
  11. Junio C HamanoJun 15, 2015
  12. Jeff KingJun 15, 2015
  13. Junio C HamanoJun 15, 2015
  14. 07/12 repack_without_refs(): make function privateMichael Haggerty, Jun 13, 2015
  15. 08/12 initial_ref_transaction_commit(): function for initial ref creationMichael Haggerty, Jun 13, 2015
  16. Junio C HamanoJun 15, 2015
  17. Junio C HamanoJun 15, 2015
  18. Jeff KingJun 15, 2015
  19. Junio C HamanoJun 15, 2015
  20. Michael HaggertyJun 20, 2015
  21. 09/12 refs: remove some functions from the module's public interfaceMichael Haggerty, Jun 13, 2015
  22. 10/12 initial_ref_transaction_commit(): check for duplicate refsMichael Haggerty, Jun 13, 2015
  23. 11/12 initial_ref_transaction_commit(): check for ref D/F conflictsMichael Haggerty, Jun 13, 2015
  24. 12/12 refs: move the remaining ref module declarations to refs.hMichael Haggerty, Jun 13, 2015
  25. Junio C HamanoJun 15, 2015
  26. Jeff KingJun 15, 2015
  27. Junio C HamanoJun 15, 2015
  28. Michael HaggertyJun 20, 2015
  29. Junio C HamanoJun 20, 2015
  30. Stefan BellerJun 15, 2015

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.