commit 60aa1fa0d1bdadc23596e5d614d184525b17935c from: Omar Polo date: Thu Mar 17 16:38:43 2022 UTC augment patch progress callback with hunks info; recover from errors Augment got_patch_progress_cb by providing the hunks that were applied with offset (or that failed) and the recoverable error encountered during the operation (bad status, missing file, ...) got_patch now proceeds when a file fails to be patched and exits with GOT_ERR_PATCH_FAILED if no other errors are encountered. While here, also add a test for the 'hunk applied with offset' case and shrink test_patch_dont_apply and illegal_status by taking advantage that 'got patch' doesn't stop at the first error. (And add some other cases to illegal_status too.) discussed with and ok stsp@ commit - 95d683408adee5188de396567e8e9746b703d7dd commit + 60aa1fa0d1bdadc23596e5d614d184525b17935c 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 + fb5211d500574e714e2964dd6a9ab1ac8433e57d --- got/got.c +++ got/got.c @@ -7186,13 +7186,31 @@ 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 status, const struct got_error *error, long old_from, + long old_lines, long new_from, long new_lines, long offset, + const struct got_error *hunk_err) { const char *path = new == NULL ? old : new; while (*path == '/') path++; - printf("%c %s\n", mode, path); + + if (status != 0) + printf("%c %s\n", status, path); + + if (error != NULL) + fprintf(stderr, "%s: %s\n", getprogname(), error->msg); + + if (offset != 0 || hunk_err != NULL) { + printf("@@ -%ld,%ld +%ld,%ld @@ ", old_from, + old_lines, new_from, new_lines); + if (hunk_err != NULL) + printf("%s\n", hunk_err->msg); + else + printf("applied with offset %ld\n", offset); + } + return NULL; } blob - 11708ccf458de5506eccdad8dafcaaed715432bb blob + 020d902fa5e38787f662f34a05d1a3b2472f3443 --- include/got_error.h +++ include/got_error.h @@ -164,8 +164,9 @@ #define GOT_ERR_FILE_BINARY 146 #define GOT_ERR_PATCH_MALFORMED 147 #define GOT_ERR_PATCH_TRUNCATED 148 -#define GOT_ERR_PATCH_DONT_APPLY 149 -#define GOT_ERR_NO_PATCH 150 +#define GOT_ERR_NO_PATCH 149 +#define GOT_ERR_HUNK_FAILED 150 +#define GOT_ERR_PATCH_FAILED 151 static const struct got_error { int code; @@ -344,8 +345,9 @@ static const struct got_error { { GOT_ERR_FILE_BINARY, "found a binary file instead of text" }, { GOT_ERR_PATCH_MALFORMED, "malformed patch" }, { GOT_ERR_PATCH_TRUNCATED, "patch truncated" }, - { GOT_ERR_PATCH_DONT_APPLY, "patch doesn't apply" }, { GOT_ERR_NO_PATCH, "no patch found" }, + { GOT_ERR_HUNK_FAILED, "hunk failed to apply" }, + { GOT_ERR_PATCH_FAILED, "patch failed to apply" }, }; /* blob - 391c0cf0a69178e759d03ba7c752062f5752dfc6 blob + 959a2129f32e5f561fc9ccfdc186d10507d45e00 --- include/got_patch.h +++ include/got_patch.h @@ -17,10 +17,12 @@ /* * 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, if an error occurred while + * applying the patch, and a hunk applied with offset or its error. */ 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_error *, + long, long, long, long, long, const struct got_error *); /* * Apply the (already opened) patch to the repository and register the blob - 58d4125553be6fd56d2408805b9c001d9c08c698 blob + acd793bdf71f41420eda39d2a713aaca2c159a43 --- lib/patch.c +++ lib/patch.c @@ -56,6 +56,8 @@ 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; @@ -65,15 +67,17 @@ struct got_patch_hunk { char **lines; }; +STAILQ_HEAD(got_patch_hunk_head, got_patch_hunk); 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 * @@ -273,7 +277,7 @@ copy(FILE *tmp, FILE *orig, off_t copypos, off_t pos) if (r != len && feof(orig)) { if (pos == -1) return NULL; - return got_error(GOT_ERR_PATCH_DONT_APPLY); + return got_error(GOT_ERR_HUNK_FAILED); } } return NULL; @@ -296,7 +300,7 @@ locate_hunk(FILE *orig, struct got_patch_hunk *h, off_ if (ferror(orig)) err = got_error_from_errno("getline"); else if (match == -1) - err = got_error(GOT_ERR_PATCH_DONT_APPLY); + err = got_error(GOT_ERR_HUNK_FAILED); break; } (*lineno)++; @@ -348,11 +352,11 @@ test_hunk(FILE *orig, struct got_patch_hunk *h) err = got_error_from_errno("getline"); else err = got_error( - GOT_ERR_PATCH_DONT_APPLY); + GOT_ERR_HUNK_FAILED); goto done; } if (strcmp(h->lines[i]+1, line)) { - err = got_error(GOT_ERR_PATCH_DONT_APPLY); + err = got_error(GOT_ERR_HUNK_FAILED); goto done; } break; @@ -433,6 +437,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_HUNK_FAILED) + h->err = err; if (err != NULL) goto done; if (!nop) @@ -442,7 +448,7 @@ patch_file(struct got_patch *p, const char *path, FILE copypos = pos; err = test_hunk(orig, h); - if (err != NULL && err->code == GOT_ERR_PATCH_DONT_APPLY) { + if (err != NULL && err->code == GOT_ERR_HUNK_FAILED) { /* * try to apply the hunk again starting the search * after the previous partial match. @@ -462,6 +468,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 +483,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_HUNK_FAILED); + err = h->err; + } else if (!nop && !feof(orig)) err = copy(tmp, orig, copypos, -1); done: @@ -571,53 +582,60 @@ check_file_status(struct got_patch *p, int file_rename } static const struct got_error * +report_progress(struct patch_args *pa, const char *old, const char *new, + unsigned char status, const struct got_error *orig_error) +{ + const struct got_error *err; + struct got_patch_hunk *h; + + err = pa->progress_cb(pa->progress_arg, old, new, status, + orig_error, 0, 0, 0, 0, 0, NULL); + if (err) + return err; + + STAILQ_FOREACH(h, pa->head, entries) { + if (h->offset == 0 && h->err == NULL) + continue; + + err = pa->progress_cb(pa->progress_arg, old, new, 0, NULL, + h->old_from, h->old_lines, h->new_from, h->new_lines, + h->offset, h->err); + if (err) + return err; + } + + return NULL; +} + +static const struct got_error * patch_delete(void *arg, unsigned char status, unsigned char staged_status, const char *path) { - struct patch_args *pa = arg; - - return pa->progress_cb(pa->progress_arg, path, NULL, status); + return report_progress(arg, path, NULL, status, NULL); } static const struct got_error * patch_add(void *arg, unsigned char status, const char *path) { - struct patch_args *pa = arg; - - return pa->progress_cb(pa->progress_arg, NULL, path, status); + return report_progress(arg, NULL, path, status, NULL); } static const struct got_error * apply_patch(struct got_worktree *worktree, struct got_repository *repo, - struct got_patch *p, int nop, struct patch_args *pa, - got_cancel_cb cancel_cb, void *cancel_arg) + struct got_pathlist_head *oldpaths, struct got_pathlist_head *newpaths, + const char *oldpath, const char *newpath, struct got_patch *p, + int nop, struct patch_args *pa, got_cancel_cb cancel_cb, void *cancel_arg) { const struct got_error *err = NULL; - struct got_pathlist_head oldpaths, newpaths; int file_renamed = 0; - char *oldpath = NULL, *newpath = NULL, *parent = NULL; - char *tmppath = NULL, *template = NULL; + char *tmppath = NULL, *template = NULL, *parent = NULL;; FILE *tmp = NULL; mode_t mode = GOT_DEFAULT_FILE_MODE; - TAILQ_INIT(&oldpaths); - TAILQ_INIT(&newpaths); + file_renamed = strcmp(oldpath, newpath); - err = build_pathlist(p->old != NULL ? p->old : p->new, &oldpath, - &oldpaths, worktree); - if (err) - goto done; - - err = build_pathlist(p->new != NULL ? p->new : p->old, &newpath, - &newpaths, worktree); - if (err) - goto done; - - if (p->old != NULL && p->new != NULL && strcmp(p->old, p->new)) - file_renamed = 1; - - err = check_file_status(p, file_renamed, worktree, repo, &oldpaths, - &newpaths, cancel_cb, cancel_arg); + err = check_file_status(p, file_renamed, worktree, repo, oldpaths, + newpaths, cancel_cb, cancel_arg); if (err) goto done; @@ -639,7 +657,7 @@ apply_patch(struct got_worktree *worktree, struct got_ goto done; if (p->old != NULL && p->new == NULL) { - err = got_worktree_schedule_delete(worktree, &oldpaths, + err = got_worktree_schedule_delete(worktree, oldpaths, 0, NULL, patch_delete, pa, repo, 0, 0); goto done; } @@ -670,17 +688,17 @@ apply_patch(struct got_worktree *worktree, struct got_ } if (file_renamed) { - err = got_worktree_schedule_delete(worktree, &oldpaths, + err = got_worktree_schedule_delete(worktree, oldpaths, 0, NULL, patch_delete, pa, repo, 0, 0); if (err == NULL) - err = got_worktree_schedule_add(worktree, &newpaths, + err = got_worktree_schedule_add(worktree, newpaths, patch_add, pa, repo, 1); } else if (p->old == NULL) - err = got_worktree_schedule_add(worktree, &newpaths, + err = got_worktree_schedule_add(worktree, newpaths, patch_add, pa, repo, 1); else - err = pa->progress_cb(pa->progress_arg, oldpath, newpath, - GOT_STATUS_MODIFY); + err = report_progress(pa, oldpath, newpath, GOT_STATUS_MODIFY, + NULL); done: if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL)) @@ -690,28 +708,53 @@ done: if (tmppath != NULL) unlink(tmppath); free(tmppath); - got_pathlist_free(&oldpaths); - got_pathlist_free(&newpaths); - free(oldpath); - free(newpath); return err; } +static const struct got_error * +resolve_paths(struct got_patch *p, struct got_worktree *worktree, + struct got_repository *repo, struct got_pathlist_head *oldpaths, + struct got_pathlist_head *newpaths, char **old, char **new) +{ + const struct got_error *err; + + TAILQ_INIT(oldpaths); + TAILQ_INIT(newpaths); + *old = NULL; + *new = NULL; + + err = build_pathlist(p->old != NULL ? p->old : p->new, old, + oldpaths, worktree); + if (err) + goto err; + + err = build_pathlist(p->new != NULL ? p->new : p->old, new, + newpaths, worktree); + if (err) + goto err; + return NULL; + +err: + free(*old); + free(*new); + got_pathlist_free(oldpaths); + got_pathlist_free(newpaths); + return err; +} + const struct got_error * got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo, int nop, got_patch_progress_cb progress_cb, void *progress_arg, got_cancel_cb cancel_cb, void *cancel_arg) { const struct got_error *err = NULL; - struct patch_args pa; + struct got_pathlist_head oldpaths, newpaths; + char *oldpath, *newpath; struct imsgbuf *ibuf; int imsg_fds[2] = {-1, -1}; - int done = 0; + int done = 0, failed = 0; pid_t pid; - pa.progress_cb = progress_cb; - pa.progress_arg = progress_arg; - ibuf = calloc(1, sizeof(*ibuf)); if (ibuf == NULL) { err = got_error_from_errno("calloc"); @@ -747,14 +790,41 @@ got_patch(int fd, struct got_worktree *worktree, struc while (!done && err == NULL) { struct got_patch p; + struct patch_args pa; + pa.progress_cb = progress_cb; + pa.progress_arg = progress_arg; + pa.head = &p.head; + err = recv_patch(ibuf, &done, &p); if (err || done) break; - err = apply_patch(worktree, repo, &p, nop, &pa, - cancel_cb, cancel_arg); + err = resolve_paths(&p, worktree, repo, &oldpaths, + &newpaths, &oldpath, &newpath); + if (err) + break; + + err = apply_patch(worktree, repo, &oldpaths, &newpaths, + oldpath, newpath, &p, nop, &pa, cancel_cb, cancel_arg); + if (err != NULL) { + failed = 1; + /* recoverable errors */ + if (err->code == GOT_ERR_FILE_STATUS || + (err->code == GOT_ERR_ERRNO && errno == ENOENT)) + err = report_progress(&pa, p.old, p.new, + GOT_STATUS_CANNOT_UPDATE, err); + else if (err->code == GOT_ERR_HUNK_FAILED) + err = report_progress(&pa, p.old, p.new, + GOT_STATUS_CANNOT_UPDATE, NULL); + } + + free(oldpath); + free(newpath); + got_pathlist_free(&oldpaths); + got_pathlist_free(&newpaths); patch_free(&p); + if (err) break; } @@ -768,5 +838,7 @@ done: err = got_error_from_errno("close"); if (imsg_fds[1] != -1 && close(imsg_fds[1]) == -1 && err == NULL) err = got_error_from_errno("close"); + if (err == NULL && failed) + err = got_error(GOT_ERR_PATCH_FAILED); return err; } blob - 92f950fb6b4fb18338a3b9edcd4318445564b3e7 blob + 65fb8dead18ae13233a8eb24e2f1f26a41d8906e --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -402,44 +402,7 @@ test_patch_dont_apply() { test_done $testroot $ret return 1 fi - - cat < $testroot/wt/patch ---- alpha -+++ alpha -@@ -1 +1,2 @@ -+hatsuseno - alpha something -EOF - echo -n > $testroot/stdout.expected - echo "got: patch doesn't apply" > $testroot/stderr.expected - - (cd $testroot/wt && got patch patch) \ - > $testroot/stdout \ - 2> $testroot/stderr - ret=$? - if [ $ret -eq 0 ]; then # should fail - test_done $testroot 1 - return 1 - fi - - cmp -s $testroot/stdout.expected $testroot/stdout - ret=$? - if [ $ret -ne 0 ]; then - diff -u $testroot/stdout.expected $testroot/stdout - test_done $testroot $ret - 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') \ >/dev/null @@ -450,6 +413,11 @@ EOF fi cat < $testroot/wt/patch +--- alpha ++++ alpha +@@ -1 +1,2 @@ ++hatsuseno + alpha something --- numbers +++ /dev/null @@ -1,9 +0,0 @@ @@ -464,18 +432,24 @@ 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 + cat < $testroot/stdout.expected +# alpha +@@ -1,1 +1,2 @@ hunk failed to apply +# numbers +@@ -1,9 +0,0 @@ hunk failed to apply +EOF + + 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 } @@ -779,75 +753,40 @@ test_patch_illegal_status() { return 1 fi - # edit an non-existent and unknown file + # try to patch an obstructed file, add a versioned one, edit a + # non existent file and an unversioned one, and remove a + # non existent file. cat < $testroot/wt/patch +--- alpha ++++ alpha +@@ -1 +1,2 @@ + alpha ++was edited +--- /dev/null ++++ beta +@@ -0,0 +1 @@ ++beta --- iota +++ iota @@ -1 +1 @@ -- iota -+ IOTA +-iota ++IOTA +--- kappa ++++ kappa +@@ -1 +1 @@ +-kappa ++KAPPA +--- lambda ++++ /dev/null +@@ -1 +0,0 @@ +-lambda EOF - (cd $testroot/wt && got patch patch) > /dev/null \ - 2> $testroot/stderr - ret=$? - if [ $ret -eq 0 ]; then - echo "edited a missing file" >&2 - test_done $testroot $ret - return 1 - fi - - echo "got: iota: No such file or directory" \ - > $testroot/stderr.expected - 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 - - # create iota and re-try - echo iota > $testroot/wt/iota - - (cd $testroot/wt && got patch patch) > /dev/null \ - 2> $testroot/stderr - ret=$? - if [ $ret -eq 0 ]; then - echo "patched an unknown file" >&2 - test_done $testroot $ret - return 1 - fi - - echo "got: iota: file has unexpected status" \ - > $testroot/stderr.expected - 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 - - rm $testroot/wt/iota - ret=$? - if [ $ret -ne 0 ]; then - test_done $testroot $ret - return 1 - fi - - # edit obstructed file + echo kappa > $testroot/wt/kappa rm $testroot/wt/alpha mkdir $testroot/wt/alpha - cat < $testroot/wt/patch ---- alpha -+++ alpha -@@ -1 +1,2 @@ - alpha -+was edited -EOF - (cd $testroot/wt && got patch patch) > /dev/null \ + (cd $testroot/wt && got patch patch) > $testroot/stdout \ 2> $testroot/stderr ret=$? if [ $ret -eq 0 ]; then @@ -856,61 +795,36 @@ EOF return 1 fi - echo "got: alpha: file has unexpected status" \ - > $testroot/stderr.expected - 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 + cat < $testroot/stdout.expected +# alpha +# beta +# iota +# kappa +# lambda +EOF - # delete an unknown file - cat < $testroot/wt/patch ---- iota -+++ /dev/null -@@ -1 +0,0 @@ --iota + cat < $testroot/stderr.expected +got: alpha: file has unexpected status +got: beta: file has unexpected status +got: iota: No such file or directory +got: kappa: file has unexpected status +got: lambda: No such file or directory +got: patch failed to apply EOF - (cd $testroot/wt && got patch patch) > /dev/null \ - 2> $testroot/stderr + cmp -s $testroot/stdout.expected $testroot/stdout ret=$? - if [ $ret -eq 0 ]; then - echo "deleted a missing file?" >&2 + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout test_done $testroot $ret return 1 fi - echo "got: iota: No such file or directory" \ - > $testroot/stderr.expected cmp -s $testroot/stderr.expected $testroot/stderr ret=$? - if [ $ret -eq 0 ]; then + if [ $ret -ne 0 ]; then diff -u $testroot/stderr.expected $testroot/stderr - test_done $testroot $ret - return 1 fi - - # try again with iota in place but still not registered - echo iota > $testroot/wt/iota - (cd $testroot/wt && got patch patch) > /dev/null \ - 2> $testroot/stderr - ret=$? - if [ $ret -eq 0 ]; then - echo "deleted an unversioned file?" >&2 - test_done $testroot $ret - return 1 - fi - - echo "got: iota: file has unexpected status" \ - > $testroot/stderr.expected - cmp -s $testroot/stderr.expected $testroot/stderr - ret=$? - if [ $ret -eq 0 ]; then - diff -u $testroot/stderr.expected $testroot/stderr - fi test_done $testroot $ret } @@ -1049,6 +963,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 tru 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/wt $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 +1050,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