Commit acf749fc600a43d8e276321e8a63cd97484f30bb

Omar Polo 2022-07-02T13:56:21

refactor the patch parser Introduce a patch_start routine that finds the next "diff" header (if there is one); the idea is to persist some state (commit id and wether it's a "git diff") while processing the content of the diff. It's needed because in the case of 'got diff' some information like the commit id are only present once at the beginning. As a consequence, the patch parser becomes slightly more robust (concatenating diffs produced by different means shouldn't confuse it anymore) and drops the support for "old" got diffs, the ones previous the introduction of the "commit -/+" header. ok tracey@

diff --git a/libexec/got-read-patch/got-read-patch.c b/libexec/got-read-patch/got-read-patch.c
index d6eea51..a48a4df 100644
--- a/libexec/got-read-patch/got-read-patch.c
+++ b/libexec/got-read-patch/got-read-patch.c
@@ -150,16 +150,61 @@ blobid(const char *line, char **blob, int git)
 }
 
 static const struct got_error *
-find_patch(int *done, FILE *fp)
+patch_start(int *git, char **cid, FILE *fp)
+{
+	const struct got_error *err = NULL;
+	char	*line = NULL;
+	size_t	 linesize = 0;
+	ssize_t	 linelen;
+
+	*git = 0;
+
+	while ((linelen = getline(&line, &linesize, fp)) != -1) {
+		if (!strncmp(line, "diff --git ", 11)) {
+			*git = 1;
+			free(*cid);
+			*cid = NULL;
+			break;
+		} else if (!strncmp(line, "diff ", 5)) {
+			*git = 0;
+			free(*cid);
+			*cid = NULL;
+		} else if (!strncmp(line, "commit - ", 9)) {
+			free(*cid);
+			err = blobid(line + 9, cid, *git);
+			if (err)
+				break;
+		} else if (!strncmp(line, "--- ", 4) ||
+		    !strncmp(line, "+++ ", 4) ||
+		    !strncmp(line, "blob - ", 7)) {
+			/* rewind to previous line */
+			if (fseeko(fp, -linelen, SEEK_CUR) == -1)
+				err = got_error_from_errno("fseeko");
+			break;
+		}
+	}
+
+	free(line);
+	if (ferror(fp) && err == NULL)
+		err = got_error_from_errno("getline");
+	if (feof(fp) && err == NULL)
+		err = got_error(GOT_ERR_NO_PATCH);
+	return err;
+}
+
+static const struct got_error *
+find_diff(int *done, int *next, FILE *fp, int git, const char *commitid)
 {
 	const struct got_error *err = NULL;
 	char	*old = NULL, *new = NULL;
-	char	*commitid = NULL, *blob = NULL;
+	char	*blob = NULL;
 	char	*line = NULL;
 	size_t	 linesize = 0;
 	ssize_t	 linelen;
-	int	 create, rename = 0, git = 0;
+	int	 create, rename = 0;
 
+	*done = 0;
+	*next = 0;
 	while ((linelen = getline(&line, &linesize, fp)) != -1) {
 		/*
 		 * Ignore the Index name like GNU and larry' patch,
@@ -186,18 +231,12 @@ find_patch(int *done, FILE *fp)
 		else if (git && !strncmp(line, "index ", 6)) {
 			free(blob);
 			err = blobid(line + 6, &blob, git);
-		} else if (!strncmp(line, "diff --git a/", 13)) {
-			git = 1;
-			free(commitid);
-			commitid = NULL;
-			free(blob);
-			blob = NULL;
-		} else if (!git && !strncmp(line, "diff ", 5)) {
-			free(commitid);
-			err = blobid(line + 5, &commitid, git);
-		} else if (!git && !strncmp(line, "commit - ", 9)) {
-			free(commitid);
-			err = blobid(line + 9, &commitid, git);
+		} else if (!strncmp(line, "diff ", 5)) {
+			/* rewind to previous line */
+			if (fseeko(fp, -linelen, SEEK_CUR) == -1)
+				err = got_error_from_errno("fseeko");
+			*next = 1;
+			break;
 		}
 
 		if (err)
@@ -236,7 +275,6 @@ find_patch(int *done, FILE *fp)
 
 	free(old);
 	free(new);
-	free(commitid);
 	free(blob);
 	free(line);
 	if (ferror(fp) && err == NULL)
@@ -480,7 +518,8 @@ read_patch(struct imsgbuf *ibuf, int fd)
 {
 	const struct got_error *err = NULL;
 	FILE *fp;
-	int patch_found = 0;
+	int git, patch_found = 0;
+	char *cid = NULL;
 
 	if ((fp = fdopen(fd, "r")) == NULL) {
 		err = got_error_from_errno("fdopen");
@@ -488,12 +527,14 @@ read_patch(struct imsgbuf *ibuf, int fd)
 		return err;
 	}
 
-	while (!feof(fp)) {
-		int done = 0;
+	while ((err = patch_start(&git, &cid, fp)) == NULL) {
+		int done, next;
 
-		err = find_patch(&done, fp);
+		err = find_diff(&done, &next, fp, git, cid);
 		if (err)
 			goto done;
+		if (next)
+			continue;
 
 		patch_found = 1;
 
@@ -510,6 +551,7 @@ read_patch(struct imsgbuf *ibuf, int fd)
 
 done:
 	fclose(fp);
+	free(cid);
 
 	/* ignore trailing gibberish */
 	if (err != NULL && err->code == GOT_ERR_NO_PATCH && patch_found)
diff --git a/regress/cmdline/patch.sh b/regress/cmdline/patch.sh
index 5190321..40309e5 100755
--- a/regress/cmdline/patch.sh
+++ b/regress/cmdline/patch.sh
@@ -1585,9 +1585,10 @@ test_patch_merge_conflict() {
 	local commit_id=`git_show_head $testroot/repo`
 
 	jot 10 | sed 's/6/six/g' > $testroot/wt/numbers
+	echo ALPHA > $testroot/wt/alpha
 
 	(cd $testroot/wt && got diff > $testroot/old.diff \
-		&& got revert numbers) >/dev/null
+		&& got revert alpha numbers) >/dev/null
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		test_done $testroot $ret
@@ -1595,7 +1596,8 @@ test_patch_merge_conflict() {
 	fi
 
 	jot 10 | sed 's/6/3+3/g' > $testroot/wt/numbers
-	(cd $testroot/wt && got commit -m 'edit numbers') \
+	jot -c 3 a > $testroot/wt/alpha
+	(cd $testroot/wt && got commit -m 'edit alpha and numbers') \
 		> /dev/null
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -1612,7 +1614,8 @@ test_patch_merge_conflict() {
 		return 1
 	fi
 
-	echo 'C  numbers' > $testroot/stdout.expected
+	echo 'C  alpha' > $testroot/stdout.expected
+	echo 'C  numbers' >> $testroot/stdout.expected
 	cmp -s $testroot/stdout $testroot/stdout.expected
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -1623,6 +1626,18 @@ test_patch_merge_conflict() {
 
 	# XXX: prefixing every line with a tab otherwise got thinks
 	# the file has conflicts in it.
+	cat <<-EOF > $testroot/wt/alpha.expected
+	<<<<<<< --- alpha
+	ALPHA
+	||||||| commit $commit_id
+	alpha
+	=======
+	a
+	b
+	c
+	>>>>>>> +++ alpha
+EOF
+
 	cat <<-EOF > $testroot/wt/numbers.expected
 	1
 	2
@@ -1642,6 +1657,14 @@ test_patch_merge_conflict() {
 	10
 EOF
 
+	cmp -s $testroot/wt/alpha $testroot/wt/alpha.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/wt/alpha $testroot/wt/alpha.expected
+		test_done $testroot $ret
+		return 1
+	fi
+
 	cmp -s $testroot/wt/numbers $testroot/wt/numbers.expected
 	ret=$?
 	if [ $ret -ne 0 ]; then