From: Omar Polo Subject: Re: add got_patch_process_cb To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Mon, 14 Mar 2022 22:48:03 +0100 Stefan Sperling wrote: > On Mon, Mar 14, 2022 at 10:18:15PM +0100, Omar Polo wrote: > > blob - 6e5ec0f2143d39ff9eeca89060522c5155706a77 > > blob + a10f54062e0e80aa2a49665b6f36f51f610d9350 > > --- lib/patch.c > > +++ lib/patch.c > > @@ -66,12 +66,19 @@ struct got_patch_hunk { > > }; > > > > struct got_patch { > > + got_patch_progress_cb cb; > > + void *arg; > > These should no longer be needed? ^ > > Oh wait, you did not go all the way. > patch_add() was not updated to use struct patch_args. More below. > > [...] > > const struct got_error * > > got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo, > > - int nop, got_worktree_delete_cb delete_cb, void *delete_arg, > > - got_worktree_checkout_cb add_cb, void *add_arg, got_cancel_cb cancel_cb, > > - void *cancel_arg) > > + 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 imsgbuf *ibuf; > > @@ -704,9 +730,10 @@ got_patch(int fd, struct got_worktree *worktree, struc > > if (err || done) > > break; > > > > + p.cb = progress_cb; > > + p.arg = progress_arg; > > p.nop = nop; > > Could we pass progress_cb, progress_arg, and nop as separate arguments > to apply_patch(), instead of stashing them into struct got_patch? yeah, I forgot half of the fixes... apologize. here's an improved version that also takes nop out of the struct got_patch. (i initially put that there to avoid propagating it through some layers of function call, but it's ugly there and better be explicit) ----------------------------------------------- commit 75ca5b38a49164eb5fc3837c68b18cbc6c14de3d from: Omar Polo date: Mon Mar 14 21:43:24 2022 UTC introduce got_patch_progress_cb in got_patch I've used callbacks that are intended for other operations. This introduce a proper got_patch specific progress callback that is invoked after processing a patch. It also drops the hackish printf in the modified case and takes `nop' out of the struct got_patch. diff 463cdc6089041a9ec1747b58910c2f9be8bb6cd7 392f9a1b74053efa310bd05d58851846beb49465 blob - d595de198f4880c3458bb395b994e889acf68df3 blob + 541f9eac1d2cad4c888bcf3da0eb9a7a1632c750 --- got/got.c +++ got/got.c @@ -7186,6 +7186,17 @@ patch_from_stdin(int *patchfd) } static const struct got_error * +patch_progress(void *arg, const char *old, const char *new, unsigned char mode) +{ + const char *path = new == NULL ? old : new; + + while (*path == '/') + path++; + printf("%c %s\n", mode, path); + return NULL; +} + +static const struct got_error * cmd_patch(int argc, char *argv[]) { const struct got_error *error = NULL, *close_error = NULL; @@ -7247,8 +7258,8 @@ cmd_patch(int argc, char *argv[]) err(1, "pledge"); #endif - error = got_patch(patchfd, worktree, repo, nop, &print_remove_status, - NULL, &add_progress, NULL, check_cancelled, NULL); + error = got_patch(patchfd, worktree, repo, nop, &patch_progress, + NULL, check_cancelled, NULL); done: if (repo) { blob - 5f28ffc619f1b4e32ba37ff8e5361c636009ba74 blob + 391c0cf0a69178e759d03ba7c752062f5752dfc6 --- include/got_patch.h +++ include/got_patch.h @@ -15,6 +15,14 @@ */ /* + * A callback that gets invoked during the patch application. + * + * Receives the old and new path and a status code. + */ +typedef const struct got_error *(*got_patch_progress_cb)(void *, + const char *, const char *, unsigned char); + +/* * Apply the (already opened) patch to the repository and register the * status of the added and removed files. * @@ -22,5 +30,4 @@ */ const struct got_error * got_patch(int, struct got_worktree *, struct got_repository *, int, - got_worktree_delete_cb, void *, got_worktree_checkout_cb, void *, - got_cancel_cb, void *); + got_patch_progress_cb, void *, got_cancel_cb, void *); blob - 6e5ec0f2143d39ff9eeca89060522c5155706a77 blob + 5f668442ac2230bb8cd7ade435e4ef6c89434c8c --- lib/patch.c +++ lib/patch.c @@ -66,12 +66,16 @@ struct got_patch_hunk { }; struct got_patch { - int nop; char *old; char *new; STAILQ_HEAD(, got_patch_hunk) head; }; +struct patch_args { + got_patch_progress_cb progress_cb; + void *progress_arg; +}; + static const struct got_error * send_patch(struct imsgbuf *ibuf, int fd) { @@ -384,7 +388,8 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long * } static const struct got_error * -patch_file(struct got_patch *p, const char *path, FILE *tmp) +patch_file(struct got_patch *p, const char *path, FILE *tmp, int nop, + struct patch_args *pa) { const struct got_error *err = NULL; struct got_patch_hunk *h; @@ -400,7 +405,7 @@ patch_file(struct got_patch *p, const char *path, FILE h = STAILQ_FIRST(&p->head); if (h == NULL || STAILQ_NEXT(h, entries) != NULL) return got_error(GOT_ERR_PATCH_MALFORMED); - if (p->nop) + if (nop) return NULL; for (i = 0; i < h->len; ++i) { if (fprintf(tmp, "%s", h->lines[i]+1) < 0) @@ -423,7 +428,7 @@ patch_file(struct got_patch *p, const char *path, FILE err = locate_hunk(orig, h, &pos, &lineno); if (err != NULL) goto done; - if (!p->nop) + if (!nop) err = copy(tmp, orig, copypos, pos); if (err != NULL) goto done; @@ -450,7 +455,7 @@ patch_file(struct got_patch *p, const char *path, FILE if (err != NULL) goto done; - if (!p->nop) + if (!nop) err = apply_hunk(tmp, h, &lineno); if (err != NULL) goto done; @@ -470,7 +475,7 @@ patch_file(struct got_patch *p, const char *path, FILE err = got_error_from_errno("fstat"); else if (sb.st_size != copypos) err = got_error(GOT_ERR_PATCH_DONT_APPLY); - } else if (!p->nop && !feof(orig)) + } else if (!nop && !feof(orig)) err = copy(tmp, orig, copypos, -1); done: @@ -565,10 +570,26 @@ check_file_status(struct got_patch *p, int file_rename } 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); +} + +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); +} + +static const struct got_error * apply_patch(struct got_worktree *worktree, struct got_repository *repo, - struct got_patch *p, got_worktree_delete_cb delete_cb, void *delete_arg, - got_worktree_checkout_cb add_cb, void *add_arg, got_cancel_cb cancel_cb, - void *cancel_arg) + 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; @@ -604,20 +625,20 @@ apply_patch(struct got_worktree *worktree, struct got_ goto done; } - if (!p->nop) + if (!nop) err = got_opentemp_named(&tmppath, &tmp, template); if (err) goto done; - err = patch_file(p, oldpath, tmp); + err = patch_file(p, oldpath, tmp, nop, pa); if (err) goto done; - if (p->nop) + if (nop) goto done; if (p->old != NULL && p->new == NULL) { err = got_worktree_schedule_delete(worktree, &oldpaths, - 0, NULL, delete_cb, delete_arg, repo, 0, 0); + 0, NULL, patch_delete, pa, repo, 0, 0); goto done; } @@ -628,15 +649,16 @@ apply_patch(struct got_worktree *worktree, struct got_ if (file_renamed) { err = got_worktree_schedule_delete(worktree, &oldpaths, - 0, NULL, delete_cb, delete_arg, repo, 0, 0); + 0, NULL, patch_delete, pa, repo, 0, 0); if (err == NULL) err = got_worktree_schedule_add(worktree, &newpaths, - add_cb, add_arg, repo, 1); + patch_add, pa, repo, 1); } else if (p->old == NULL) err = got_worktree_schedule_add(worktree, &newpaths, - add_cb, add_arg, repo, 1); + patch_add, pa, repo, 1); else - printf("M %s\n", oldpath); /* XXX */ + err = pa->progress_cb(pa->progress_arg, oldpath, newpath, + GOT_STATUS_MODIFY); done: if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL)) @@ -654,16 +676,19 @@ done: const struct got_error * got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo, - int nop, got_worktree_delete_cb delete_cb, void *delete_arg, - got_worktree_checkout_cb add_cb, void *add_arg, got_cancel_cb cancel_cb, - void *cancel_arg) + 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 imsgbuf *ibuf; int imsg_fds[2] = {-1, -1}; int done = 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"); @@ -704,9 +729,8 @@ got_patch(int fd, struct got_worktree *worktree, struc if (err || done) break; - p.nop = nop; - err = apply_patch(worktree, repo, &p, delete_cb, delete_arg, - add_cb, add_arg, cancel_cb, cancel_arg); + err = apply_patch(worktree, repo, &p, nop, &pa, + cancel_cb, cancel_arg); patch_free(&p); if (err) break;