Subject: Re: [PATCH] am: terminate state files with a newline

From: Junio C Hamano <[email-masked]>
Date: Sun, 23 Aug 2015 12:05:32 -0700

Paul Tan <[email-masked]> writes:

> Did we ever explictly allow external programs to poke around the
> contents of the .git/rebase-apply directory?

We tell users to take a peek into it when "am" fails, don't we, by
naming $GIT_DIR/rebase-apply/patch?

> -	write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f");
> +	write_file(am_path(state, "threeway"), 1, "%s\n", state->threeway ? "t" : "f");

Stepping back a bit, after realizing that "write_file()" is a
short-hand for "I have all information necessary to produce the full
contents of a file, now go ahead and create and write that and
close", I have to wonder what caller even wants to create a file
with an incomplete line at the end.

All callers outside builtin/am.c except one caller uses it to
produce a single line file.  The oddball is "git branch" that uses
it to prepare a temporary file used to edit branch description.  

builtin/branch.c:	if (write_file(git_path(edit_description), 0, "%s", buf.buf)) {

The payload it prepares in buf.buf ends with a canned comment that
ends with LF.  So in that sense it is not even an oddball.

The above analysis makes me wonder if this is a simpler and more
future proof approach.

Or did I miss any caller or a reasonable potential future use case
that wants to create a binary file or a text file that ends with an
incomplete line?

 wrapper.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/wrapper.c b/wrapper.c
index e451463..7a92298 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -621,6 +621,13 @@ char *xgetcwd(void)
 	return strbuf_detach(&sb, NULL);
 }
 
+/*
+ * Create a TEXT file by specifying its full contents via fmt and the
+ * remainder of args that are used like "printf".  A terminating LF is
+ * added at the end of the file if it is missing (it is simpler for
+ * the callers because the function is often used to create a
+ * single-liner file).
+ */
 int write_file(const char *path, int fatal, const char *fmt, ...)
 {
 	struct strbuf sb = STRBUF_INIT;
@@ -634,6 +641,9 @@ int write_file(const char *path, int fatal, const char *fmt, ...)
 	va_start(params, fmt);
 	strbuf_vaddf(&sb, fmt, params);
 	va_end(params);
+	if (sb.len)
+		strbuf_complete_line(&sb);
+
 	if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
 		int err = errno;
 		close(fd);