"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
got patch: handle "\ No newline at end of file"
To:
gameoftrees@openbsd.org
Date:
Sat, 19 Mar 2022 17:48:05 +0100

Download raw body.

Thread
This is the last "big" missing piece for 'got patch': handle the "\ No
newline at end of file".

The implementation is quite simple: avoid printing the last newline
character if the patch says so.

I'm now chopping off all the final newline character when reading lines
(either from the patchfile or from the "original" file): this keeps the
locate_hunk and test_hunk functions (the hunk matching routines) working
in every case.

The only consequence is that we don't fail to match a hunk if the
original file has a trailing newline and patchfile says that it
shouldn't.  I don't think it's a great deal, the "\ Now newline at eof"
indicator is only allowed at the end so we don't risk to apply the patch
at a different offset, and it's common for editors to add a trailing
newline anyway (Emacs has a `require-final-newline' setting and mg(1)
always prompts to add a newline if missing when saving.)

The only vaguely tricky part is the parsing.  The hunk header (the line
that starts with "@@") has some counters for the number of following
lines that belong to the old or new file, but the "\ No final newline"
line is not accounted for in there.  It's only allowed after the last
'+' or '-' thought, so it's not that bad.  I'm peeking one character in
peek_special_line and either consume the line or put that back.
ungetc(3) says that one character of push-back is guaranteed, so it
should be safe.  It's fundamentally the same thing Larry Wall' patch
does, grep for `remove_special_line' in usr.bin/patch/pch.c

The patch also include a small tweak that I'll probably land as a
separate commit: reuse apply_hunk instead of rolling the loop again in
patch_file for the "create file" case.

ok?

diff refs/heads/main refs/heads/pnl
blob - 20b4e1e2357e5e26d85170030eb53e9f387807e0
blob + 7a531892158d2e38781806351827aa51d3c4970a
--- lib/patch.c
+++ lib/patch.c
@@ -15,9 +15,6 @@
  *
  * Apply patches.
  *
- * Things that are still missing:
- *     + "No final newline" handling
- *
  * Things that we may want to support:
  *     + support indented patches?
  *     + support other kinds of patches?
@@ -58,6 +55,7 @@ struct got_patch_hunk {
 	STAILQ_ENTRY(got_patch_hunk) entries;
 	const struct got_error *err;
 	long	offset;
+	int	nonl;
 	long	old_from;
 	long	old_lines;
 	long	new_from;
@@ -201,6 +199,10 @@ recv_patch(struct imsgbuf *ibuf, int *done, struct got
 		case GOT_IMSG_PATCH_DONE:
 			goto done;
 		case GOT_IMSG_PATCH_HUNK:
+			if (h != NULL && h->nonl) {
+				err = got_error(GOT_ERR_PATCH_MALFORMED);
+				goto done;
+			}
 			datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
 			if (datalen != sizeof(hdr)) {
 				err = got_error(GOT_ERR_PRIVSEP_LEN);
@@ -224,16 +226,22 @@ recv_patch(struct imsgbuf *ibuf, int *done, struct got
 			}
 			datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
 			t = imsg.data;
-			/* at least one char plus newline */
+			/* at least one char */
 			if (datalen < 2 || t[datalen-1] != '\0') {
 				err = got_error(GOT_ERR_PRIVSEP_MSG);
 				goto done;
 			}
-			if (*t != ' ' && *t != '-' && *t != '+') {
+			if (*t != ' ' && *t != '-' && *t != '+' &&
+			    *t != '\\') {
 				err = got_error(GOT_ERR_PRIVSEP_MSG);
 				goto done;
 			}
-			err = pushline(h, t);
+			if (h->nonl)
+				err = got_error(GOT_ERR_PATCH_MALFORMED);
+			if (*t == '\\')
+				h->nonl = 1;
+			else
+				err = pushline(h, t);
 			if (err)
 				goto done;
 			break;
@@ -303,6 +311,8 @@ locate_hunk(FILE *orig, struct got_patch_hunk *h, off_
 				err = got_error(GOT_ERR_HUNK_FAILED);
 			break;
 		}
+		if (line[linelen - 1] == '\n')
+			line[linelen - 1] = '\0';
 		(*lineno)++;
 
 		if ((mode == ' ' && !strcmp(h->lines[0] + 1, line)) ||
@@ -355,6 +365,8 @@ test_hunk(FILE *orig, struct got_patch_hunk *h)
 					    GOT_ERR_HUNK_FAILED);
 				goto done;
 			}
+			if (line[linelen - 1] == '\n')
+				line[linelen - 1] = '\0';
 			if (strcmp(h->lines[i] + 1, line)) {
 				err = got_error(GOT_ERR_HUNK_FAILED);
 				goto done;
@@ -376,7 +388,7 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long *
 	for (i = 0; i < h->len; ++i) {
 		switch (*h->lines[i]) {
 		case ' ':
-			if (fprintf(tmp, "%s", h->lines[i] + 1) < 0)
+			if (fprintf(tmp, "%s\n", h->lines[i] + 1) < 0)
 				return got_error_from_errno("fprintf");
 			/* fallthrough */
 		case '-':
@@ -385,6 +397,11 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long *
 		case '+':
 			if (fprintf(tmp, "%s", h->lines[i] + 1) < 0)
 				return got_error_from_errno("fprintf");
+			if (i != h->len - 1 || !h->nonl) {
+				if (fprintf(tmp, "\n") < 0)
+					return got_error_from_errno(
+					    "fprintf");
+			}
 			break;
 		}
 	}
@@ -398,7 +415,6 @@ patch_file(struct got_patch *p, const char *path, FILE
 	const struct got_error *err = NULL;
 	struct got_patch_hunk *h;
 	struct stat sb;
-	size_t i;
 	long lineno = 0;
 	FILE *orig;
 	off_t copypos, pos;
@@ -412,11 +428,7 @@ patch_file(struct got_patch *p, const char *path, FILE
 			return got_error(GOT_ERR_PATCH_MALFORMED);
 		if (nop)
 			return NULL;
-		for (i = 0; i < h->len; ++i) {
-			if (fprintf(tmp, "%s", h->lines[i] + 1) < 0)
-				return got_error_from_errno("fprintf");
-		}
-		return err;
+		return apply_hunk(tmp, h, &lineno);
 	}
 
 	if ((orig = fopen(path, "r")) == NULL) {
blob - deb0e8192f09bb269bb784019dc776e0602e2925
blob + 8a973bff5b5c2c99f8716e24031ce264f15f09e2
--- libexec/got-read-patch/got-read-patch.c
+++ libexec/got-read-patch/got-read-patch.c
@@ -285,7 +285,7 @@ send_line(const char *line)
 	static const struct got_error *err = NULL;
 	char *p = NULL;
 
-	if (*line != '+' && *line != '-' && *line != ' ') {
+	if (*line != '+' && *line != '-' && *line != ' ' && *line != '\\') {
 		if (asprintf(&p, " %s", line) == -1)
 			return got_error_from_errno("asprintf");
 		line = p;
@@ -301,6 +301,32 @@ send_line(const char *line)
 }
 
 static const struct got_error *
+peek_special_line(FILE *fp, int send)
+{
+	const struct got_error *err;
+	char ch;
+
+	ch = fgetc(fp);
+	if (ch != EOF && ch != '\\') {
+		ungetc(ch, fp);
+		return NULL;
+	}
+
+	if (ch == '\\' && send) {
+		err = send_line("\\");
+		if (err)
+			return err;
+	}
+
+	while (ch != EOF && ch != '\n')
+		ch = fgetc(fp);
+
+	if (ch != EOF || feof(fp))
+		return NULL;
+	return got_error(GOT_ERR_IO);
+}
+
+static const struct got_error *
 parse_hunk(FILE *fp, int *ok)
 {
 	static const struct got_error *err = NULL;
@@ -345,13 +371,15 @@ parse_hunk(FILE *fp, int *ok)
 			err = got_error(GOT_ERR_PATCH_TRUNCATED);
 			goto done;
 		}
+		if (line[linelen - 1] == '\n')
+			line[linelen - 1] = '\0';
 
 		/* usr.bin/patch allows '=' as context char */
 		if (*line == '=')
 			*line = ' ';
 
 		ch = *line;
-		if (ch == '\t' || ch == '\n')
+		if (ch == '\t' || ch == '\0')
 			ch = ' ';	/* the space got eaten */
 
 		switch (ch) {
@@ -378,6 +406,13 @@ parse_hunk(FILE *fp, int *ok)
 		err = send_line(line);
 		if (err)
 			goto done;
+
+		if ((ch == '-' && leftold == 0) ||
+		    (ch == '+' && leftnew == 0)) {
+			err = peek_special_line(fp, ch == '+');
+			if (err)
+				goto done;
+		}
 	}
 
 done:
blob - 32c852932a5cc8f51426acc413933a80b95f41de
blob + c26c65dcf84ae62b83f03b858b5db0ae3d699eba
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -1048,6 +1048,132 @@ EOF
 	test_done $testroot $ret
 }
 
+test_patch_no_newline() {
+	local testroot=`test_init patch_no_newline`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/wt/patch
+--- /dev/null
++++ eta
+@@ -0,0 +1 @@
++eta
+\ No newline at end of file
+EOF
+
+	(cd $testroot/wt && got patch patch) > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo "A  eta" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo -n eta > $testroot/wt/eta.expected
+	cmp -s $testroot/wt/eta.expected $testroot/wt/eta
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/wt/eta.expected $testroot/wt/eta
+		test_done $testroot $ret
+		return 1
+	fi
+
+	(cd $testroot/wt && got commit -m 'add eta') > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/wt/patch
+--- eta
++++ eta
+@@ -1 +1 @@
+-eta
+\ No newline at end of file
++ETA
+\ No newline at end of file
+EOF
+
+	(cd $testroot/wt && got patch patch) > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo "M  eta" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo -n ETA > $testroot/wt/eta.expected
+	cmp -s $testroot/wt/eta.expected $testroot/wt/eta
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/wt/eta.expected $testroot/wt/eta
+		test_done $testroot $ret
+		return 1
+	fi
+
+	(cd $testroot/wt && got commit -m 'edit eta') > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/wt/patch
+--- eta
++++ eta
+@@ -1 +1 @@
+-ETA
+\ No newline at end of file
++eta
+EOF
+
+	(cd $testroot/wt && got patch patch) > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo "M  eta" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo eta > $testroot/wt/eta.expected
+	cmp -s $testroot/wt/eta.expected $testroot/wt/eta
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/wt/eta.expected $testroot/wt/eta
+	fi
+	test_done $testroot $ret
+}
+
 test_parseargs "$@"
 run_test test_patch_simple_add_file
 run_test test_patch_simple_rm_file
@@ -1066,3 +1192,4 @@ run_test test_patch_nop
 run_test test_patch_preserve_perm
 run_test test_patch_create_dirs
 run_test test_patch_with_offset
+run_test test_patch_no_newline