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

[PATCH v2 0/6] "am" state file fix with write_file() clean-up

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 24, 2015, 20:58 UTC
Message-ID
<1440449890-29490-1-git-send-email-gitster@pobox.com>
In-Reply-To
<xmqqzj1g31e5.fsf@gitster.dls.corp.google.com>

"git am" was recently reimplemented in C. While the implementation was done conservatively and followed the original logic in the scripted version fairly faithfully, the state files it left in the $GIT_DIR/rebase-apply directory were made slightly different by mistake---they lacked the final LF, leaving their last line incomplete.

The patch [1/6] is Peff's idea to consolidate callers in "am", in a more concrete form.

The patch [2/6] is the fix to the state files with incomplete lines.

The workhorse helper function that implements "we have this (short) body of text; create a new file that contains it" has a "fatal" parameter, to which 1 was passed by almost all callers, but to casual readers, it was unclear what that 1 meant. The patch [3/6] splits it to write_file() and write_file_gently() and drops this parameter that looks mysterious at the callsites. A common helper function write_file_v() is introduced to implement these two as thin wrappers of it.

The patch [4/6] updates write_file_v() so that it does the "we are writing a text file. Make sure it does not end with an incomplete line" logic that [2/6] added only to builtin/am.c, thusly reverting what was done to builtin/am.c in [2/6].

The patch [5/6] stops all callers that creates a single-liner file using write_file() and write_file_gently() from including the final LF to the format they pass. This should not change the behaviour, but it probably makes it conceptually cleaner. You have the contents to be placed on a single line, and the helper turns the contents into a proper "line".

The patch [6/6] drops the final LF from the parameter to create a multi-line file; while this does not hurt in the sense that the callee will add a necessary LF back, I do not think it should be applied. Conceptually, if you have a buffer that contains a bunch of lines and throw it at a helper to create a file, you'd better have the terminating LF yourself before asking the helper to put them in the file.

Junio C Hamano (6):
  builtin/am: introduce write_state_*() helper functions
  builtin/am: make sure state files are text
  write_file(): drop "fatal" parameter
  write_file_v(): do not leave incomplete line at the end
  write_file(): drop caller-supplied LF from calls to create a one-liner
    file
  write_file(): drop caller-supplied LF from multi-line file
 builtin/am.c       | 69 ++++++++++++++++++++++++++++++++----------------------
 builtin/branch.c   |  4 ++--
 builtin/init-db.c  |  2 +-
 builtin/worktree.c | 10 ++++----
 cache.h            |  5 ++--
 daemon.c           |  2 +-
 setup.c            |  2 +-
 submodule.c        |  2 +-
 transport.c        |  2 +-
 wrapper.c          | 36 ++++++++++++++++++++++++----
 10 files changed, 88 insertions(+), 46 deletions(-)
-- 
2.5.0-568-g53a3e28
Previous: Junio C HamanoNext: Junio C Hamano
Message 24 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.