commit acf749fc600a43d8e276321e8a63cd97484f30bb from: Omar Polo date: Sat Jul 02 13:56:21 2022 UTC 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@ commit - f0032ce63b4f4f035e5f7894a406a96931f99f3f commit + acf749fc600a43d8e276321e8a63cd97484f30bb blob - d6eea51f8d3203ae05b110adaac04ee330bd4d28 blob + a48a4df951d4896c4fd29a461f3c17002b01bc3e --- libexec/got-read-patch/got-read-patch.c +++ 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 *old = NULL, *new = NULL; - char *commitid = NULL, *blob = NULL; char *line = NULL; size_t linesize = 0; ssize_t linelen; - int create, rename = 0, git = 0; + + *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 *blob = NULL; + char *line = NULL; + size_t linesize = 0; + ssize_t linelen; + 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) blob - 51903218ccbe2f14d00ba02be9bbc8ead7c2f1e4 blob + 40309e5b82bddf39321e6da00a91b79dd7ec65b0 --- regress/cmdline/patch.sh +++ 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