From: Stefan Sperling Subject: removing got rebase/histedit backups To: gameoftrees@openbsd.org Date: Thu, 15 Jul 2021 16:02:14 +0200 Following up on the discussion about 'gotadmin cleanup' and how references could be deleted: My initial idea was to offer an interface via 'got ref', such as: got ref -d -R refs/got/backups I am concerned about the possibility of somebody running commands like 'got ref -d -R refs'. This would delete all references except the symbolic HEAD reference and render the repository unusable. Having a command which can break the repository is asking for trouble. And trying to catch mistakes on the user's command line is not trivial. So I would rather avoid adding recursive removal to 'got ref'. If someone wants to break a repository in this way, they'll have to delete all the references one-by-one ;) My new plan is to add removal functionality to specific commands which create a lot of references. The patch below starts off with 'got rebase' and 'got histedit'. Regarding the option name, I want an option letter which is not yet in use by any command. This way, other commands can use the same option letter when they add the same feature. One example would be 'got fetch'. We may want to delete references under "refs/remotes/" which belong to a remote repository that is no longer reachable, for example. I ended up choosing the option -X, which is not used anywhere yet. So we could now remove backups with: got histedit -X And this also works for specific branches only: got histedit -X main The patch is relatively small because my implementation re-uses the logic which is already present for listing existing backups. The commands print all references which were deleted and their values. This makes it possible to re-create references which were deleted in error, as long as the output is preserved. I've made a small related change to check for cancellation while looping over references. This catches the case where someone changes their mind after starting a -X command and hits Ctrl-C. ok? diff 739c7757611e77d1f83847e58167b8df51d19cb8 85ae0b977286b125144bddd85552518464382e97 blob - 905badf9f77075dc5c72caa5308358990d308e9b blob + 64eb89a0f0d09c013a42191b548de64e1ecc96c7 --- got/got.1 +++ got/got.1 @@ -1437,7 +1437,7 @@ conflicts must be resolved first. .It Cm bo Short alias for .Cm backout . -.It Cm rebase Oo Fl a Oc Oo Fl c Oc Oo Fl l Oc Op Ar branch +.It Cm rebase Oo Fl a Oc Oo Fl c Oc Oo Fl l Oc Oo Fl X Oc Op Ar branch Rebase commits on the specified .Ar branch onto the tip of the current branch of the work tree. @@ -1585,11 +1585,34 @@ If this option is used, does not require a work tree. None of the other options can be used together with .Fl l . +.It Fl X +Delete backups created by past rebase operations, represented by references +in the +.Dq refs/got/backup/rebase +reference namespace. +.Pp +If a +.Ar branch +is specified, only delete backups which at some point in time represented +this branch. +Otherwise, delete all references found within +.Dq refs/got/backup/rebase . +.Pp +Any commit, tree, tag, and blob objects belonging to deleted backups +remain in the repository and may be removed separately with +Git's garbage collector or +.Cm gotadmin cleanup . +.Pp +If this option is used, +.Cm got rebase +does not require a work tree. +None of the other options can be used together with +.Fl X . .El .It Cm rb Short alias for .Cm rebase . -.It Cm histedit Oo Fl a Oc Oo Fl c Oc Oo Fl f Oc Oo Fl F Ar histedit-script Oc Oo Fl m Oc Oo Fl l Oc Op Ar branch +.It Cm histedit Oo Fl a Oc Oo Fl c Oc Oo Fl f Oc Oo Fl F Ar histedit-script Oc Oo Fl m Oc Oo Fl l Oc Oo Fl X Oc Op Ar branch Edit commit history between the work tree's current base commit and the tip commit of the work tree's current branch. .Pp @@ -1785,6 +1808,29 @@ If this option is used, does not require a work tree. None of the other options can be used together with .Fl l . +.It Fl X +Delete backups created by past histedit operations, represented by references +in the +.Dq refs/got/backup/histedit +reference namespace. +.Pp +If a +.Ar branch +is specified, only delete backups which at some point in time represented +this branch. +Otherwise, delete all references found within +.Dq refs/got/backup/histedit . +.Pp +Any commit, tree, tag, and blob objects belonging to deleted backups +remain in the repository and may be removed separately with +Git's garbage collector or +.Cm gotadmin cleanup . +.Pp +If this option is used, +.Cm got histedit +does not require a work tree. +None of the other options can be used together with +.Fl X . .El .It Cm he Short alias for blob - 36887d987d05638e632528887359fa9921003127 blob + 8dc4d8f2f703bc7e275e8efeece9dd5690dc546f --- got/got.c +++ got/got.c @@ -7518,7 +7518,7 @@ done: __dead static void usage_rebase(void) { - fprintf(stderr, "usage: %s rebase [-a] [-c] [-l] [branch]\n", + fprintf(stderr, "usage: %s rebase [-a] [-c] [-l] [-X] [branch]\n", getprogname()); exit(1); } @@ -7834,6 +7834,27 @@ done: } static const struct got_error * +delete_backup_ref(struct got_reference *ref, struct got_object_id *id, + struct got_repository *repo) +{ + const struct got_error *err; + char *id_str; + + err = got_object_id_str(&id_str, id); + if (err) + return err; + + err = got_ref_delete(ref, repo); + if (err) + goto done; + + printf("Deleted %s: %s\n", got_ref_get_name(ref), id_str); +done: + free(id_str); + return err; +} + +static const struct got_error * print_backup_ref(const char *branch_name, const char *new_id_str, struct got_object_id *old_commit_id, struct got_commit_object *old_commit, struct got_reflist_object_id_map *refs_idmap, @@ -7929,8 +7950,8 @@ done: } static const struct got_error * -list_backup_refs(const char *backup_ref_prefix, const char *wanted_branch_name, - struct got_repository *repo) +process_backup_refs(const char *backup_ref_prefix, const char *wanted_branch_name, + int delete, struct got_repository *repo) { const struct got_error *err; struct got_reflist_head refs, backup_refs; @@ -7967,6 +7988,10 @@ list_backup_refs(const char *backup_ref_prefix, const const char *refname = got_ref_get_name(re->ref); char *slash; + err = check_cancelled(NULL); + if (err) + break; + err = got_ref_resolve(&old_commit_id, repo, re->ref); if (err) break; @@ -7997,8 +8022,14 @@ list_backup_refs(const char *backup_ref_prefix, const if (wanted_branch_name == NULL || strcmp(wanted_branch_name, branch_name) == 0) { wanted_branch_found = 1; - err = print_backup_ref(branch_name, refname, - old_commit_id, old_commit, refs_idmap, repo); + if (delete) { + err = delete_backup_ref(re->ref, + old_commit_id, repo); + } else { + err = print_backup_ref(branch_name, refname, + old_commit_id, old_commit, refs_idmap, + repo); + } if (err) break; } @@ -8043,6 +8074,7 @@ cmd_rebase(int argc, char *argv[]) struct got_commit_object *commit = NULL; int ch, rebase_in_progress = 0, abort_rebase = 0, continue_rebase = 0; int histedit_in_progress = 0, create_backup = 1, list_backups = 0; + int delete_backups = 0; unsigned char rebase_status = GOT_STATUS_NO_CHANGE; struct got_object_id_queue commits; struct got_pathlist_head merged_paths; @@ -8052,7 +8084,7 @@ cmd_rebase(int argc, char *argv[]) STAILQ_INIT(&commits); TAILQ_INIT(&merged_paths); - while ((ch = getopt(argc, argv, "acl")) != -1) { + while ((ch = getopt(argc, argv, "aclX")) != -1) { switch (ch) { case 'a': abort_rebase = 1; @@ -8063,6 +8095,9 @@ cmd_rebase(int argc, char *argv[]) case 'l': list_backups = 1; break; + case 'X': + delete_backups = 1; + break; default: usage_rebase(); /* NOTREACHED */ @@ -8082,8 +8117,19 @@ cmd_rebase(int argc, char *argv[]) option_conflict('l', 'a'); if (continue_rebase) option_conflict('l', 'c'); + if (delete_backups) + option_conflict('l', 'X'); if (argc != 0 && argc != 1) usage_rebase(); + } else if (delete_backups) { + if (abort_rebase) + option_conflict('X', 'a'); + if (continue_rebase) + option_conflict('X', 'c'); + if (list_backups) + option_conflict('l', 'X'); + if (argc != 0 && argc != 1) + usage_rebase(); } else { if (abort_rebase && continue_rebase) usage_rebase(); @@ -8101,7 +8147,7 @@ cmd_rebase(int argc, char *argv[]) } error = got_worktree_open(&worktree, cwd); if (error) { - if (list_backups) { + if (list_backups || delete_backups) { if (error->code != GOT_ERR_NOT_WORKTREE) goto done; } else { @@ -8122,9 +8168,10 @@ cmd_rebase(int argc, char *argv[]) if (error) goto done; - if (list_backups) { - error = list_backup_refs(GOT_WORKTREE_REBASE_BACKUP_REF_PREFIX, - argc == 1 ? argv[0] : NULL, repo); + if (list_backups || delete_backups) { + error = process_backup_refs( + GOT_WORKTREE_REBASE_BACKUP_REF_PREFIX, + argc == 1 ? argv[0] : NULL, delete_backups, repo); goto done; /* nothing else to do */ } @@ -8359,7 +8406,8 @@ __dead static void usage_histedit(void) { fprintf(stderr, "usage: %s histedit [-a] [-c] [-f] " - "[-F histedit-script] [-m] [-l] [branch]\n", getprogname()); + "[-F histedit-script] [-m] [-l] [-X] [branch]\n", + getprogname()); exit(1); } @@ -9193,7 +9241,7 @@ cmd_histedit(int argc, char *argv[]) struct got_update_progress_arg upa; int edit_in_progress = 0, abort_edit = 0, continue_edit = 0; int edit_logmsg_only = 0, fold_only = 0; - int list_backups = 0; + int list_backups = 0, delete_backups = 0; const char *edit_script_path = NULL; unsigned char rebase_status = GOT_STATUS_NO_CHANGE; struct got_object_id_queue commits; @@ -9208,7 +9256,7 @@ cmd_histedit(int argc, char *argv[]) TAILQ_INIT(&merged_paths); memset(&upa, 0, sizeof(upa)); - while ((ch = getopt(argc, argv, "acfF:ml")) != -1) { + while ((ch = getopt(argc, argv, "acfF:mlX")) != -1) { switch (ch) { case 'a': abort_edit = 1; @@ -9228,6 +9276,9 @@ cmd_histedit(int argc, char *argv[]) case 'l': list_backups = 1; break; + case 'X': + delete_backups = 1; + break; default: usage_histedit(); /* NOTREACHED */ @@ -9269,8 +9320,25 @@ cmd_histedit(int argc, char *argv[]) option_conflict('l', 'm'); if (fold_only) option_conflict('l', 'f'); + if (delete_backups) + option_conflict('l', 'X'); if (argc != 0 && argc != 1) usage_histedit(); + } else if (delete_backups) { + if (abort_edit) + option_conflict('X', 'a'); + if (continue_edit) + option_conflict('X', 'c'); + if (edit_script_path) + option_conflict('X', 'F'); + if (edit_logmsg_only) + option_conflict('X', 'm'); + if (fold_only) + option_conflict('X', 'f'); + if (list_backups) + option_conflict('X', 'l'); + if (argc != 0 && argc != 1) + usage_histedit(); } else if (argc != 0) usage_histedit(); @@ -9290,7 +9358,7 @@ cmd_histedit(int argc, char *argv[]) } error = got_worktree_open(&worktree, cwd); if (error) { - if (list_backups) { + if (list_backups || delete_backups) { if (error->code != GOT_ERR_NOT_WORKTREE) goto done; } else { @@ -9301,7 +9369,7 @@ cmd_histedit(int argc, char *argv[]) } } - if (list_backups) { + if (list_backups || delete_backups) { error = got_repo_open(&repo, worktree ? got_worktree_get_repo_path(worktree) : cwd, NULL); @@ -9311,9 +9379,9 @@ cmd_histedit(int argc, char *argv[]) worktree ? got_worktree_get_root_path(worktree) : NULL); if (error) goto done; - error = list_backup_refs( + error = process_backup_refs( GOT_WORKTREE_HISTEDIT_BACKUP_REF_PREFIX, - argc == 1 ? argv[0] : NULL, repo); + argc == 1 ? argv[0] : NULL, delete_backups, repo); goto done; /* nothing else to do */ } blob - b6f5b85dd337c44468c9ff11b1cf6976705b46fb blob + 15eedc6e738a474075643efa1fcd4692f2e2cec1 --- regress/cmdline/histedit.sh +++ regress/cmdline/histedit.sh @@ -171,7 +171,38 @@ EOF ret="$?" if [ "$ret" != "0" ]; then diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 fi + + (cd $testroot/repo && got histedit -X master \ + > $testroot/stdout 2> $testroot/stderr) + echo -n "Deleted refs/got/backup/histedit/master/$new_commit2: " \ + > $testroot/stdout.expected + echo "$old_commit2" >> $testroot/stdout.expected + echo -n > $testroot/stderr.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 + 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 + + (cd $testroot/repo && got histedit -l > $testroot/stdout) + echo -n > $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi test_done "$testroot" "$ret" } blob - a1f81c4a5bbd0d9e60e07289ec4e53d119b78dd6 blob + d590abf4efb9a74b11e35c01ec82cfdfad3c7893 --- regress/cmdline/rebase.sh +++ regress/cmdline/rebase.sh @@ -186,12 +186,45 @@ EOF if [ "$ret" != "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" != "0" ]; then diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 fi + + # Delete all backup refs + (cd $testroot/repo && got rebase -X \ + > $testroot/stdout 2> $testroot/stderr) + echo -n "Deleted refs/got/backup/rebase/newbranch/$new_commit2: " \ + > $testroot/stdout.expected + echo "$orig_commit2" >> $testroot/stdout.expected + echo -n > $testroot/stderr.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 + 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 + + (cd $testroot/repo && got rebase -l > $testroot/stdout) + echo -n > $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi test_done "$testroot" "$ret" }