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

[PATCH 0/5] fix pack-refs pruning of refs/foo

From
Jeff King <peff@peff.net>
Date
Aug 23, 2014, 05:23 UTC
Message-ID
<20140823052334.GA17813@peff.net>

I noticed that "git pack-refs --all" will pack a top-level ref like "refs/foo", but will not actually prune "$GIT_DIR/refs/foo". I do not see the point in having a policy not to pack "refs/foo" if "--all" is given. But even if we did have such a policy, this seems broken; we should either pack and prune, or leave them untouched. I don't see any indication that the existing behavior was intentional.

The problem is that pack-refs's prune_ref calls lock_ref_sha1, which enforces this "no toplevel" behavior. I am not sure there is any real point to this, given that most callers use lock_any_ref_for_update, which is exactly equivalent but without the toplevel check.

The first two patches deal with this by switching pack-refs to use lock_any_ref_for_update. This will conflict slightly with Ronnie's ref-transaction work, as he gets rid of lock_ref_sha1 entirely, and moves the code directly into prune_ref. This can be trivially resolved in favor of my patch, I think.

The third patch is a cleanup I noticed while looking around, and I think should not conflict with anyone (and is a good thing to do).

The last two are trickier. I wondered if we could get rid of lock_ref_sha1 entirely. After pack-refs, there are two callers: fast-import.c and walker.c. After converting the first, it occurred to me that Ronnie might be touching the same areas, and I see that yes, indeed, there's quite a bit of conflict (and he reaches the same end goal of dropping it entirely).

So in that sense I do not mind dropping the final two patches. Ronnie's endpoint is much better, moving to a ref_transaction. However, there is actually a buffer overflow in the existing code. Ronnie's series fixes it in a similar way (moving to a strbuf), and I'm fine with that endpoint. But given that the ref transaction code is not yet merged (and would certainly never be maint-track), I think it is worth applying the buffer overflow fix separately.

I think the final patch can probably be dropped, then. It is a clean-up, but one that we can just get when Ronnie's series is merged.

  [1/5]: git-prompt: do not look for refs/stash in $GIT_DIR
  [2/5]: pack-refs: prune top-level refs like "refs/foo"
  [3/5]: fast-import: clean up pack_data pointer in end_packfile
  [4/5]: fast-import: fix buffer overflow in dump_tags
  [5/5]: fast-import: stop using lock_ref_sha1
-Peff
Next: Jeff King
Message 1 of 14 in “fix pack-refs pruning of refs/foo”
  1. 0/5 fix pack-refs pruning of refs/fooJeff King, Aug 23, 2014
  2. 1/5 git-prompt: do not look for refs/stash in $GIT_DIRJeff King, Aug 23, 2014
  3. 2/5 pack-refs: prune top-level refs like "refs/foo"Jeff King, Aug 23, 2014
  4. 3/5 fast-import: clean up pack_data pointer in end_packfileJeff King, Aug 23, 2014
  5. Ronnie SahlbergAug 25, 2014
  6. Jeff KingAug 25, 2014
  7. 4/5 fast-import: fix buffer overflow in dump_tagsJeff King, Aug 23, 2014
  8. Ronnie SahlbergAug 25, 2014
  9. 5/5 fast-import: stop using lock_ref_sha1Jeff King, Aug 23, 2014
  10. Ronnie SahlbergAug 25, 2014
  11. Michael HaggertyAug 23, 2014
  12. Jeff KingAug 23, 2014
  13. Ronnie SahlbergAug 25, 2014
  14. Jeff KingAug 25, 2014

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.