Download raw body.
[PATCH] got histedit: make mesg behave like pick
On Tue, Oct 24, 2023 at 07:29:46PM +0200, Lorenz (xha) wrote:
> On Wed, Oct 25, 2023 at 02:36:37AM +1100, Mark Jamsek wrote:
> > Regarding the bug, we need to save the 'mesg' instruction as a 'pick' so
> > it's not reprocessed again after 'got he -c', but also launch the editor
> > to write the new message. The below diff applies on top of Lorenz's to
> > make this change, which should fix this bug and keep regress happy:
>
> i think i found a better solution to this problem, which is to just
> apply this in the main loop where we iterate over all histedit entries.
> this way we don't apply it twice since we save the commits that we've
> already done in resume_commit_id. regress is still happy and everything
> works as expected for me :-)
Seems fine to me. Nice simplification overall.
Mark, do you agreee with this new version of the diff?
> diff /home/lorenz/src/got
> commit - 41208069f572694ef8847dbe9d9a47337f7083bd
> path + /home/lorenz/src/got
> blob - 52eeeda8df9fb52a8581faa28c2d29c19ca5a2be
> file + got/got.1
> --- got/got.1
> +++ got/got.1
> @@ -3016,18 +3016,8 @@ Regular editing of history must eventually be resumed
> .It Cm fold Ar commit Ta Combine the specified commit with the next commit
> listed further below that will be used.
> .It Cm drop Ar commit Ta Remove this commit from the edited history.
> -.It Cm mesg Oo Ar log-message Oc Ta Create a new log message for the commit of
> -a preceding
> -.Cm pick
> -or
> -.Cm edit
> -command on the previous line of the histedit script.
> -The optional
> -.Ar log-message
> -argument provides a new single-line log message to use.
> -If the
> -.Ar log-message
> -argument is omitted, open an editor where a new log message can be written.
> +.It Cm mesg Ar commit Ta Open an editor to create a new log message for this
> +commit.
> .El
> .Pp
> Every commit in the history being edited must be mentioned in the script.
> blob - 73812ddbb0f2e3a0fee927fc0ae5b873c04e385a
> file + got/got.c
> --- got/got.c
> +++ got/got.c
> @@ -11696,8 +11696,7 @@ static const struct got_histedit_cmd {
> { GOT_HISTEDIT_FOLD, "fold", "combine with next commit that will "
> "be used" },
> { GOT_HISTEDIT_DROP, "drop", "remove commit from history" },
> - { GOT_HISTEDIT_MESG, "mesg",
> - "single-line log message for commit above (open editor if empty)" },
> + { GOT_HISTEDIT_MESG, "mesg", "open editor to edit the log message" },
> };
>
> struct got_histedit_list_entry {
> @@ -11760,16 +11759,11 @@ histedit_write_commit_list(struct got_object_id_queue
> histedit_cmd = "edit";
> else if (fold_only && STAILQ_NEXT(qid, entry) != NULL)
> histedit_cmd = "fold";
> + else if (edit_logmsg_only)
> + histedit_cmd = "mesg";
> err = histedit_write_commit(&qid->id, histedit_cmd, f, repo);
> if (err)
> break;
> - if (edit_logmsg_only) {
> - int n = fprintf(f, "%c\n", GOT_HISTEDIT_MESG);
> - if (n < 0) {
> - err = got_ferror(f, GOT_ERR_IO);
> - break;
> - }
> - }
> }
>
> return err;
> @@ -11986,7 +11980,7 @@ histedit_parse_list(struct got_histedit_list *histedit
> char *line = NULL, *p, *end;
> size_t i, linesize = 0;
> ssize_t linelen;
> - int lineno = 0, lastcmd = -1;
> + int lineno = 0;
> const struct got_histedit_cmd *cmd;
> struct got_object_id *commit_id = NULL;
> struct got_histedit_list_entry *hle = NULL;
> @@ -12026,37 +12020,15 @@ histedit_parse_list(struct got_histedit_list *histedit
> }
> while (isspace((unsigned char)p[0]))
> p++;
> - if (cmd->code == GOT_HISTEDIT_MESG) {
> - if (lastcmd != GOT_HISTEDIT_PICK &&
> - lastcmd != GOT_HISTEDIT_EDIT) {
> - err = got_error(GOT_ERR_HISTEDIT_CMD);
> - break;
> - }
> - if (p[0] == '\0') {
> - err = histedit_edit_logmsg(hle, repo);
> - if (err)
> - break;
> - } else {
> - hle->logmsg = strdup(p);
> - if (hle->logmsg == NULL) {
> - err = got_error_from_errno("strdup");
> - break;
> - }
> - }
> - lastcmd = cmd->code;
> - continue;
> - } else {
> - end = p;
> - while (end[0] && !isspace((unsigned char)end[0]))
> - end++;
> - *end = '\0';
> -
> - err = got_object_resolve_id_str(&commit_id, repo, p);
> - if (err) {
> - /* override error code */
> - err = histedit_syntax_error(lineno);
> - break;
> - }
> + end = p;
> + while (end[0] && !isspace((unsigned char)end[0]))
> + end++;
> + *end = '\0';
> + err = got_object_resolve_id_str(&commit_id, repo, p);
> + if (err) {
> + /* override error code */
> + err = histedit_syntax_error(lineno);
> + break;
> }
> hle = malloc(sizeof(*hle));
> if (hle == NULL) {
> @@ -12068,7 +12040,6 @@ histedit_parse_list(struct got_histedit_list *histedit
> hle->logmsg = NULL;
> commit_id = NULL;
> TAILQ_INSERT_TAIL(histedit_cmds, hle, entry);
> - lastcmd = cmd->code;
> }
>
> free(line);
> @@ -12270,15 +12241,6 @@ histedit_save_list(struct got_histedit_list *histedit_
> repo);
> if (err)
> break;
> -
> - if (hle->logmsg) {
> - int n = fprintf(f, "%c %s\n",
> - GOT_HISTEDIT_MESG, hle->logmsg);
> - if (n < 0) {
> - err = got_ferror(f, GOT_ERR_IO);
> - break;
> - }
> - }
> }
> done:
> if (f && fclose(f) == EOF && err == NULL)
> @@ -12416,6 +12378,7 @@ show_histedit_progress(struct got_commit_object *commi
> switch (hle->cmd->code) {
> case GOT_HISTEDIT_PICK:
> case GOT_HISTEDIT_EDIT:
> + case GOT_HISTEDIT_MESG:
> printf("%s -> %s: %s\n", old_id_str,
> new_id_str ? new_id_str : "no-op change", logmsg);
> break;
> @@ -13036,7 +12999,6 @@ cmd_histedit(int argc, char *argv[])
> goto done;
> continue;
> }
> -
> error = got_object_open_as_commit(&commit, repo,
> hle->commit_id);
> if (error)
> @@ -13077,13 +13039,15 @@ cmd_histedit(int argc, char *argv[])
> error = got_worktree_histedit_postpone(worktree,
> fileindex);
> goto done;
> - }
> -
> - if (hle->cmd->code == GOT_HISTEDIT_FOLD) {
> + } else if (hle->cmd->code == GOT_HISTEDIT_FOLD) {
> error = histedit_skip_commit(hle, worktree, repo);
> if (error)
> goto done;
> continue;
> + } else if (hle->cmd->code == GOT_HISTEDIT_MESG) {
> + error = histedit_edit_logmsg(hle, repo);
> + if (error)
> + goto done;
> }
>
> error = histedit_commit(&merged_paths, worktree, fileindex,
> blob - a8c20c791a546fc257c69e471cb4ff9e59cd4d66
> file + regress/cmdline/histedit.sh
> --- regress/cmdline/histedit.sh
> +++ regress/cmdline/histedit.sh
> @@ -491,13 +491,22 @@ test_histedit_fold() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "drop $old_commit2" >> $testroot/histedit-script
> echo "pick $old_commit3" >> $testroot/histedit-script
> - echo "mesg committing folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_parent_commit $testroot/repo`
> local new_commit2=`git_show_head $testroot/repo`
> @@ -603,7 +612,6 @@ test_histedit_edit() {
> fi
>
> echo "edit $old_commit1" > $testroot/histedit-script
> - echo "mesg committing changes" >> $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> @@ -650,8 +658,20 @@ test_histedit_edit() {
> fi
>
> (cd $testroot/wt && got unstage alpha > /dev/null)
> - (cd $testroot/wt && got histedit -c > $testroot/stdout)
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -c > $testroot/stdout)
> +
> local new_commit1=`git_show_parent_commit $testroot/repo`
> local new_commit2=`git_show_head $testroot/repo`
>
> @@ -771,7 +791,7 @@ test_histedit_fold_last_commit() {
> test_done "$testroot" "$ret"
> }
>
> -test_histedit_missing_commit() {
> +test_histedit_missing_commit_pick() {
> local testroot=`test_init histedit_missing_commit`
>
> local orig_commit=`git_show_head $testroot/repo`
> @@ -795,7 +815,6 @@ test_histedit_missing_commit() {
> fi
>
> echo "pick $old_commit1" > $testroot/histedit-script
> - echo "mesg committing changes" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> > $testroot/stdout 2> $testroot/stderr)
> @@ -818,6 +837,63 @@ test_histedit_missing_commit() {
> test_done "$testroot" "$ret"
> }
>
> +test_histedit_missing_commit_mesg() {
> + local testroot=`test_init histedit_missing_commit`
> +
> + local orig_commit=`git_show_head $testroot/repo`
> +
> + echo "modified alpha on master" > $testroot/repo/alpha
> + git -C $testroot/repo rm -q beta
> + echo "new file on master" > $testroot/repo/epsilon/new
> + git -C $testroot/repo add epsilon/new
> + git_commit $testroot/repo -m "committing changes"
> + local old_commit1=`git_show_head $testroot/repo`
> +
> + echo "modified zeta on master" > $testroot/repo/epsilon/zeta
> + git_commit $testroot/repo -m "committing to zeta on master"
> + local old_commit2=`git_show_head $testroot/repo`
> +
> + got checkout -c $orig_commit $testroot/repo $testroot/wt > /dev/null
> + ret=$?
> + if [ $ret -ne 0 ]; then
> + test_done "$testroot" "$ret"
> + return 1
> + fi
> +
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> + echo "mesg $old_commit1" > $testroot/histedit-script
> +
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout \
> + 2>$testroot/stderr)
> +
> + ret=$?
> + if [ $ret -eq 0 ]; then
> + echo "histedit succeeded unexpectedly" >&2
> + test_done "$testroot" "1"
> + return 1
> + fi
> +
> + echo "got: commit $old_commit2 missing from histedit script" \
> + > $testroot/stderr.expected
> +
> + cmp -s $testroot/stderr.expected $testroot/stderr
> + ret=$?
> + if [ $ret -ne 0 ]; then
> + diff -u $testroot/stderr.expected $testroot/stderr
> + fi
> + test_done "$testroot" "$ret"
> +}
> +
> test_histedit_abort() {
> local testroot=`test_init histedit_abort`
>
> @@ -845,7 +921,6 @@ test_histedit_abort() {
> touch $testroot/wt/unversioned-file
>
> echo "edit $old_commit1" > $testroot/histedit-script
> - echo "mesg committing changes" >> $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> @@ -1051,7 +1126,6 @@ test_histedit_path_prefix_edit() {
> fi
>
> echo "edit $old_commit1" > $testroot/histedit-script
> - echo "mesg modified zeta" >> $testroot/histedit-script
>
> (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> > $testroot/stdout 2> $testroot/stderr)
> @@ -1120,8 +1194,19 @@ test_histedit_path_prefix_edit() {
> test_done "$testroot" "$ret"
> return 1
> fi
> +
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/modified zeta/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
>
> - (cd $testroot/wt && got histedit -c > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -c > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
> local short_new_commit1=`trim_obj_id 28 $new_commit1`
> @@ -1244,13 +1329,23 @@ test_histedit_fold_last_commit_swap() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/committing folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> # fold commit2 into commit1 (requires swapping commits)
> echo "fold $old_commit2" > $testroot/histedit-script
> - echo "pick $old_commit1" >> $testroot/histedit-script
> - echo "mesg committing folded changes" >> $testroot/histedit-script
> + echo "mesg $old_commit1" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout \
> + 2> $testroot/stderr)
>
> ret=$?
> if [ $ret -ne 0 ]; then
> @@ -1470,13 +1565,22 @@ test_histedit_fold_add_delete() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "fold $old_commit2" >> $testroot/histedit-script
> echo "pick $old_commit3" >> $testroot/histedit-script
> - echo "mesg folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
>
> @@ -1565,12 +1669,21 @@ test_histedit_fold_edit_delete() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
> - echo "mesg folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
>
> @@ -1645,12 +1758,21 @@ test_histedit_fold_delete_add() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folded changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo "fold $old_commit1" > $testroot/histedit-script
> echo "pick $old_commit2" >> $testroot/histedit-script
> - echo "mesg folded changes" >> $testroot/histedit-script
>
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout)
> + (cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
> + VISUAL="$testroot/editor.sh" \
> + got histedit -F $testroot/histedit-script > $testroot/stdout)
>
> local new_commit1=`git_show_head $testroot/repo`
>
> @@ -2142,126 +2264,6 @@ EOF
> test_done "$testroot" $ret
> }
>
> -test_histedit_mesg_invalid() {
> - local testroot=`test_init mesg_invalid`
> -
> - local orig_commit=`git_show_head $testroot/repo`
> -
> - echo "modified alpha on master" > $testroot/repo/alpha
> - git -C $testroot/repo rm -q beta
> - echo "new file on master" > $testroot/repo/epsilon/new
> - git -C $testroot/repo add epsilon/new
> - git_commit $testroot/repo -m 'committing changes'
> - local old_commit1=`git_show_head $testroot/repo`
> -
> - echo "modified zeta on master" > $testroot/repo/epsilon/zeta
> - git_commit $testroot/repo -m 'committing to zeto on master'
> - local old_commit2=`git_show_head $testroot/repo`
> -
> - got checkout -c $orig_commit $testroot/repo $testroot/wt > /dev/null
> - ret=$?
> - if [ $ret -ne 0 ]; then
> - test_done "$testroot" $ret
> - return 1
> - fi
> -
> - # try with a leading mesg
> -
> - echo "mesg something something" > $testroot/histedit-script
> - echo "pick $old_commit1" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $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
> -
> - # try again with mesg -> mesg
> -
> - echo "pick $old_commit1" > $testroot/histedit-script
> - echo "mesg something something" >> $testroot/histedit-script
> - echo "mesg something something else" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $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
> -
> - # try again with drop -> mesg
> -
> - echo "drop $old_commit1" > $testroot/histedit-script
> - echo "mesg something something" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $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
> -
> - # try again with fold -> mesg
> -
> - echo "fold $old_commit1" > $testroot/histedit-script
> - echo "mesg something something" >> $testroot/histedit-script
> - echo "pick $old_commit2" >> $testroot/histedit-script
> -
> - (cd $testroot/wt && got histedit -F $testroot/histedit-script \
> - > $testroot/stdout 2> $testroot/stderr)
> - ret=$?
> - if [ $ret -eq 0 ]; then
> - echo "histedit succeeded unexpectedly" >&2
> - test_done "$testroot" 1
> - return 1
> - fi
> -
> - echo "got: bad histedit command" > $testroot/stderr.expected
> - cmp -s $testroot/stderr.expected $testroot/stderr
> - ret=$?
> - if [ $ret -ne 0 ]; then
> - diff -u $testroot/stderr.expected $testroot/stderr
> - fi
> - test_done "$testroot" $ret
> -}
> -
> test_histedit_resets_committer() {
> local testroot=`test_init histedit_resets_committer`
> local orig_commit=`git_show_head $testroot/repo`
> @@ -2354,13 +2356,22 @@ test_histedit_umask() {
> return 1
> fi
>
> + cat > $testroot/editor.sh <<EOF
> +#!/bin/sh
> +ed -s "\$1" <<-EOF
> + ,s/.*/folding changes/
> + w
> + EOF
> +EOF
> + chmod +x $testroot/editor.sh
> +
> echo fold $commit1 >$testroot/histedit-script
> echo fold $commit2 >>$testroot/histedit-script
> echo pick $commit3 >>$testroot/histedit-script
> - echo mesg folding changes >>$testroot/histedit-script
>
> # using a subshell to avoid clobbering global umask
> (umask 077 && cd "$testroot/wt" && \
> + env EDITOR="$testroot/editor.sh" VISUAL="$testroot/editor.sh" \
> got histedit -F "$testroot/histedit-script") >/dev/null
> ret=$?
>
> @@ -2604,7 +2615,8 @@ run_test test_histedit_drop
> run_test test_histedit_fold
> run_test test_histedit_edit
> run_test test_histedit_fold_last_commit
> -run_test test_histedit_missing_commit
> +run_test test_histedit_missing_commit_pick
> +run_test test_histedit_missing_commit_mesg
> run_test test_histedit_abort
> run_test test_histedit_path_prefix_drop
> run_test test_histedit_path_prefix_edit
> @@ -2619,7 +2631,6 @@ run_test test_histedit_fold_only
> run_test test_histedit_fold_only_empty_logmsg
> run_test test_histedit_edit_only
> run_test test_histedit_prepend_line
> -run_test test_histedit_mesg_invalid
> run_test test_histedit_resets_committer
> run_test test_histedit_umask
> run_test test_histedit_mesg_filemode_change
>
>
[PATCH] got histedit: make mesg behave like pick