got remove recursion

On Thu, Dec 12, 2019 at 07:02:08PM +0100, Stefan Sperling wrote: > > Maybe 'report_unchanged'? That makes more sense. > We should try to somehow improve the clarity of this code but I don't > have a good suggestion right now. Thanks for combing through the details. It makes much more sense now. Below is a diff based on the add recursion diff. I think it's greatly improved. Thoughts? -- Tracey Emery diff 6db9f7f6b17d77932489a53452856224c8ca1645 /home/basepr1me/Documents/got/got_rmrc blob - 6b4ec9a6c5c0f9240144d60d19c72d8af64f0b0e file + TODO --- TODO +++ TODO @@ -11,7 +11,6 @@ lib: got: - 'histedit -c' prompts for log message even if there are no changes to commit -- recursive removal: got rm -R tog: - implement horizonal scrolling in all views blob - fa001c2db61fe9127ca82232eb555991984c9dcb file + got/got.1 --- got/got.1 +++ got/got.1 @@ -645,7 +645,7 @@ With -R, add files even if they match a .Cm got status ignore pattern. .El -.It Cm remove Ar file-path ... +.It Cm remove Oo Fl R Oc Ar file-path ... Remove versioned files from a work tree and schedule them for deletion from the repository in the next commit. .Pp @@ -655,6 +655,13 @@ are as follows: .Bl -tag -width Ds .It Fl f Perform the operation even if a file contains uncommitted modifications. +.It Fl R +Permit recursion into directories. +If this option is not specified, +.Cm got remove +will refuse to run if a specified +.Ar path +is a directory. .El .It Cm rm Short alias for blob - 67836e9dac99a1e361945334537edee3a628ccf6 file + got/got.c --- got/got.c +++ got/got.c @@ -4276,16 +4276,17 @@ done: __dead static void usage_remove(void) { - fprintf(stderr, "usage: %s remove [-f] file-path ...\n", getprogname()); + fprintf(stderr, "usage: %s remove [-f] [-R] file-path ...\n", + getprogname()); exit(1); } static const struct got_error * print_remove_status(void *arg, unsigned char status, - unsigned char staged_status, const char *path, - struct got_object_id *blob_id, struct got_object_id *staged_blob_id, - struct got_object_id *commit_id) + unsigned char staged_status, const char *path) { + while (path[0] == '/') + path++; if (status == GOT_STATUS_NONEXISTENT) return NULL; if (status == staged_status && (status == GOT_STATUS_DELETE)) @@ -4303,17 +4304,20 @@ cmd_remove(int argc, char *argv[]) char *cwd = NULL; struct got_pathlist_head paths; struct got_pathlist_entry *pe; - int ch, delete_local_mods = 0; + int ch, delete_local_mods = 0, can_recurse = 0; TAILQ_INIT(&paths); - while ((ch = getopt(argc, argv, "f")) != -1) { + while ((ch = getopt(argc, argv, "fR")) != -1) { switch (ch) { case 'f': delete_local_mods = 1; break; + case 'R': + can_recurse = 1; + break; default: - usage_add(); + usage_remove(); /* NOTREACHED */ } } @@ -4351,6 +4355,35 @@ cmd_remove(int argc, char *argv[]) error = get_worktree_paths_from_argv(&paths, argc, argv, worktree); if (error) goto done; + + if (!can_recurse) { + char *ondisk_path; + struct stat sb; + TAILQ_FOREACH(pe, &paths, entry) { + if (asprintf(&ondisk_path, "%s/%s", + got_worktree_get_root_path(worktree), + pe->path) == -1) { + error = got_error_from_errno("asprintf"); + goto done; + } + if (lstat(ondisk_path, &sb) == -1) { + if (errno == ENOENT) { + free(ondisk_path); + continue; + } + error = got_error_from_errno2("lstat", + ondisk_path); + free(ondisk_path); + goto done; + } + free(ondisk_path); + if (S_ISDIR(sb.st_mode)) { + error = got_error_msg(GOT_ERR_BAD_PATH, + "removing directories requires -R option"); + goto done; + } + } + } error = got_worktree_schedule_delete(worktree, &paths, delete_local_mods, print_remove_status, NULL, repo); blob - 33c5e5f8529ba9bf1fcf626666817f9317db7d02 file + include/got_worktree.h --- include/got_worktree.h +++ include/got_worktree.h @@ -104,6 +104,10 @@ const struct got_error *got_worktree_set_base_commit_i typedef const struct got_error *(*got_worktree_checkout_cb)(void *, unsigned char, const char *); +/* A callback function which is invoked when a path is removed. */ +typedef const struct got_error *(*got_worktree_delete_cb)(void *, + unsigned char, unsigned char, const char *); + /* * Attempt to check out files into a work tree from its associated repository * and path prefix, and update the work tree's file index accordingly. @@ -168,7 +172,7 @@ const struct got_error *got_worktree_schedule_add(stru */ const struct got_error * got_worktree_schedule_delete(struct got_worktree *, - struct got_pathlist_head *, int, got_worktree_status_cb, void *, + struct got_pathlist_head *, int, got_worktree_delete_cb, void *, struct got_repository *); /* A callback function which is used to select or reject a patch. */ blob - a1011c791c8036dbbd7c2f29f5cd7ca8ecafb60c file + lib/worktree.c --- lib/worktree.c +++ lib/worktree.c @@ -2308,12 +2308,13 @@ struct diff_dir_cb_arg { void *cancel_arg; /* A pathlist containing per-directory pathlists of ignore patterns. */ struct got_pathlist_head ignores; + int report_unchanged; }; static const struct got_error * report_file_status(struct got_fileindex_entry *ie, const char *abspath, got_worktree_status_cb status_cb, void *status_arg, - struct got_repository *repo) + struct got_repository *repo, int report_unchanged) { const struct got_error *err = NULL; unsigned char status = GOT_STATUS_NO_CHANGE; @@ -2328,7 +2329,7 @@ report_file_status(struct got_fileindex_entry *ie, con return err; if (status == GOT_STATUS_NO_CHANGE && - staged_status == GOT_STATUS_NO_CHANGE) + staged_status == GOT_STATUS_NO_CHANGE && !report_unchanged) return NULL; if (got_fileindex_entry_has_blob(ie)) { @@ -2377,7 +2378,7 @@ status_old_new(void *arg, struct got_fileindex_entry * } err = report_file_status(ie, abspath, a->status_cb, a->status_arg, - a->repo); + a->repo, a->report_unchanged); free(abspath); return err; } @@ -2608,7 +2609,7 @@ status_new(void *arg, struct dirent *de, const char *p static const struct got_error * report_single_file_status(const char *path, const char *ondisk_path, struct got_fileindex *fileindex, got_worktree_status_cb status_cb, -void *status_arg, struct got_repository *repo) +void *status_arg, struct got_repository *repo, int report_unchanged) { struct got_fileindex_entry *ie; struct stat sb; @@ -2616,7 +2617,7 @@ void *status_arg, struct got_repository *repo) ie = got_fileindex_entry_get(fileindex, path, strlen(path)); if (ie) return report_file_status(ie, ondisk_path, status_cb, - status_arg, repo); + status_arg, repo, report_unchanged); if (lstat(ondisk_path, &sb) == -1) { if (errno != ENOENT) @@ -2637,7 +2638,8 @@ static const struct got_error * worktree_status(struct got_worktree *worktree, const char *path, struct got_fileindex *fileindex, struct got_repository *repo, got_worktree_status_cb status_cb, void *status_arg, - got_cancel_cb cancel_cb, void *cancel_arg, int no_ignores) + got_cancel_cb cancel_cb, void *cancel_arg, int no_ignores, + int report_unchanged) { const struct got_error *err = NULL; DIR *workdir = NULL; @@ -2655,7 +2657,8 @@ worktree_status(struct got_worktree *worktree, const c err = got_error_from_errno2("opendir", ondisk_path); else err = report_single_file_status(path, ondisk_path, - fileindex, status_cb, status_arg, repo); + fileindex, status_cb, status_arg, repo, + report_unchanged); } else { fdiff_cb.diff_old_new = status_old_new; fdiff_cb.diff_old = status_old; @@ -2669,6 +2672,7 @@ worktree_status(struct got_worktree *worktree, const c arg.status_arg = status_arg; arg.cancel_cb = cancel_cb; arg.cancel_arg = cancel_arg; + arg.report_unchanged = report_unchanged; TAILQ_INIT(&arg.ignores); if (!no_ignores) { err = add_ignores(&arg.ignores, worktree->root_path, @@ -2706,7 +2710,7 @@ got_worktree_status(struct got_worktree *worktree, TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, - status_cb, status_arg, cancel_cb, cancel_arg, 0); + status_cb, status_arg, cancel_cb, cancel_arg, 0, 0); if (err) break; } @@ -2871,7 +2875,7 @@ got_worktree_schedule_add(struct got_worktree *worktre return got_error_from_errno("asprintf"); saa.ondisk_path = ondisk_path; err = worktree_status(worktree, pe->path, fileindex, repo, - schedule_addition, &saa, NULL, NULL, no_ignores); + schedule_addition, &saa, NULL, NULL, no_ignores, 0); free(ondisk_path); if (err) break; @@ -2889,59 +2893,90 @@ done: return err; } +struct schedule_deletion_args { + struct got_worktree *worktree; + struct got_fileindex *fileindex; + got_worktree_delete_cb progress_cb; + void *progress_arg; + struct got_repository *repo; + int delete_local_mods; +}; + static const struct got_error * -schedule_for_deletion(const char *ondisk_path, struct got_fileindex *fileindex, - const char *relpath, int delete_local_mods, - got_worktree_status_cb status_cb, void *status_arg, - struct got_repository *repo) +schedule_for_deletion(void *arg, unsigned char status, + unsigned char staged_status, const char *relpath, + struct got_object_id *blob_id, struct got_object_id *staged_blob_id, + struct got_object_id *commit_id) { + struct schedule_deletion_args *a = arg; const struct got_error *err = NULL; struct got_fileindex_entry *ie = NULL; - unsigned char status, staged_status; struct stat sb; + char *ondisk_path; - ie = got_fileindex_entry_get(fileindex, relpath, strlen(relpath)); - if (ie == NULL) - return got_error(GOT_ERR_BAD_PATH); + if (asprintf(&ondisk_path, "%s/%s", a->worktree->root_path, + relpath) == -1) + return got_error_from_errno("asprintf"); + ie = got_fileindex_entry_get(a->fileindex, relpath, strlen(relpath)); + if (ie == NULL) { + err = got_error(GOT_ERR_BAD_PATH); + goto done; + } + staged_status = get_staged_status(ie); if (staged_status != GOT_STATUS_NO_CHANGE) { if (staged_status == GOT_STATUS_DELETE) - return NULL; - return got_error_path(relpath, GOT_ERR_FILE_STAGED); + goto done; + err = got_error_path(relpath, GOT_ERR_FILE_STAGED); + goto done; } - err = get_file_status(&status, &sb, ie, ondisk_path, repo); + err = get_file_status(&status, &sb, ie, ondisk_path, a->repo); if (err) - return err; + goto done; if (status != GOT_STATUS_NO_CHANGE) { if (status == GOT_STATUS_DELETE) - return NULL; - if (status == GOT_STATUS_MODIFY && !delete_local_mods) - return got_error_path(relpath, GOT_ERR_FILE_MODIFIED); + goto done; + if (status == GOT_STATUS_MODIFY && !a->delete_local_mods) { + err = got_error_path(relpath, GOT_ERR_FILE_MODIFIED); + goto done; + } if (status != GOT_STATUS_MODIFY && - status != GOT_STATUS_MISSING) - return got_error_path(relpath, GOT_ERR_FILE_STATUS); + status != GOT_STATUS_MISSING) { + err = got_error_path(relpath, GOT_ERR_FILE_STATUS); + goto done; + } } - if (status != GOT_STATUS_MISSING && unlink(ondisk_path) != 0) - return got_error_from_errno2("unlink", ondisk_path); + if (status != GOT_STATUS_MISSING && unlink(ondisk_path) != 0) { + err = got_error_from_errno2("unlink", ondisk_path); + goto done; + } got_fileindex_entry_mark_deleted_from_disk(ie); - return report_file_status(ie, ondisk_path, status_cb, status_arg, repo); +done: + free(ondisk_path); + if (err) + return err; + if (status == GOT_STATUS_DELETE) + return NULL; + return (*a->progress_cb)(a->progress_arg, GOT_STATUS_DELETE, + staged_status, relpath); } const struct got_error * got_worktree_schedule_delete(struct got_worktree *worktree, struct got_pathlist_head *paths, int delete_local_mods, - got_worktree_status_cb status_cb, void *status_arg, + got_worktree_delete_cb progress_cb, void *progress_arg, struct got_repository *repo) { struct got_fileindex *fileindex = NULL; char *fileindex_path = NULL; const struct got_error *err = NULL, *sync_err, *unlockerr; struct got_pathlist_entry *pe; + struct schedule_deletion_args sda; err = lock_worktree(worktree, LOCK_EX); if (err) @@ -2951,14 +2986,16 @@ got_worktree_schedule_delete(struct got_worktree *work if (err) goto done; + sda.worktree = worktree; + sda.fileindex = fileindex; + sda.progress_cb = progress_cb; + sda.progress_arg = progress_arg; + sda.repo = repo; + sda.delete_local_mods = delete_local_mods; + TAILQ_FOREACH(pe, paths, entry) { - char *ondisk_path; - if (asprintf(&ondisk_path, "%s/%s", worktree->root_path, - pe->path) == -1) - return got_error_from_errno("asprintf"); - err = schedule_for_deletion(ondisk_path, fileindex, pe->path, - delete_local_mods, status_cb, status_arg, repo); - free(ondisk_path); + err = worktree_status(worktree, pe->path, fileindex, repo, + schedule_for_deletion, &sda, NULL, NULL, 0, 1); if (err) break; } @@ -3519,7 +3556,7 @@ got_worktree_revert(struct got_worktree *worktree, rfa.repo = repo; TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, - revert_file, &rfa, NULL, NULL, 0); + revert_file, &rfa, NULL, NULL, 0, 0); if (err) break; } @@ -4432,7 +4469,7 @@ got_worktree_commit(struct got_object_id **new_commit_ cc_arg.have_staged_files = have_staged_files; TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, - collect_commitables, &cc_arg, NULL, NULL, 0); + collect_commitables, &cc_arg, NULL, NULL, 0, 0); if (err) goto done; } @@ -4990,13 +5027,14 @@ rebase_commit(struct got_object_id **new_commit_id, } TAILQ_FOREACH(pe, merged_paths, entry) { err = worktree_status(worktree, pe->path, fileindex, - repo, collect_commitables, &cc_arg, NULL, NULL, 0); + repo, collect_commitables, &cc_arg, NULL, NULL, 0, + 0); if (err) goto done; } } else { err = worktree_status(worktree, "", fileindex, repo, - collect_commitables, &cc_arg, NULL, NULL, 0); + collect_commitables, &cc_arg, NULL, NULL, 0, 0); if (err) goto done; } @@ -5304,7 +5342,7 @@ got_worktree_rebase_abort(struct got_worktree *worktre rfa.patch_arg = NULL; rfa.repo = repo; err = worktree_status(worktree, "", fileindex, repo, - revert_file, &rfa, NULL, NULL, 0); + revert_file, &rfa, NULL, NULL, 0, 0); if (err) goto sync; @@ -5657,7 +5695,7 @@ got_worktree_histedit_abort(struct got_worktree *workt rfa.patch_arg = NULL; rfa.repo = repo; err = worktree_status(worktree, "", fileindex, repo, - revert_file, &rfa, NULL, NULL, 0); + revert_file, &rfa, NULL, NULL, 0, 0); if (err) goto sync; @@ -6113,7 +6151,7 @@ got_worktree_stage(struct got_worktree *worktree, oka.have_changes = 0; TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, - check_stage_ok, &oka, NULL, NULL, 0); + check_stage_ok, &oka, NULL, NULL, 0, 0); if (err) goto done; } @@ -6132,7 +6170,7 @@ got_worktree_stage(struct got_worktree *worktree, spa.staged_something = 0; TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, - stage_path, &spa, NULL, NULL, 0); + stage_path, &spa, NULL, NULL, 0, 0); if (err) goto done; } @@ -6483,7 +6521,7 @@ got_worktree_unstage(struct got_worktree *worktree, upa.patch_arg = patch_arg; TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, - unstage_path, &upa, NULL, NULL, 0); + unstage_path, &upa, NULL, NULL, 0, 0); if (err) goto done; } blob - 03c8d8fc147c4298c5425973fe32318e931e8992 file + regress/cmdline/rm.sh --- regress/cmdline/rm.sh +++ regress/cmdline/rm.sh @@ -193,7 +193,57 @@ function test_rm_and_add_elsewhere { test_done "$testroot" "$ret" } +function test_rm_directory { + local testroot=`test_init rm_directory` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret="$?" + if [ "$ret" != "0" ]; then + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got rm . > $testroot/stdout 2> $testroot/stderr) + ret="$?" + echo "got: removing directories requires -R option" \ + > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + echo -n > $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got rm -R . > $testroot/stdout) + + echo 'D alpha' > $testroot/stdout.expected + echo 'D beta' >> $testroot/stdout.expected + echo 'D epsilon/zeta' >> $testroot/stdout.expected + echo 'D gamma/delta' >> $testroot/stdout.expected + + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + test_done "$testroot" "$ret" +} + run_test test_rm_basic run_test test_rm_with_local_mods run_test test_double_rm run_test test_rm_and_add_elsewhere +run_test test_rm_directory

2019-12-12 18:38 Stefan Sperling:
got remove recursion
- 2019-12-12 20:08 Tracey Emery:
  got remove recursion
2019-12-12 20:37 Tracey Emery:
got remove recursion
- 2019-12-12 20:51 Tracey Emery:
  got remove recursion
- - 2019-12-12 23:33 Stefan Sperling:
    got remove recursion