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

Re: [PATCH 0/5] "am" state file fix with write_file() clean-up

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 24, 2015, 18:15 UTC
Message-ID
<xmqqlhd04ko4.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20150824174142.GA4794@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 11 quoted lines
> On Mon, Aug 24, 2015 at 10:09:41AM -0700, Junio C Hamano wrote:
>
>> So here is an solution based on the "write_file() is primarily to
>> produce text, so it should be able to correct the incomplete line
>> at the end" approach.
>
> This all looks good to me. The topics-in-flight compatibility stuff in
> patches 3 and 5 is neatly done. Usually I would just cheat and change
> the order of arguments to make the compiler notice such problems, but
> that's hard to do here because of the varargs (you cannot just bump
> "flags" to the end).

Actually, I think my compatibility stuff is worthless. It would not catch new callers that wants to only probe and do their own error handling by passing 0 (and besides, assert() is a shoddy way to do this---there is no guarantee that tests will trigger all the codepaths in the first place).

We should deprecate and remove write_file() by renaming the one with the updated semantics to something else, possibly with a backward compatiblity thin wrapper around it that is called write_file(), or without it to force a link-time error.

Thanks for a dose of sanity.
Previous: Jeff KingNext: Jeff King
Message 21 of 36 in “Minor builtin 'git am' side-effect”
  1. SZEDER GáborAug 20, 2015
  2. Junio C HamanoAug 20, 2015
  3. am: terminate state files with a newlinePaul Tan, Aug 23, 2015
  4. SZEDER GáborAug 23, 2015
  5. Junio C HamanoAug 23, 2015
  6. Jeff KingAug 24, 2015
  7. Junio C HamanoAug 24, 2015
  8. Jeff KingAug 24, 2015
  9. 0/5 "am" state file fix with write_file() clean-upJunio C Hamano, Aug 24, 2015
  10. 1/5 builtin/am: introduce write_state_*() helper functionsJunio C Hamano, Aug 24, 2015
  11. 2/5 builtin/am: make sure state files are textJunio C Hamano, Aug 24, 2015
  12. 3/5 write_file(): introduce an explicit WRITE_FILE_GENTLY requestJunio C Hamano, Aug 24, 2015
  13. Junio C HamanoAug 24, 2015
  14. Duy NguyenAug 25, 2015
  15. setup: update the right file in multiple checkoutsNguyễn Thái Ngọc Duy, Aug 25, 2015
  16. Junio C HamanoAug 25, 2015
  17. Duy NguyenAug 31, 2015
  18. 4/5 write_file(): do not leave incomplete line at the endJunio C Hamano, Aug 24, 2015
  19. 5/5 write_file(): clean up transitional mess of flag words and terminating LFJunio C Hamano, Aug 24, 2015
  20. Jeff KingAug 24, 2015
  21. Junio C HamanoAug 24, 2015
  22. Jeff KingAug 24, 2015
  23. Junio C HamanoAug 24, 2015
  24. 0/6 "am" state file fix with write_file() clean-upJunio C Hamano, Aug 24, 2015
  25. 1/6 builtin/am: introduce write_state_*() helper functionsJunio C Hamano, Aug 24, 2015
  26. 2/6 builtin/am: make sure state files are textJunio C Hamano, Aug 24, 2015
  27. Jeff KingAug 24, 2015
  28. Junio C HamanoAug 25, 2015
  29. Jeff KingAug 25, 2015
  30. Junio C HamanoAug 25, 2015
  31. 3/6 write_file(): drop "fatal" parameterJunio C Hamano, Aug 24, 2015
  32. 4/6 write_file_v(): do not leave incomplete line at the endJunio C Hamano, Aug 24, 2015
  33. 5/6 write_file(): drop caller-supplied LF from calls to create a one-liner fileJunio C Hamano, Aug 24, 2015
  34. 6/6 write_file(): drop caller-supplied LF from multi-line fileJunio C Hamano, Aug 24, 2015
  35. Jeff KingAug 25, 2015
  36. brian m. carlsonAug 24, 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.