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

Re: [PATCH 2/2] pack-refs: add fully-peeled trait

From
Jeff King <peff@peff.net>
Date
Mar 17, 2013, 06:04 UTC
Message-ID
<20130317060408.GE16070@sigill.intra.peff.net>
In-Reply-To
<51447C5E.3050808@alum.mit.edu>
On Sat, Mar 16, 2013 at 03:06:22PM +0100, Michael Haggerty wrote:
Show 24 quoted lines
> >  		refname = parse_ref_line(refline, sha1);
> >  		if (refname) {
> > -			last = create_ref_entry(refname, sha1, flag, 1);
> > +			/*
> > +			 * Older git did not write peel lines for anything
> > +			 * outside of refs/tags/; if the fully-peeled trait
> > +			 * is not set, we are dealing with such an older
> > +			 * git and cannot assume an omitted peel value
> > +			 * means the ref is not a tag object.
> > +			 */
> > +			int this_flag = flag;
> > +			if (!fully_peeled && prefixcmp(refname, "refs/tags/"))
> > +				this_flag &= ~REF_KNOWS_PEELED;
> > +
> > +			last = create_ref_entry(refname, sha1, this_flag, 1);
> >  			add_ref(dir, last);
> >  			continue;
> >  		}
> 
> I have to admit that I am partial to my variant of this code [1] because
> the logic makes it clearer when the affirmative decision can be made to
> set the REF_KNOWS_PEELED flag.  But this version also looks correct to
> me and equivalent (aside from the idea that a few lines later if a
> peeled value is found then the REF_KNOWS_PEELED bit could also be set).

Yeah, I think they are equivalent, but I agree yours is a little more readable. I'll switch it in my re-roll, and I will go ahead and set the REF_KNOWS_PEELED bit when we see a peel line. That code should not be triggered in general, but it is the sane thing for the reader to do, so it makes the code more obvious and readable.

-Peff
Previous: Michael HaggertyNext: Junio C Hamano
Message 7 of 19 in “peel-ref optimization fixes”
  1. 0/2 peel-ref optimization fixesJeff King, Mar 16, 2013
  2. 1/2 pack-refs: write peeled entry for non-tagsJeff King, Mar 16, 2013
  3. Michael HaggertyMar 16, 2013
  4. Jeff KingMar 17, 2013
  5. 2/2 pack-refs: add fully-peeled traitJeff King, Mar 16, 2013
  6. Michael HaggertyMar 16, 2013
  7. Jeff KingMar 17, 2013
  8. Junio C HamanoMar 17, 2013
  9. Jeff KingMar 17, 2013
  10. 0/4 peel-ref optimization fixesJeff King, Mar 17, 2013
  11. 1/4 avoid segfaults on parse_object failureJeff King, Mar 17, 2013
  12. 2/4 use parse_object_or_die instead of die("bad object")Jeff King, Mar 17, 2013
  13. 3/4 pack-refs: write peeled entry for non-tagsJeff King, Mar 17, 2013
  14. 4/4 pack-refs: add fully-peeled traitJeff King, Mar 17, 2013
  15. Junio C HamanoMar 17, 2013
  16. 4/4 pack-refs: add fully-peeled traitJeff King, Mar 18, 2013
  17. Junio C HamanoMar 18, 2013
  18. Michael HaggertyMar 18, 2013
  19. Junio C HamanoMar 18, 2013

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.