got remove recursion

Hello, The diff below adds recursion to got remove. New test added and all tests pass. TODO and man page updated. Small fix for incorrect usage. I'm not really keen on the ignore_nochange arg name. If someone has a better idea, let me know. Comments? Ok? -- Tracey Emery diff 6db9f7f6b17d77932489a53452856224c8ca1645 /home/basepr1me/Documents/got/got 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 ignore_nochange; }; 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 ignore_nochange) { 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 && !ignore_nochange) 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->ignore_nochange); 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 ignore_nochange) { 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, ignore_nochange); 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 ignore_nochange) { 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, + ignore_nochange); } 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.ignore_nochange = ignore_nochange; 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,18 +2893,29 @@ done: return err; } +struct schedule_deletion_args { + struct got_worktree *worktree; + struct got_fileindex *fileindex; + const char *ondisk_path; + 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 *path = NULL; - ie = got_fileindex_entry_get(fileindex, relpath, strlen(relpath)); + ie = got_fileindex_entry_get(a->fileindex, relpath, strlen(relpath)); if (ie == NULL) return got_error(GOT_ERR_BAD_PATH); @@ -2911,37 +2926,55 @@ schedule_for_deletion(const char *ondisk_path, struct return got_error_path(relpath, GOT_ERR_FILE_STAGED); } - err = get_file_status(&status, &sb, ie, ondisk_path, repo); + err = get_file_status(&status, &sb, ie, a->ondisk_path, a->repo); if (err) return err; + if (S_ISDIR(sb.st_mode)) { + if (asprintf(&path, "%s/%s", + got_worktree_get_root_path(a->worktree), + relpath) == -1) { + err = got_error_from_errno("asprintf"); + return err; + } + + err = get_file_status(&status, &sb, ie, path, a->repo); + if (err) + return err; + } else + path = strdup(a->ondisk_path); + if (status != GOT_STATUS_NO_CHANGE) { if (status == GOT_STATUS_DELETE) return NULL; - if (status == GOT_STATUS_MODIFY && !delete_local_mods) + if (status == GOT_STATUS_MODIFY && !a->delete_local_mods) return got_error_path(relpath, GOT_ERR_FILE_MODIFIED); if (status != GOT_STATUS_MODIFY && status != GOT_STATUS_MISSING) return got_error_path(relpath, GOT_ERR_FILE_STATUS); } - if (status != GOT_STATUS_MISSING && unlink(ondisk_path) != 0) - return got_error_from_errno2("unlink", ondisk_path); + if (status != GOT_STATUS_MISSING && unlink(path) != 0) + return got_error_from_errno2("unlink", path); got_fileindex_entry_mark_deleted_from_disk(ie); - return report_file_status(ie, ondisk_path, status_cb, status_arg, repo); + + free(path); + 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,13 +2984,21 @@ 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); + sda.ondisk_path = ondisk_path; + err = worktree_status(worktree, pe->path, fileindex, repo, + schedule_for_deletion, &sda, NULL, NULL, 0, 1); free(ondisk_path); if (err) break; @@ -3519,7 +3560,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 +4473,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 +5031,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 +5346,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 +5699,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 +6155,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 +6174,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 +6525,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-11 22:36 Tracey Emery:
got remove recursion
- 2019-12-12 18:02 Stefan Sperling:
  got remove recursion
- - 2019-12-12 18:38 Stefan Sperling:
    got remove recursion