Download raw body.
got_patch_progress_cb: handle offsets and errors too
This extends the newly added -and just committed!- got_patch_progress_cb to deal with offsets and errros too. 'got patch' was able to apply patches with offsets (i.e. at a different line then what indicated in the patch) since its addition, but never actually printed that information. Another thing that's useful is to indicate which hunk failed to be applied instead of the currently "patch doesn't apply" message. The format was suggested by stsp (thanks!) on irc a couple of days ago; see the bits in the regression suite. To do so I'm moving the struct got_patch_hunk and the definition of its head in got_patch.h and passing to the callback the STAILQ of hunks. The struct got_patch_hunk is also augmented with the `offset' and `err' field which are filled before invoking the callback. ok? P.S.: i intend to change the "patch doesn't apply" error with "hunk failed" soon, but I have another patch to send before doing it ;) P.P.S: in apply_patch I'm deliberating ignoring the error returned by the callback. It won't stay like that for too long; next diff will deal with that. (i wanted to avoid adding a conditional there only to drop it in the next commit) ----------------------------------------------- commit 6ad371bc6e8b073709e8a64cc5af6f5cbb8f48f7 (pscb) from: Omar Polo <op@omarpolo.com> date: Wed Mar 16 16:35:48 2022 UTC got_patch_progress_cb: handle offsets and errors too This makes 'got patch' print the hunks applied with offset or that failed. diff 535daea402634388631e7318cfd30537e1530cc2 b9350c67630a6f6564236281d5da950e2123275e blob - 1109e3c97f334540e101df641fdfc3423cedb552 blob + e45d19779f8e92372a5e4aa275cfc6837fc9b8e0 --- got/got.1 +++ got/got.1 @@ -1313,6 +1313,7 @@ Show the status of each affected file, using the follo .It M Ta file was modified .It D Ta file was deleted .It A Ta file was added +.It # Ta failed to patch the file .El .Pp If a change does not match at its exact line number, attempt to blob - a56994ab50aea3ed8cbe907c4d8fa2cc65f6ae14 blob + 9bd5f7c573cffd1336ab46219c217f864e713f6f --- got/got.c +++ got/got.c @@ -7186,13 +7186,32 @@ patch_from_stdin(int *patchfd) } static const struct got_error * -patch_progress(void *arg, const char *old, const char *new, unsigned char mode) +patch_progress(void *arg, const char *old, const char *new, + unsigned char mode, const struct got_patch_hunk_head *head) { + struct got_patch_hunk *h; const char *path = new == NULL ? old : new; while (*path == '/') path++; printf("%c %s\n", mode, path); + + STAILQ_FOREACH(h, head, entries) { + if (h->offset != 0 || h->err != NULL) { + printf("@@ -%ld", h->old_from); + if (h->old_lines != 1) + printf(",%ld", h->old_lines); + printf(" +%ld", h->new_from); + if (h->new_lines != 1) + printf(",%ld", h->new_lines); + printf(" @@ "); + } + if (h->offset != 0) + printf("applied with offset %ld\n", h->offset); + if (h->err != NULL) + printf("%s\n", h->err->msg); + } + return NULL; } blob - 391c0cf0a69178e759d03ba7c752062f5752dfc6 blob + 5b5f8d153e01e5595219bcda45f025ba7289bbbb --- include/got_patch.h +++ include/got_patch.h @@ -14,13 +14,28 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +STAILQ_HEAD(got_patch_hunk_head, got_patch_hunk); +struct got_patch_hunk { + STAILQ_ENTRY(got_patch_hunk) entries; + const struct got_error *err; + long offset; + long old_from; + long old_lines; + long new_from; + long new_lines; + size_t len; + size_t cap; + char **lines; +}; + /* * A callback that gets invoked during the patch application. * - * Receives the old and new path and a status code. + * Receives the old and new path, a status code and the hunks. */ typedef const struct got_error *(*got_patch_progress_cb)(void *, - const char *, const char *, unsigned char); + const char *, const char *, unsigned char, + const struct got_patch_hunk_head *); /* * Apply the (already opened) patch to the repository and register the blob - 93a9570fa38f33af30ac5d909b03d23f7c63eda3 blob + 87da8b803d66c73d3e8cbf6156ee1a0f72299292 --- lib/patch.c +++ lib/patch.c @@ -54,26 +54,16 @@ #define MIN(a, b) ((a) < (b) ? (a) : (b)) -struct got_patch_hunk { - STAILQ_ENTRY(got_patch_hunk) entries; - long old_from; - long old_lines; - long new_from; - long new_lines; - size_t len; - size_t cap; - char **lines; -}; - struct got_patch { char *old; char *new; - STAILQ_HEAD(, got_patch_hunk) head; + struct got_patch_hunk_head head; }; struct patch_args { got_patch_progress_cb progress_cb; void *progress_arg; + struct got_patch_hunk_head *head; }; static const struct got_error * @@ -344,11 +334,8 @@ test_hunk(FILE *orig, struct got_patch_hunk *h) case '-': linelen = getline(&line, &linesize, orig); if (linelen == -1) { - if (ferror(orig)) - err = got_error_from_errno("getline"); - else - err = got_error( - GOT_ERR_PATCH_DONT_APPLY); + err = got_ferror(orig, + GOT_ERR_PATCH_DONT_APPLY); goto done; } if (strcmp(h->lines[i]+1, line)) { @@ -425,6 +412,8 @@ patch_file(struct got_patch *p, const char *path, FILE tryagain: err = locate_hunk(orig, h, &pos, &lineno); + if (err != NULL && err->code == GOT_ERR_PATCH_DONT_APPLY) + h->err = err; if (err != NULL) goto done; if (!nop) @@ -445,7 +434,13 @@ patch_file(struct got_patch *p, const char *path, FILE } linelen = getline(&line, &linesize, orig); if (linelen == -1) { - err = got_error_from_errno("getline"); + if (ferror(orig)) + err = got_error_from_errno("getline"); + else { + err = got_error( + GOT_ERR_PATCH_DONT_APPLY); + h->err = err; + } goto done; } lineno++; @@ -454,6 +449,9 @@ patch_file(struct got_patch *p, const char *path, FILE if (err != NULL) goto done; + if (lineno+1 != h->old_from) + h->offset = lineno+1 - h->old_from; + if (!nop) err = apply_hunk(tmp, h, &lineno); if (err != NULL) @@ -472,8 +470,11 @@ patch_file(struct got_patch *p, const char *path, FILE if (fstat(fileno(orig), &sb) == -1) err = got_error_from_errno("fstat"); - else if (sb.st_size != copypos) - err = got_error(GOT_ERR_PATCH_DONT_APPLY); + else if (sb.st_size != copypos) { + h = STAILQ_FIRST(&p->head); + h->err = got_error(GOT_ERR_PATCH_DONT_APPLY); + err = h->err; + } } else if (!nop && !feof(orig)) err = copy(tmp, orig, copypos, -1); @@ -574,7 +575,8 @@ patch_delete(void *arg, unsigned char status, unsigned { struct patch_args *pa = arg; - return pa->progress_cb(pa->progress_arg, path, NULL, status); + return pa->progress_cb(pa->progress_arg, path, NULL, status, + pa->head); } static const struct got_error * @@ -582,7 +584,8 @@ patch_add(void *arg, unsigned char status, const char { struct patch_args *pa = arg; - return pa->progress_cb(pa->progress_arg, NULL, path, status); + return pa->progress_cb(pa->progress_arg, NULL, path, status, + pa->head); } static const struct got_error * @@ -629,6 +632,9 @@ apply_patch(struct got_worktree *worktree, struct got_ if (err) goto done; err = patch_file(p, oldpath, tmp, nop); + if (err && err->code == GOT_ERR_PATCH_DONT_APPLY) + pa->progress_cb(pa->progress_arg, oldpath, newpath, + GOT_STATUS_CANNOT_UPDATE, pa->head); if (err) goto done; @@ -657,7 +663,7 @@ apply_patch(struct got_worktree *worktree, struct got_ patch_add, pa, repo, 1); else err = pa->progress_cb(pa->progress_arg, oldpath, newpath, - GOT_STATUS_MODIFY); + GOT_STATUS_MODIFY, pa->head); done: if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL)) @@ -728,6 +734,7 @@ got_patch(int fd, struct got_worktree *worktree, struc if (err || done) break; + pa.head = &p.head; err = apply_patch(worktree, repo, &p, nop, &pa, cancel_cb, cancel_arg); patch_free(&p); blob - 41f290c529e09c20b73b0a6752c3351664a9559d blob + 605e38fe907986317b2d535e316fea2b2957166a --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -411,12 +411,10 @@ test_patch_dont_apply() { alpha something EOF - echo -n > $testroot/stdout.expected - echo "got: patch doesn't apply" > $testroot/stderr.expected + echo "# alpha" > $testroot/stdout.expected + echo "@@ -1 +1,2 @@ patch doesn't apply" >> $testroot/stdout.expected - (cd $testroot/wt && got patch patch) \ - > $testroot/stdout \ - 2> $testroot/stderr + (cd $testroot/wt && got patch patch) > $testroot/stdout 2>/dev/null ret=$? if [ $ret -eq 0 ]; then # should fail test_done $testroot 1 @@ -431,14 +429,6 @@ EOF return 1 fi - cmp -s $testroot/stderr.expected $testroot/stderr - ret=$? - if [ $ret -ne 0 ]; then - diff -u $testroot/stderr.expected $testroot/stderr - test_done $testroot $ret - return 1 - fi - # try to delete a file with a patch that doesn't match jot 100 > $testroot/wt/numbers (cd $testroot/wt && got add numbers && got commit -m 'add numbers') \ @@ -464,18 +454,21 @@ EOF -9 EOF - (cd $testroot/wt && got patch patch) > /dev/null 2> $testroot/stderr + (cd $testroot/wt && got patch patch) > $testroot/stdout 2> /dev/null ret=$? if [ $ret -eq 0 ]; then # should fail test_done $testroot 1 return 1 fi - echo "got: patch doesn't apply" > $testroot/stderr.expected - cmp -s $testroot/stderr.expected $testroot/stderr + echo "# numbers" > $testroot/stdout.expected + echo "@@ -1,9 +0,0 @@ patch doesn't apply" \ + >> $testroot/stdout.expected + + cmp -s $testroot/stdout.expected $testroot/stdout ret=$? if [ $ret -ne 0 ]; then - diff -u $testroot/stderr.expected $testroot/stderr + diff -u $testroot/stdout.expected $testroot/stdout fi test_done $testroot $ret } @@ -967,6 +960,76 @@ EOF test_done $testroot $ret } +test_patch_with_offset() { + local testroot=`test_init patch_with_offset` + + 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 +--- numbers ++++ numbers +@@ -47,7 +47,7 @@ + 47 + 48 + 49 +-50 ++midway thru it! + 51 + 52 + 53 +@@ -87,7 +87,7 @@ + 87 + 88 + 89 +-90 ++almost there! + 91 + 92 + 93 +EOF + + jot 100 > $testroot/wt/numbers + ed $testroot/wt/numbers <<EOF > /dev/null 2> /dev/null +1,10d +50r !jot 20 +w +q +EOF + + (cd $testroot/wt && got add numbers && got commit -m '+numbers') \ + > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + (cd $testroot/wt && got patch patch) > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + cat <<EOF > $testroot/stdout.expected +M numbers +@@ -47,7 +47,7 @@ applied with offset -10 +@@ -87,7 +87,7 @@ applied with offset 10 +EOF + + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi + test_done $testroot $ret +} + test_parseargs "$@" run_test test_patch_simple_add_file run_test test_patch_simple_rm_file @@ -982,3 +1045,4 @@ run_test test_patch_equals_for_context run_test test_patch_rename run_test test_patch_illegal_status run_test test_patch_nop +run_test test_patch_with_offset
got_patch_progress_cb: handle offsets and errors too