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

Re: [PATCH v3 1/7] fetch: increase test coverage of fetches

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 3, 2022, 06:51 UTC
Message-ID
<YiBlXSrSy1iajd9r@ncase>
In-Reply-To
<xmqqmti7bkf2.fsf@gitster.g>
On Wed, Mar 02, 2022 at 10:49:05PM -0800, Junio C Hamano wrote:
Show 44 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> >> +				# would cause us to die immediately.
> >> >> +				git update-ref refs/tags/tag1/nested $B
> >> >> +				exit \$!
> >> >> +			fi
> >> >> +		done
> >> >> +	EOF
> >> >
> >> > I think I've reviewed the previous round of these patches in
> >> > detail.  I by mistake sent a comment for this step in v2, but I
> >> > think the same puzzlement exists in this round, too.
> >> 
> >> Namely:
> >> 
> >> I have been wondering if we need to do this from the hook?  If we
> >> have this ref before we start "fetch", would it have the same
> >> effect, or "fetch" notices that this interfering ref exists and
> >> removes it to make room for storing refs/tags/tag1, making the whole
> >> thing fail to fail?
> >> 
> >> > +				exit \$!
> >> 
> >> In any case, "exit 0" or "exit \$?" would be understandable, but
> >> exit with "$!", which is ...?  The process ID of the most recent
> >> background command?  Puzzled.
> >
> > Oof, this was supposed to be `exit \$?`, thanks for catching this. But
> > your above comment is right: we can indeed just create the D/F conflict
> > outside of the hook and thus avoid the hook script altogether. Thanks!
> 
> I see.
> 
> As that shell does not send anything to background, at the point of
> the reference $! would yield an empty string, and "exit" is
> equivalent to "exit $?", it is doing the right thing, I presume.
> 
> The topic has been in 'next' for a while, so if you are inclined to
> fix it up, please send an incremental patch.  If you do "exit" it
> would be a one-liner change, or if you use a different "cause D/F
> conflict outside the hook" approach, the change may become a bit
> more involved.
> 
> Thanks.

Fair enough, thanks for clarifying the steps. Does it make sense to also change indentantion of the heredocs as per your review of v2 or should I just keep that as-is now?

Patrick
Previous: Junio C HamanoNext: Patrick Steinhardt
Message 29 of 35 in “fetch: improve atomicity of `--atomic` flag”
  1. 0/7 fetch: improve atomicity of `--atomic` flagPatrick Steinhardt, Feb 17, 2022
  2. 1/7 fetch: increase test coverage of fetchesPatrick Steinhardt, Feb 17, 2022
  3. Christian CouderFeb 17, 2022
  4. Patrick SteinhardtFeb 21, 2022
  5. Junio C HamanoFeb 17, 2022
  6. Junio C HamanoFeb 17, 2022
  7. Patrick SteinhardtFeb 18, 2022
  8. Junio C HamanoFeb 18, 2022
  9. Junio C HamanoMar 3, 2022
  10. Patrick SteinhardtMar 3, 2022
  11. 2/7 fetch: backfill tags before setting upstreamPatrick Steinhardt, Feb 17, 2022
  12. Junio C HamanoFeb 17, 2022
  13. 3/7 fetch: control lifecycle of FETCH_HEAD in a single placePatrick Steinhardt, Feb 17, 2022
  14. Junio C HamanoFeb 17, 2022
  15. 4/7 fetch: report errors when backfilling tags failsPatrick Steinhardt, Feb 17, 2022
  16. Junio C HamanoFeb 17, 2022
  17. 6/7 fetch: make `--atomic` flag cover backfilling of tagsPatrick Steinhardt, Feb 17, 2022
  18. Junio C HamanoFeb 17, 2022
  19. 5/7 refs: add interface to iterate over queued transactional updatesPatrick Steinhardt, Feb 17, 2022
  20. 7/7 fetch: make `--atomic` flag cover pruning of refsPatrick Steinhardt, Feb 17, 2022
  21. Christian CouderFeb 17, 2022
  22. Junio C HamanoFeb 17, 2022
  23. 0/7 fetch: improve atomicity of `--atomic` flagPatrick Steinhardt, Feb 21, 2022
  24. 1/7 fetch: increase test coverage of fetchesPatrick Steinhardt, Feb 21, 2022
  25. Junio C HamanoMar 3, 2022
  26. Junio C HamanoMar 3, 2022
  27. Patrick SteinhardtMar 3, 2022
  28. Junio C HamanoMar 3, 2022
  29. Patrick SteinhardtMar 3, 2022
  30. 2/7 fetch: backfill tags before setting upstreamPatrick Steinhardt, Feb 21, 2022
  31. 3/7 fetch: control lifecycle of FETCH_HEAD in a single placePatrick Steinhardt, Feb 21, 2022
  32. 4/7 fetch: report errors when backfilling tags failsPatrick Steinhardt, Feb 21, 2022
  33. 5/7 refs: add interface to iterate over queued transactional updatesPatrick Steinhardt, Feb 21, 2022
  34. 6/7 fetch: make `--atomic` flag cover backfilling of tagsPatrick Steinhardt, Feb 21, 2022
  35. 7/7 fetch: make `--atomic` flag cover pruning of refsPatrick Steinhardt, Feb 21, 2022

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.