From: Omar Polo Subject: Re: got_patch_progress_cb: handle offsets and errors too To: Omar Polo Cc: gameoftrees@openbsd.org Date: Wed, 16 Mar 2022 19:30:12 +0100 Omar Polo wrote: > 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) updated diff after recent changes (i'll get to the point where 'got patch' will be able to merge these kinds of conflicts with diff3 eventually....) ----------------------------------------------- commit 2f276013a6ef2116a3b2f7e49828e51653ba22f2 (pscb) from: Omar Polo date: Wed Mar 16 18:26:02 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 03c350e0fd68967ea9bfeca0795fbd79de3dcb1b 0548581184e3531979d3a1d9a47af0df513730c0 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 - 6e4f19aa3e9703b1e7a3b57f3d76fe41a2b801ac blob + 453af5ad8978ba78acf573f3beb5d5f0d2092688 --- 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 - 58d4125553be6fd56d2408805b9c001d9c08c698 blob + 9eab8fa2fd1714e2fa85193a6843cc5480466f4d --- 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)) { @@ -433,6 +420,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) @@ -453,7 +442,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++; @@ -462,6 +457,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) @@ -474,9 +472,11 @@ patch_file(struct got_patch *p, const char *path, FILE } } - if (p->new == NULL && sb.st_size != copypos) - err = got_error(GOT_ERR_PATCH_DONT_APPLY); - else if (!nop && !feof(orig)) + if (p->new == NULL && 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); done: @@ -576,7 +576,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 * @@ -584,7 +585,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 * @@ -632,6 +634,9 @@ apply_patch(struct got_worktree *worktree, struct got_ if (err) goto done; err = patch_file(p, oldpath, tmp, nop, &mode); + 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; @@ -680,7 +685,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)) @@ -752,6 +757,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 - 92f950fb6b4fb18338a3b9edcd4318445564b3e7 blob + bba237d06710fbca86b604cfa5f598c7226cc8cf --- 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 } @@ -1049,6 +1042,76 @@ EOF test_done $testroot 0 } +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 < $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 < /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 < $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 @@ -1066,3 +1129,4 @@ run_test test_patch_illegal_status run_test test_patch_nop run_test test_patch_preserve_perm run_test test_patch_create_dirs +run_test test_patch_with_offset