commit 68ceedb3e696e840d09ca33bbc4db85d514f3d67 from: Omar Polo via: Thomas Adam date: Sun Jul 03 22:07:55 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 - fa502711bbf1a59b3e27e7fb6adc03b16a15892a commit + 68ceedb3e696e840d09ca33bbc4db85d514f3d67 blob - 3e5d28e8bf764b595e938091addf64eb8753d5ed blob + fa98b905970c2644ac714da07924df54c6939cc7 --- libexec/got-read-patch/got-read-patch.c +++ libexec/got-read-patch/got-read-patch.c @@ -149,16 +149,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, @@ -185,18 +230,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) @@ -235,7 +274,6 @@ find_patch(int *done, FILE *fp) free(old); free(new); - free(commitid); free(blob); free(line); if (ferror(fp) && err == NULL) @@ -479,7 +517,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"); @@ -487,12 +526,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; @@ -509,6 +550,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