# [RFC PATCH 1/6] Documentation: document post-rewrite hook 34 messages from 2010-02-14 to 2010-02-20. Participants: Thomas Rast, Johan Herland, Johannes Sixt, Junio C Hamano. Thread: https://gitlist.dev/t/22652 ## Thomas Rast, 2010-02-14 16:17 Subject: [RFC PATCH 0/6] post-rewrite hook and copying notes Message-ID: URL: https://gitlist.dev/e/cover.1266164150.git.trast%40student.ethz.ch ``` This series implements a post-rewrite hook. This has been on my todo list since we first got notes, but I never got around to it. My original intended use-case was to add a note for the source of the rewritten commit, but recent discussion on git-notes was that we should add some feature that lets users copy over their notes when rewriting commits. The hook in the last patch, together with the 'git notes copy' command now in 'pu', can achieve this. I'm not sure whether we should stop here and declare it the user's responsibility to enable the hook though. My idea of a "full integration" approach would be to instead teach 'git notes copy' to read the same type of lists that I'm handing to post-rewrite, and then wire calls to it into the scripts in the same places that call post-rewrite. Opinions? Thomas Rast (6): Documentation: document post-rewrite hook commit --amend: invoke post-rewrite hook filter-branch: invoke post-rewrite hook rebase: invoke post-rewrite hook rebase -i: invoke post-rewrite hook contrib: add a hook that copies notes over rewrites Documentation/githooks.txt | 36 +++++++ builtin-commit.c | 43 ++++++++ contrib/hooks/post-rewrite-copy-notes | 37 +++++++ git-am.sh | 10 ++ git-filter-branch.sh | 10 ++ git-rebase--interactive.sh | 41 +++++++- git-rebase.sh | 5 + t/t5407-post-rewrite-hook.sh | 180 +++++++++++++++++++++++++++++++++ 8 files changed, 361 insertions(+), 1 deletions(-) create mode 100644 contrib/hooks/post-rewrite-copy-notes create mode 100755 t/t5407-post-rewrite-hook.sh ``` ## Thomas Rast, 2010-02-14 16:17 Subject: [RFC PATCH 1/6] Documentation: document post-rewrite hook Message-ID: <508ef869b6ac0b7240d6608e24a791777686c610.1266164150.git.trast@student.ethz.ch> URL: https://gitlist.dev/e/508ef869b6ac0b7240d6608e24a791777686c610.1266164150.git.trast%40student.ethz.ch In-Reply-To: ``` This defines the behaviour of the post-rewrite hook support, which will be implemented in the following patches. We deliberately do not document how often the hook will be invoked per rewriting command, but the interface is designed to keep that at "once". This would currently not matter too much, since both rebase and filter-branch are shellscripts and spawn many processes anyway. However, when a fast sequencer in C is implemented, it will be beneficial to only have to run the hook once. Signed-off-by: Thomas Rast --- Documentation/githooks.txt | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 87e2c03..41895e9 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -317,6 +317,42 @@ This hook is invoked by 'git gc --auto'. It takes no parameter, and exiting with non-zero status from this script causes the 'git gc --auto' to abort. +post-rewrite +~~~~~~~~~~~~ + +This hook is invoked by commands that rewrite commits (`git commit +--amend`, 'git-rebase', 'git-filter-branch'). Its first argument +denotes the command it was invoked by: currently one of `amend`, +`rebase`, or `filter-branch`. Further command-dependent arguments may +be passed in the future. + +The hook receives a list of the rewritten commits on stdin, in the +format + + SP [ SP ] LF + +The 'extra-info' is again command-dependent. If it is empty, the +preceding SP is also omitted. Currently, no commands pass any +'extra-info'. + +The following command-specific comments apply: + +rebase:: + For the 'squash' and 'fixup' operation, all commits that were + squashed are listed as being rewritten to the squashed commit. + This means that there will be several lines sharing the same + 'new-sha1'. + +filter-branch:: + Commits that were processed by 'git-filter-branch', but not + changed, are not included in the list. If the list is empty + after this filtering, the hook is not invoked at all. + +There is no default 'post-rewrite' hook, but see the +`post-receive-copy-notes` script in `contrib/hooks` for an example +that copies your git-notes to the rewritten commits. + + GIT --- Part of the linkgit:git[1] suite -- 1.7.0.216.g74d8e ``` ## Thomas Rast, 2010-02-14 16:17 Subject: [RFC PATCH 2/6] commit --amend: invoke post-rewrite hook Message-ID: URL: https://gitlist.dev/e/f2a61f3f01c39bb20de020d2e7454417624e4d23.1266164150.git.trast%40student.ethz.ch In-Reply-To: ``` The rough structure of run_rewrite_hook() comes from run_receive_hook() in receive-pack. We introduce a --no-post-rewrite option and use it to avoid the hook when called from git-rebase -i 'edit'. The next patch will add full support in git-rebase, and we only want to invoke the hook once. Signed-off-by: Thomas Rast --- builtin-commit.c | 43 ++++++++++++++++++++++++++++++++++ git-rebase--interactive.sh | 2 +- t/t5407-post-rewrite-hook.sh | 52 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletions(-) create mode 100755 t/t5407-post-rewrite-hook.sh diff --git a/builtin-commit.c b/builtin-commit.c index 55676fd..9b67649 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -66,6 +66,7 @@ static char *author_name, *author_email, *author_date; static int all, edit_flag, also, interactive, only, amend, signoff; static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship; +static int no_post_rewrite; static char *untracked_files_arg, *force_date; /* * The default commit message cleanup mode will remove the lines @@ -137,6 +138,7 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset) OPT_BOOLEAN('z', "null", &null_termination, "terminate entries with NUL"), OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"), + OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, "bypass post-rewrite hook"), { OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, "mode", "show untracked files, optional modes: all, normal, no. (Default: all)", PARSE_OPT_OPTARG, NULL, (intptr_t)"all" }, OPT_BOOLEAN(0, "allow-empty", &allow_empty, "ok to record an empty change"), /* end commit contents options */ @@ -1160,6 +1162,40 @@ static int git_commit_config(const char *k, const char *v, void *cb) return git_status_config(k, v, s); } +static const char post_rewrite_hook[] = "hooks/post-rewrite"; + +static int run_rewrite_hook(const unsigned char *oldsha1, + const unsigned char *newsha1) +{ + /* oldsha1 SP newsha1 LF NUL */ + static char buf[2*40 + 3]; + struct child_process proc; + const char *argv[3]; + int code; + size_t n; + + if (access(git_path(post_rewrite_hook), X_OK) < 0) + return 0; + + argv[0] = git_path(post_rewrite_hook); + argv[1] = "amend"; + argv[2] = NULL; + + memset(&proc, 0, sizeof(proc)); + proc.argv = argv; + proc.in = -1; + proc.stdout_to_stderr = 1; + + code = start_command(&proc); + if (code) + return code; + n = snprintf(buf, sizeof(buf), "%s %s\n", + sha1_to_hex(oldsha1), sha1_to_hex(newsha1)); + write_in_full(proc.in, buf, n); + close(proc.in); + return finish_command(&proc); +} + int cmd_commit(int argc, const char **argv, const char *prefix) { struct strbuf sb = STRBUF_INIT; @@ -1303,6 +1339,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix) rerere(0); run_hook(get_index_file(), "post-commit", NULL); + if (amend && !no_post_rewrite) { + struct commit *commit; + commit = lookup_commit(head_sha1); + if (!commit || parse_commit(commit)) + die("HEAD commit disappeared right under my eyes?"); + run_rewrite_hook(commit->object.sha1, commit_sha1); + } if (!quiet) print_summary(prefix, commit_sha1); diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 3e4fd14..5735859 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -445,7 +445,7 @@ do_next () { mark_action_done pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" - git commit --amend + git commit --amend --no-post-rewrite ;; edit|e) comment_for_reflog edit diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh new file mode 100755 index 0000000..1020af9 --- /dev/null +++ b/t/t5407-post-rewrite-hook.sh @@ -0,0 +1,52 @@ +#!/bin/sh +# +# Copyright (c) 2010 Thomas Rast +# + +test_description='Test the post-rewrite hook.' +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit A foo A && + test_commit B foo B && + test_commit C foo C && + test_commit D foo D +' + +mkdir .git/hooks + +cat >.git/hooks/post-rewrite < "$TRASH_DIRECTORY"/post-rewrite.args +cat > "$TRASH_DIRECTORY"/post-rewrite.data +EOF +chmod u+x .git/hooks/post-rewrite + +clear_hook_input () { + rm -f post-rewrite.args post-rewrite.data +} + +verify_hook_input () { + test_cmp "$TRASH_DIRECTORY"/post-rewrite.args expected.args && + test_cmp "$TRASH_DIRECTORY"/post-rewrite.data expected.data +} + +test_expect_success 'git commit --amend' ' + clear_hook_input && + echo "D new message" > newmsg && + oldsha=$(git rev-parse HEAD^0) && + git commit -Fnewmsg --amend && + echo amend > expected.args && + echo $oldsha $(git rev-parse HEAD^0) > expected.data && + verify_hook_input +' + +test_expect_success 'git commit --amend --no-post-rewrite' ' + clear_hook_input && + echo "D new message again" > newmsg && + git commit --no-post-rewrite -Fnewmsg --amend && + test ! -f post-rewrite.args && + test ! -f post-rewrite.data +' + +test_done -- 1.7.0.216.g74d8e ``` ## Thomas Rast, 2010-02-14 16:17 Subject: [RFC PATCH 3/6] filter-branch: invoke post-rewrite hook Message-ID: <0ad4b8c1a5377d697513cd8e49f64419cd8deef4.1266164150.git.trast@student.ethz.ch> URL: https://gitlist.dev/e/0ad4b8c1a5377d697513cd8e49f64419cd8deef4.1266164150.git.trast%40student.ethz.ch In-Reply-To: ``` Signed-off-by: Thomas Rast --- git-filter-branch.sh | 10 ++++++++++ t/t5407-post-rewrite-hook.sh | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 0 deletions(-) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 88fb0f0..531cc8e 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -358,6 +358,10 @@ while read commit parents; do @SHELL_PATH@ -c "$filter_commit" "git commit-tree" \ $(git write-tree) $parentstr < ../message > ../map/$commit || die "could not write rewritten commit" + new_commit=$(map $commit) + if test $commit != $new_commit; then + echo $commit $new_commit >> "$workdir"/../rewritten + fi done <../revs # If we are filtering for paths, as in the case of a subdirectory @@ -484,6 +488,12 @@ if [ "$filter_tag_name" ]; then fi cd ../.. + +if test -x "$GIT_DIR"/hooks/post-rewrite && + test -s "$workdir"/../rewritten; then + "$GIT_DIR"/hooks/post-rewrite filter-branch < "$workdir"/../rewritten +fi + rm -rf "$tempdir" trap - 0 diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index 1020af9..ff23ebf 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -49,4 +49,24 @@ test_expect_success 'git commit --amend --no-post-rewrite' ' test ! -f post-rewrite.data ' +test_expect_success 'git filter-branch' ' + git reset --hard D && + clear_hook_input && + git filter-branch -f --tree-filter "touch newfile" B..HEAD && + echo filter-branch >expected.args && + cat >expected.data < URL: https://gitlist.dev/e/8493a921273c327c9da61d1e0ce887ce5e45dd5a.1266164150.git.trast%40student.ethz.ch In-Reply-To: ``` We have to deal with two separate code paths: a normal rebase, which actually goes through git-am; and rebase {-m|-s}. The only small issue with both is that they need to remember the original sha1 across a possible conflict resolution. rebase -m already puts this information in $dotest/current, and we just introduce a similar file for git-am. Note that in git-am, the hook really only runs when coming from git-rebase: the code path that sets the $dotest/original-commit file is guarded by a test for $dotest/rebasing. Signed-off-by: Thomas Rast --- git-am.sh | 10 ++++++++++ git-rebase.sh | 5 +++++ t/t5407-post-rewrite-hook.sh | 30 ++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 0 deletions(-) diff --git a/git-am.sh b/git-am.sh index 3c08d53..56428b4 100755 --- a/git-am.sh +++ b/git-am.sh @@ -575,6 +575,7 @@ do echo "Patch is empty. Was it split wrong?" stop_here $this } + rm -f "$dotest/original-commit" if test -f "$dotest/rebasing" && commit=$(sed -e 's/^From $[0-9a-f]*$ .*/\1/' \ -e q "$dotest/$msgnum") && @@ -582,6 +583,7 @@ do then git cat-file commit "$commit" | sed -e '1,/^$/d' >"$dotest/msg-clean" + echo "$commit" > "$dotest/original-commit" else { sed -n '/^Subject/ s/Subject: //p' "$dotest/info" @@ -768,6 +770,10 @@ do git update-ref -m "$GIT_REFLOG_ACTION: $FIRSTLINE" HEAD $commit $parent || stop_here $this + if test -f "$dotest/original-commit"; then + echo "$(cat "$dotest/original-commit") $commit" >> "$dotest/rewritten" + fi + if test -x "$GIT_DIR"/hooks/post-applypatch then "$GIT_DIR"/hooks/post-applypatch @@ -776,6 +782,10 @@ do go_next done +if test -s "$dotest"/rewritten && test -x "$GIT_DIR"/hooks/post-rewrite; then + "$GIT_DIR"/hooks/post-rewrite rebase < "$dotest"/rewritten +fi + git gc --auto rm -fr "$dotest" diff --git a/git-rebase.sh b/git-rebase.sh index fb4fef7..52f8b9b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -79,6 +79,7 @@ continue_merge () { then printf "Committed: %0${prec}d " $msgnum fi + echo "$cmt $(git rev-parse HEAD^0)" >> "$dotest/rewritten" else if test -z "$GIT_QUIET" then @@ -151,6 +152,10 @@ move_to_original_branch () { finish_rb_merge () { move_to_original_branch + if test -x "$GIT_DIR"/hooks/post-rewrite && + test -s "$dotest"/rewritten; then + "$GIT_DIR"/hooks/post-rewrite rebase < "$dotest"/rewritten + fi rm -r "$dotest" say All done. } diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index ff23ebf..d6438a7 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -69,4 +69,34 @@ test_expect_success 'git filter-branch (no-op)' ' test ! -f post-rewrite.data ' +test_expect_success 'git rebase' ' + git reset --hard D && + clear_hook_input && + test_must_fail git rebase --onto A B && + echo C > foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < URL: https://gitlist.dev/e/b32eedebe7dceced1f1714d4a3ffd064fe74ef36.1266164150.git.trast%40student.ethz.ch In-Reply-To: ``` Aside from the same issue that rebase also has (remembering the original commit across a conflict resolution), rebase -i brings an extra twist: We need to defer writing the rewritten list in the case of {squash,fixup} because their rewritten result should be the last commit in the squashed group. Signed-off-by: Thomas Rast --- git-rebase--interactive.sh | 39 +++++++++++++++++++++ t/t5407-post-rewrite-hook.sh | 78 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 0 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 5735859..fc4f7e9 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -96,6 +96,13 @@ AUTHOR_SCRIPT="$DOTEST"/author-script # command is processed, this file is deleted. AMEND="$DOTEST"/amend +# For the post-rewrite hook, we make a list of rewritten commits and +# their new sha1s. The rewritten-pending list keeps the sha1s of +# commits that have been processed, but not committed yet, +# e.g. because they are waiting for a 'squash' command. +REWRITTEN_LIST="$DOTEST"/rewritten-list +REWRITTEN_PENDING="$DOTEST"/rewritten-pending + PRESERVE_MERGES= STRATEGY= ONTO= @@ -198,6 +205,7 @@ make_patch () { } die_with_patch () { + echo "$1" > "$DOTEST"/conflicted-sha make_patch "$1" git rerere die "$2" @@ -348,6 +356,7 @@ pick_one_preserving_merges () { printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG die_with_patch $sha1 "Error redoing merge $sha1" fi + echo "$sha1 $(git rev-parse HEAD^0)" >> "$REWRITTEN_LIST" ;; *) output git cherry-pick "$@" || @@ -425,6 +434,26 @@ die_failed_squash() { die_with_patch $1 "" } +flush_rewritten_pending() { + test -s "$REWRITTEN_PENDING" || return + newsha1="$(git rev-parse HEAD^0)" + sed "s/$/ $newsha1/" < "$REWRITTEN_PENDING" >> "$REWRITTEN_LIST" + rm -f "$REWRITTEN_PENDING" +} + +record_in_rewritten() { + oldsha1="$(git rev-parse $1)" + echo "$oldsha1" >> "$REWRITTEN_PENDING" + + case "$(peek_next_command)" in + squash|s|fixup|f) + ;; + *) + flush_rewritten_pending + ;; + esac +} + do_next () { rm -f "$MSG" "$AUTHOR_SCRIPT" "$AMEND" || exit read command sha1 rest < "$TODO" @@ -438,6 +467,7 @@ do_next () { mark_action_done pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" + record_in_rewritten $sha1 ;; reword|r) comment_for_reflog reword @@ -446,6 +476,7 @@ do_next () { pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" git commit --amend --no-post-rewrite + record_in_rewritten $sha1 ;; edit|e) comment_for_reflog edit @@ -454,6 +485,7 @@ do_next () { pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" make_patch $sha1 + record_in_rewritten "$sha1" git rev-parse --verify HEAD > "$AMEND" warn "Stopped at $sha1... $rest" warn "You can amend the commit now, with" @@ -509,6 +541,7 @@ do_next () { rm -f "$SQUASH_MSG" "$FIXUP_MSG" ;; esac + record_in_rewritten $sha1 ;; *) warn "Unknown command: $command $sha1 $rest" @@ -537,6 +570,11 @@ do_next () { test ! -f "$DOTEST"/verbose || git diff-tree --stat $(cat "$DOTEST"/head)..HEAD } && + if test -x "$GIT_DIR"/hooks/post-rewrite && + test -s "$REWRITTEN_LIST"; then + "$GIT_DIR"/hooks/post-rewrite rebase < "$REWRITTEN_LIST" + true # we don't care if this hook failed + fi && rm -rf "$DOTEST" && git gc --auto && warn "Successfully rebased and updated $HEADNAME." @@ -685,6 +723,7 @@ first and then run 'git rebase --continue' again." test -n "$amend" && git reset --soft $amend die "Could not commit staged changes." } + record_in_rewritten "$(cat "$DOTEST"/conflicted-sha)" fi require_clean_work_tree diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index d6438a7..d3ac469 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -99,4 +99,82 @@ EOF verify_hook_input ' +test_expect_success 'git rebase -m' ' + git reset --hard D && + clear_hook_input && + test_must_fail git rebase -m --onto A B && + echo C > foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < URL: https://gitlist.dev/e/fba55036b8c07f1ee10c4d8f7ed22045277aeb21.1266164150.git.trast%40student.ethz.ch In-Reply-To: ``` This hook can be used to copy your notes across rewrites. You must also choose across which type of rewrite you want to copy them, or just enable all of them by saying git config hooks.rewriteCopyNotes all Signed-off-by: Thomas Rast --- contrib/hooks/post-rewrite-copy-notes | 37 +++++++++++++++++++++++++++++++++ 1 files changed, 37 insertions(+), 0 deletions(-) create mode 100644 contrib/hooks/post-rewrite-copy-notes diff --git a/contrib/hooks/post-rewrite-copy-notes b/contrib/hooks/post-rewrite-copy-notes new file mode 100644 index 0000000..73d6a82 --- /dev/null +++ b/contrib/hooks/post-rewrite-copy-notes @@ -0,0 +1,37 @@ +#!/bin/sh + +# This hook copies over the git-notes annotation from the +# pre-rewritten commit to the post-rewritten one. It only does so if +# the invoking command is listed in hooks.rewriteCopyNotes (which must +# be space separated). If hooks.rewriteCopyNotesVerbose is set, it +# lists the commits for which 'git notes copy' was successful on +# stderr. + +type="$1" +run= + +enabled="$(git config --get hooks.rewriteCopyNotes)" +if test "$enabled" = all || test "$enabled" = true; then + enabled="amend rebase filter-branch" +fi + +for enabled_type in $enabled; do + test "$enabled_type" = "$type" && run=t +done + +if test -z "$run"; then + exit +done + +verbose=$(git config --get hooks.rewriteCopyNotesVerbose) +test "$verbose" = true && echo "Copying notes:" >2 +end_msg="No commit had any notes." + +while read pre post extra; do + git notes copy "$pre" "$post" 2>/dev/null && + end_msg="Done." && + test "$verbose" = true && + echo " $pre -> $post" >2 +done + +test "$verbose" = true && echo "$end_msg" >2 -- 1.7.0.216.g74d8e ``` ## Thomas Rast, 2010-02-14 16:21 Subject: Re: [RFC PATCH 6/6] contrib: add a hook that copies notes over rewrites Message-ID: <201002141721.16165.trast@student.ethz.ch> URL: https://gitlist.dev/e/201002141721.16165.trast%40student.ethz.ch In-Reply-To: ``` On Sunday 14 February 2010 17:17:11 Thomas Rast wrote: > +if test -z "$run"; then > + exit > +done Argh, that's what I get for writing lots of tests except for the PoC hook. This should of course say 'fi' instead of 'done'. Sorry. -- Thomas Rast trast@{inf,student}.ethz.ch ``` ## Thomas Rast, 2010-02-14 21:46 Subject: [PATCH] WIP: git notes copy --stdin Message-ID: <0b241a335d71bdf56a07a712cb5506e1b1e1db6f.1266183704.git.trast@student.ethz.ch> URL: https://gitlist.dev/e/0b241a335d71bdf56a07a712cb5506e1b1e1db6f.1266183704.git.trast%40student.ethz.ch In-Reply-To: ``` This implements a 'git notes copy --stdin' mode suitable to read the data fed to the post-rewrite hook. --- Ok, so I changed my mind and decided to implement the "make it built-in and configurable" way. This helper should work. I spent some time trying to refactor cmd_notes() into something that can nicely handle commands that do not fit the normal scheme, but eventually was too tired to continue and just crafted it in the existing code in the right place. If nobody beats me to it I'll pick this up later this week, but tomorrow will be too busy to even think about it. builtin-notes.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 56 insertions(+), 0 deletions(-) diff --git a/builtin-notes.c b/builtin-notes.c index 123ecad..3cd9e45 100644 --- a/builtin-notes.c +++ b/builtin-notes.c @@ -278,6 +278,52 @@ int commit_notes(struct notes_tree *t, const char *msg) return 0; } +int notes_copy_from_stdin(int force) +{ + struct strbuf buf = STRBUF_INIT; + struct notes_tree *t; + + init_notes(NULL, NULL, NULL, 0); + t = &default_notes_tree; + + while (strbuf_getline(&buf, stdin, '\n') != EOF) { + unsigned char from_obj[20], to_obj[20]; + struct strbuf **split; + const unsigned char *note; + + split = strbuf_split(&buf, ' '); + if (!split[0] || !split[1]) + die("Malformed input line: '%s'.", buf.buf); + strbuf_rtrim(split[0]); + strbuf_rtrim(split[1]); + if (get_sha1(split[0]->buf, from_obj)) + die("Failed to resolve '%s' as a valid ref.", split[0]->buf); + if (get_sha1(split[1]->buf, to_obj)) + die("Failed to resolve '%s' as a valid ref.", split[1]->buf); + + note = get_note(t, from_obj); + if (!force) { + const unsigned char *existing_note = get_note(t, to_obj); + if (existing_note) { + error("Cannot copy notes. Found existing notes for object" + " %s. Use '-f' to overwrite existing notes", + sha1_to_hex(to_obj)); + return 1; + } + } + + if (note) + add_note(t, to_obj, note, combine_notes_overwrite); + else + remove_note(t, to_obj); + + strbuf_list_free(split); + } + + commit_notes(t, "Notes added by 'git notes copy'"); + return 0; +} + int cmd_notes(int argc, const char **argv, const char *prefix) { struct notes_tree *t; @@ -301,6 +347,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix) OPT_CALLBACK('C', "reuse-message", &msg, "OBJECT", "reuse specified note object", parse_reuse_arg), OPT_BOOLEAN('f', "force", &force, "replace existing notes"), + OPT_BOOLEAN(0, "stdin", &force, "read objects from stdin"), OPT_END() }; @@ -351,6 +398,15 @@ int cmd_notes(int argc, const char **argv, const char *prefix) if (copy) { const char *from_ref; + if (stdin) { + if (argc > 2) { + error("too few parameters"); + usage_with_options(git_notes_usage, options); + } else { + retval = notes_copy_from_stdin(force); + goto end; + } + } if (argc < 3) { error("too few parameters"); usage_with_options(git_notes_usage, options); -- 1.7.0.224.g30f23.dirty ``` ## Johan Herland, 2010-02-15 01:25 Subject: Re: [PATCH] WIP: git notes copy --stdin Message-ID: <201002150225.52953.johan@herland.net> URL: https://gitlist.dev/e/201002150225.52953.johan%40herland.net In-Reply-To: <0b241a335d71bdf56a07a712cb5506e1b1e1db6f.1266183704.git.trast@student.ethz.ch> ``` On Sunday 14 February 2010, Thomas Rast wrote: > This implements a 'git notes copy --stdin' mode suitable to read the > data fed to the post-rewrite hook. > --- > > Ok, so I changed my mind and decided to implement the "make it > built-in and configurable" way. This helper should work. I spent > some time trying to refactor cmd_notes() into something that can > nicely handle commands that do not fit the normal scheme, but > eventually was too tired to continue and just crafted it in the > existing code in the right place. > > If nobody beats me to it I'll pick this up later this week, but > tomorrow will be too busy to even think about it. > > > builtin-notes.c | 56 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, > 56 insertions(+), 0 deletions(-) > > diff --git a/builtin-notes.c b/builtin-notes.c > index 123ecad..3cd9e45 100644 > --- a/builtin-notes.c > +++ b/builtin-notes.c > @@ -278,6 +278,52 @@ int commit_notes(struct notes_tree *t, const char > *msg) return 0; > } > > +int notes_copy_from_stdin(int force) > +{ > + struct strbuf buf = STRBUF_INIT; > + struct notes_tree *t; > + > + init_notes(NULL, NULL, NULL, 0); > + t = &default_notes_tree; As you say, cmd_notes() should be refactored, so that you don't have to duplicate all of this (and some of the below stuff). > + while (strbuf_getline(&buf, stdin, '\n') != EOF) { > + unsigned char from_obj[20], to_obj[20]; > + struct strbuf **split; > + const unsigned char *note; > + > + split = strbuf_split(&buf, ' '); > + if (!split[0] || !split[1]) > + die("Malformed input line: '%s'.", buf.buf); > + strbuf_rtrim(split[0]); > + strbuf_rtrim(split[1]); > + if (get_sha1(split[0]->buf, from_obj)) > + die("Failed to resolve '%s' as a valid ref.", split[0]->buf); > + if (get_sha1(split[1]->buf, to_obj)) > + die("Failed to resolve '%s' as a valid ref.", split[1]->buf); > + > + note = get_note(t, from_obj); > + if (!force) { > + const unsigned char *existing_note = get_note(t, to_obj); > + if (existing_note) { > + error("Cannot copy notes. Found existing notes for object" > + " %s. Use '-f' to overwrite existing notes", > + sha1_to_hex(to_obj)); > + return 1; > + } > + } > + > + if (note) > + add_note(t, to_obj, note, combine_notes_overwrite); > + else > + remove_note(t, to_obj); > + > + strbuf_list_free(split); > + } > + > + commit_notes(t, "Notes added by 'git notes copy'"); We (this goes for the existing commit_notes() call in cmd_notes() as well) should either check the return value for commit_notes(), or (since it currently only returns 0) we should change commit_notes() to void instead of int. Actually, the correct solution is probably to "return error()" instead of "die()" inside commit_notes(), and then check the return value. > + return 0; > +} > + > int cmd_notes(int argc, const char **argv, const char *prefix) > { > struct notes_tree *t; > @@ -301,6 +347,7 @@ int cmd_notes(int argc, const char **argv, const char > *prefix) OPT_CALLBACK('C', "reuse-message", &msg, "OBJECT", > "reuse specified note object", parse_reuse_arg), > OPT_BOOLEAN('f', "force", &force, "replace existing notes"), > + OPT_BOOLEAN(0, "stdin", &force, "read objects from stdin"), Why are you setting &force from "--stdin"? > @@ -351,6 +398,15 @@ int cmd_notes(int argc, const char **argv, const > char *prefix) > > if (copy) { > const char *from_ref; > + if (stdin) { I don't think this is intended. Isn't stdin (the standard input FILE *) _always_ true? > + if (argc > 2) { > + error("too few parameters"); > + usage_with_options(git_notes_usage, options); > + } else { > + retval = notes_copy_from_stdin(force); > + goto end; > + } > + } > if (argc < 3) { > error("too few parameters"); > usage_with_options(git_notes_usage, options); The patch needs to add code that barfs if "--stdin" is used with any subcommand other than "copy". Of course, the patch also needs documentation and tests, but I guess you knew that... ;) I like the intent of this patch, although it needs more work before it's finished. ...Johan -- Johan Herland, www.herland.net ``` ## Johannes Sixt, 2010-02-15 20:36 Subject: Re: [RFC PATCH 3/6] filter-branch: invoke post-rewrite hook Message-ID: <4B79B035.2080604@kdbg.org> URL: https://gitlist.dev/e/4B79B035.2080604%40kdbg.org In-Reply-To: <0ad4b8c1a5377d697513cd8e49f64419cd8deef4.1266164150.git.trast@student.ethz.ch> ``` Thomas Rast schrieb: > +if test -x "$GIT_DIR"/hooks/post-rewrite && > + test -s "$workdir"/../rewritten; then > + "$GIT_DIR"/hooks/post-rewrite filter-branch < "$workdir"/../rewritten > +fi This sounds extra-strange. As if filter-branch is used 20 times a day. If the intent is to carry notes over to new commits, then filter-branch should grow a --notes-filter instruction, no? With an easily accessible copy-everything mode like --tagname-filter=cat. -- Hannes ``` ## Thomas Rast, 2010-02-16 23:25 Subject: [RFC PATCH v2 00/11] post-rewrite / automatic notes copying Message-ID: URL: https://gitlist.dev/e/cover.1266361759.git.trast%40student.ethz.ch In-Reply-To: ``` I originally had two series, but to retain my sanity while shuffling fixups, I based the entire batch on a rebase of jh/notes onto current master. The first four do not depend on jh/notes though, and are almost unchanged from the first round. The docs are probably sprinkled a bit strangely over the patches, not sure if it's worth a cleanup though (or I could just gather all the docs in a final patch). At least tests should be in the right place. Based on Hannes's comment: On Monday 15 February 2010 21:36:05 Johannes Sixt wrote: > Thomas Rast schrieb: > > +if test -x "$GIT_DIR"/hooks/post-rewrite && > > + test -s "$workdir"/../rewritten; then > > + "$GIT_DIR"/hooks/post-rewrite filter-branch < "$workdir"/../rewritten > > +fi > > This sounds extra-strange. As if filter-branch is used 20 times a day. If > the intent is to carry notes over to new commits, then filter-branch > should grow a --notes-filter instruction, no? With an easily accessible > copy-everything mode like --tagname-filter=cat. I made filter-branch extra-tweakable, but tried to stay consistent with the configuration of the rest of the series. Maybe it's a bit overengineered now, dunno. On Monday 15 February 2010 02:25:52 Johan Herland wrote: > On Sunday 14 February 2010, Thomas Rast wrote: > > > > I spent some time trying to refactor cmd_notes() into something > > that can nicely handle commands that do not fit the normal scheme, > > but eventually was too tired to continue and just crafted it in > > the existing code in the right place. [...] > As you say, cmd_notes() should be refactored, so that you don't have to > duplicate all of this (and some of the below stuff). Actually I meant something different: it took me quite some time to figure out what goes where in the flow of the current main() function, since it is sprinkled with if(subcommand) -- for most of them, several such clauses. I guess it made sense at the time, because all commands more or less worked with the same arguments and mostly the same background logic, just in a slightly different way. But I think it's quite unnatural for an entirely-different command like 'copy --stdin'. But maybe that's just me. > We (this goes for the existing commit_notes() call in cmd_notes() as well) > should either check the return value for commit_notes(), or (since it > currently only returns 0) we should change commit_notes() to void instead of > int. Actually, the correct solution is probably to "return error()" instead > of "die()" inside commit_notes(), and then check the return value. I'll leave that up to your judgement; I haven't looked into the API workings much, so I'm not sure when it should fail how. [Thanks for the other comments on my embarrassing mistakes; I was apparently rather too tired to continue programming.] Thomas Rast (11): Documentation: document post-rewrite hook commit --amend: invoke post-rewrite hook rebase: invoke post-rewrite hook rebase -i: invoke post-rewrite hook notes: clean up t3301 notes: implement 'git notes copy --stdin' notes: implement helpers needed for note copying during rewrite rebase: support automatic notes copying commit --amend: copy notes to the new commit filter-branch: invoke post-rewrite hook filter-branch: learn how to filter notes Documentation/config.txt | 17 +++ Documentation/git-filter-branch.txt | 20 +++- Documentation/git-notes.txt | 16 ++- Documentation/githooks.txt | 40 +++++ builtin-commit.c | 49 +++++++ builtin-notes.c | 134 +++++++++++++++++- builtin.h | 15 ++ git-am.sh | 13 ++ git-filter-branch.sh | 37 +++++ git-rebase--interactive.sh | 45 ++++++- git-rebase.sh | 6 + notes.c | 21 +++ notes.h | 9 ++ t/t3301-notes.sh | 273 +++++++++++++++++++++++------------ t/t3400-rebase.sh | 16 ++ t/t3404-rebase-interactive.sh | 10 ++ t/t5407-post-rewrite-hook.sh | 188 ++++++++++++++++++++++++ t/t7003-filter-branch.sh | 22 +++ t/t7501-commit.sh | 11 ++ 19 files changed, 843 insertions(+), 99 deletions(-) create mode 100755 t/t5407-post-rewrite-hook.sh ``` ## Thomas Rast, 2010-02-16 23:25 Subject: [RFC PATCH v2 01/11] Documentation: document post-rewrite hook Message-ID: <8ee58b83a10dcb302a33136768039729fbebae53.1266361759.git.trast@student.ethz.ch> URL: https://gitlist.dev/e/8ee58b83a10dcb302a33136768039729fbebae53.1266361759.git.trast%40student.ethz.ch In-Reply-To: ``` This defines the behaviour of the post-rewrite hook support, which will be implemented in the following patches. We deliberately do not document how often the hook will be invoked per rewriting command, but the interface is designed to keep that at "once". This would currently not matter too much, since both rebase and filter-branch are shellscripts and spawn many processes anyway. However, when a fast sequencer in C is implemented, it will be beneficial to only have to run the hook once. Signed-off-by: Thomas Rast --- Documentation/githooks.txt | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 87e2c03..41895e9 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -317,6 +317,42 @@ This hook is invoked by 'git gc --auto'. It takes no parameter, and exiting with non-zero status from this script causes the 'git gc --auto' to abort. +post-rewrite +~~~~~~~~~~~~ + +This hook is invoked by commands that rewrite commits (`git commit +--amend`, 'git-rebase', 'git-filter-branch'). Its first argument +denotes the command it was invoked by: currently one of `amend`, +`rebase`, or `filter-branch`. Further command-dependent arguments may +be passed in the future. + +The hook receives a list of the rewritten commits on stdin, in the +format + + SP [ SP ] LF + +The 'extra-info' is again command-dependent. If it is empty, the +preceding SP is also omitted. Currently, no commands pass any +'extra-info'. + +The following command-specific comments apply: + +rebase:: + For the 'squash' and 'fixup' operation, all commits that were + squashed are listed as being rewritten to the squashed commit. + This means that there will be several lines sharing the same + 'new-sha1'. + +filter-branch:: + Commits that were processed by 'git-filter-branch', but not + changed, are not included in the list. If the list is empty + after this filtering, the hook is not invoked at all. + +There is no default 'post-rewrite' hook, but see the +`post-receive-copy-notes` script in `contrib/hooks` for an example +that copies your git-notes to the rewritten commits. + + GIT --- Part of the linkgit:git[1] suite -- 1.7.0.53.g5c2e6.dirty ``` ## Thomas Rast, 2010-02-16 23:25 Subject: [RFC PATCH v2 02/11] commit --amend: invoke post-rewrite hook Message-ID: <297ce521ac8fa25ae00cbe8cf0a77d4fb9ee266d.1266361759.git.trast@student.ethz.ch> URL: https://gitlist.dev/e/297ce521ac8fa25ae00cbe8cf0a77d4fb9ee266d.1266361759.git.trast%40student.ethz.ch In-Reply-To: ``` The rough structure of run_rewrite_hook() comes from run_receive_hook() in receive-pack. We introduce a --no-post-rewrite option and use it to avoid the hook when called from git-rebase -i 'edit'. The next patch will add full support in git-rebase, and we only want to invoke the hook once. Signed-off-by: Thomas Rast --- builtin-commit.c | 43 ++++++++++++++++++++++++++++++++++ git-rebase--interactive.sh | 2 +- t/t5407-post-rewrite-hook.sh | 52 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletions(-) create mode 100755 t/t5407-post-rewrite-hook.sh diff --git a/builtin-commit.c b/builtin-commit.c index 55676fd..9b67649 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -66,6 +66,7 @@ static char *author_name, *author_email, *author_date; static int all, edit_flag, also, interactive, only, amend, signoff; static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship; +static int no_post_rewrite; static char *untracked_files_arg, *force_date; /* * The default commit message cleanup mode will remove the lines @@ -137,6 +138,7 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset) OPT_BOOLEAN('z', "null", &null_termination, "terminate entries with NUL"), OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"), + OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, "bypass post-rewrite hook"), { OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, "mode", "show untracked files, optional modes: all, normal, no. (Default: all)", PARSE_OPT_OPTARG, NULL, (intptr_t)"all" }, OPT_BOOLEAN(0, "allow-empty", &allow_empty, "ok to record an empty change"), /* end commit contents options */ @@ -1160,6 +1162,40 @@ static int git_commit_config(const char *k, const char *v, void *cb) return git_status_config(k, v, s); } +static const char post_rewrite_hook[] = "hooks/post-rewrite"; + +static int run_rewrite_hook(const unsigned char *oldsha1, + const unsigned char *newsha1) +{ + /* oldsha1 SP newsha1 LF NUL */ + static char buf[2*40 + 3]; + struct child_process proc; + const char *argv[3]; + int code; + size_t n; + + if (access(git_path(post_rewrite_hook), X_OK) < 0) + return 0; + + argv[0] = git_path(post_rewrite_hook); + argv[1] = "amend"; + argv[2] = NULL; + + memset(&proc, 0, sizeof(proc)); + proc.argv = argv; + proc.in = -1; + proc.stdout_to_stderr = 1; + + code = start_command(&proc); + if (code) + return code; + n = snprintf(buf, sizeof(buf), "%s %s\n", + sha1_to_hex(oldsha1), sha1_to_hex(newsha1)); + write_in_full(proc.in, buf, n); + close(proc.in); + return finish_command(&proc); +} + int cmd_commit(int argc, const char **argv, const char *prefix) { struct strbuf sb = STRBUF_INIT; @@ -1303,6 +1339,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix) rerere(0); run_hook(get_index_file(), "post-commit", NULL); + if (amend && !no_post_rewrite) { + struct commit *commit; + commit = lookup_commit(head_sha1); + if (!commit || parse_commit(commit)) + die("HEAD commit disappeared right under my eyes?"); + run_rewrite_hook(commit->object.sha1, commit_sha1); + } if (!quiet) print_summary(prefix, commit_sha1); diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 3e4fd14..5735859 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -445,7 +445,7 @@ do_next () { mark_action_done pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" - git commit --amend + git commit --amend --no-post-rewrite ;; edit|e) comment_for_reflog edit diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh new file mode 100755 index 0000000..1020af9 --- /dev/null +++ b/t/t5407-post-rewrite-hook.sh @@ -0,0 +1,52 @@ +#!/bin/sh +# +# Copyright (c) 2010 Thomas Rast +# + +test_description='Test the post-rewrite hook.' +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit A foo A && + test_commit B foo B && + test_commit C foo C && + test_commit D foo D +' + +mkdir .git/hooks + +cat >.git/hooks/post-rewrite < "$TRASH_DIRECTORY"/post-rewrite.args +cat > "$TRASH_DIRECTORY"/post-rewrite.data +EOF +chmod u+x .git/hooks/post-rewrite + +clear_hook_input () { + rm -f post-rewrite.args post-rewrite.data +} + +verify_hook_input () { + test_cmp "$TRASH_DIRECTORY"/post-rewrite.args expected.args && + test_cmp "$TRASH_DIRECTORY"/post-rewrite.data expected.data +} + +test_expect_success 'git commit --amend' ' + clear_hook_input && + echo "D new message" > newmsg && + oldsha=$(git rev-parse HEAD^0) && + git commit -Fnewmsg --amend && + echo amend > expected.args && + echo $oldsha $(git rev-parse HEAD^0) > expected.data && + verify_hook_input +' + +test_expect_success 'git commit --amend --no-post-rewrite' ' + clear_hook_input && + echo "D new message again" > newmsg && + git commit --no-post-rewrite -Fnewmsg --amend && + test ! -f post-rewrite.args && + test ! -f post-rewrite.data +' + +test_done -- 1.7.0.53.g5c2e6.dirty ``` ## Thomas Rast, 2010-02-16 23:25 Subject: [RFC PATCH v2 03/11] rebase: invoke post-rewrite hook Message-ID: <844b90d4fdb9156431c740dc9b870dd0f8c9e21b.1266361759.git.trast@student.ethz.ch> URL: https://gitlist.dev/e/844b90d4fdb9156431c740dc9b870dd0f8c9e21b.1266361759.git.trast%40student.ethz.ch In-Reply-To: ``` We have to deal with two separate code paths: a normal rebase, which actually goes through git-am; and rebase {-m|-s}. The only small issue with both is that they need to remember the original sha1 across a possible conflict resolution. rebase -m already puts this information in $dotest/current, and we just introduce a similar file for git-am. Note that in git-am, the hook really only runs when coming from git-rebase: the code path that sets the $dotest/original-commit file is guarded by a test for $dotest/rebasing. Signed-off-by: Thomas Rast --- git-am.sh | 10 ++++++++++ git-rebase.sh | 5 +++++ t/t5407-post-rewrite-hook.sh | 30 ++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 0 deletions(-) diff --git a/git-am.sh b/git-am.sh index 3c08d53..56428b4 100755 --- a/git-am.sh +++ b/git-am.sh @@ -575,6 +575,7 @@ do echo "Patch is empty. Was it split wrong?" stop_here $this } + rm -f "$dotest/original-commit" if test -f "$dotest/rebasing" && commit=$(sed -e 's/^From $[0-9a-f]*$ .*/\1/' \ -e q "$dotest/$msgnum") && @@ -582,6 +583,7 @@ do then git cat-file commit "$commit" | sed -e '1,/^$/d' >"$dotest/msg-clean" + echo "$commit" > "$dotest/original-commit" else { sed -n '/^Subject/ s/Subject: //p' "$dotest/info" @@ -768,6 +770,10 @@ do git update-ref -m "$GIT_REFLOG_ACTION: $FIRSTLINE" HEAD $commit $parent || stop_here $this + if test -f "$dotest/original-commit"; then + echo "$(cat "$dotest/original-commit") $commit" >> "$dotest/rewritten" + fi + if test -x "$GIT_DIR"/hooks/post-applypatch then "$GIT_DIR"/hooks/post-applypatch @@ -776,6 +782,10 @@ do go_next done +if test -s "$dotest"/rewritten && test -x "$GIT_DIR"/hooks/post-rewrite; then + "$GIT_DIR"/hooks/post-rewrite rebase < "$dotest"/rewritten +fi + git gc --auto rm -fr "$dotest" diff --git a/git-rebase.sh b/git-rebase.sh index fb4fef7..52f8b9b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -79,6 +79,7 @@ continue_merge () { then printf "Committed: %0${prec}d " $msgnum fi + echo "$cmt $(git rev-parse HEAD^0)" >> "$dotest/rewritten" else if test -z "$GIT_QUIET" then @@ -151,6 +152,10 @@ move_to_original_branch () { finish_rb_merge () { move_to_original_branch + if test -x "$GIT_DIR"/hooks/post-rewrite && + test -s "$dotest"/rewritten; then + "$GIT_DIR"/hooks/post-rewrite rebase < "$dotest"/rewritten + fi rm -r "$dotest" say All done. } diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index 1020af9..1ecaa4b 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -49,4 +49,34 @@ test_expect_success 'git commit --amend --no-post-rewrite' ' test ! -f post-rewrite.data ' +test_expect_success 'git rebase' ' + git reset --hard D && + clear_hook_input && + test_must_fail git rebase --onto A B && + echo C > foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < URL: https://gitlist.dev/e/17b3a5c3d40c54bc9a045c0a09c421fc4bc988e0.1266361759.git.trast%40student.ethz.ch In-Reply-To: ``` Aside from the same issue that rebase also has (remembering the original commit across a conflict resolution), rebase -i brings an extra twist: We need to defer writing the rewritten list in the case of {squash,fixup} because their rewritten result should be the last commit in the squashed group. Signed-off-by: Thomas Rast --- git-rebase--interactive.sh | 39 +++++++++++++++++++++ t/t5407-post-rewrite-hook.sh | 78 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 0 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 5735859..fc4f7e9 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -96,6 +96,13 @@ AUTHOR_SCRIPT="$DOTEST"/author-script # command is processed, this file is deleted. AMEND="$DOTEST"/amend +# For the post-rewrite hook, we make a list of rewritten commits and +# their new sha1s. The rewritten-pending list keeps the sha1s of +# commits that have been processed, but not committed yet, +# e.g. because they are waiting for a 'squash' command. +REWRITTEN_LIST="$DOTEST"/rewritten-list +REWRITTEN_PENDING="$DOTEST"/rewritten-pending + PRESERVE_MERGES= STRATEGY= ONTO= @@ -198,6 +205,7 @@ make_patch () { } die_with_patch () { + echo "$1" > "$DOTEST"/conflicted-sha make_patch "$1" git rerere die "$2" @@ -348,6 +356,7 @@ pick_one_preserving_merges () { printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG die_with_patch $sha1 "Error redoing merge $sha1" fi + echo "$sha1 $(git rev-parse HEAD^0)" >> "$REWRITTEN_LIST" ;; *) output git cherry-pick "$@" || @@ -425,6 +434,26 @@ die_failed_squash() { die_with_patch $1 "" } +flush_rewritten_pending() { + test -s "$REWRITTEN_PENDING" || return + newsha1="$(git rev-parse HEAD^0)" + sed "s/$/ $newsha1/" < "$REWRITTEN_PENDING" >> "$REWRITTEN_LIST" + rm -f "$REWRITTEN_PENDING" +} + +record_in_rewritten() { + oldsha1="$(git rev-parse $1)" + echo "$oldsha1" >> "$REWRITTEN_PENDING" + + case "$(peek_next_command)" in + squash|s|fixup|f) + ;; + *) + flush_rewritten_pending + ;; + esac +} + do_next () { rm -f "$MSG" "$AUTHOR_SCRIPT" "$AMEND" || exit read command sha1 rest < "$TODO" @@ -438,6 +467,7 @@ do_next () { mark_action_done pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" + record_in_rewritten $sha1 ;; reword|r) comment_for_reflog reword @@ -446,6 +476,7 @@ do_next () { pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" git commit --amend --no-post-rewrite + record_in_rewritten $sha1 ;; edit|e) comment_for_reflog edit @@ -454,6 +485,7 @@ do_next () { pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" make_patch $sha1 + record_in_rewritten "$sha1" git rev-parse --verify HEAD > "$AMEND" warn "Stopped at $sha1... $rest" warn "You can amend the commit now, with" @@ -509,6 +541,7 @@ do_next () { rm -f "$SQUASH_MSG" "$FIXUP_MSG" ;; esac + record_in_rewritten $sha1 ;; *) warn "Unknown command: $command $sha1 $rest" @@ -537,6 +570,11 @@ do_next () { test ! -f "$DOTEST"/verbose || git diff-tree --stat $(cat "$DOTEST"/head)..HEAD } && + if test -x "$GIT_DIR"/hooks/post-rewrite && + test -s "$REWRITTEN_LIST"; then + "$GIT_DIR"/hooks/post-rewrite rebase < "$REWRITTEN_LIST" + true # we don't care if this hook failed + fi && rm -rf "$DOTEST" && git gc --auto && warn "Successfully rebased and updated $HEADNAME." @@ -685,6 +723,7 @@ first and then run 'git rebase --continue' again." test -n "$amend" && git reset --soft $amend die "Could not commit staged changes." } + record_in_rewritten "$(cat "$DOTEST"/conflicted-sha)" fi require_clean_work_tree diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index 1ecaa4b..488d4a0 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -79,4 +79,82 @@ EOF verify_hook_input ' +test_expect_success 'git rebase -m' ' + git reset --hard D && + clear_hook_input && + test_must_fail git rebase -m --onto A B && + echo C > foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < foo && + git add foo && + git rebase --continue && + echo rebase >expected.args && + cat >expected.data < URL: https://gitlist.dev/e/2c7b0e51f7d299a481b62cbe4f728ff20e4651ab.1266361759.git.trast%40student.ethz.ch In-Reply-To: ``` The existing tests handrolled test_commit, so use that instead. Unfortunately this changes all sha1's. Since we're not supposed to unneccessarily rely on them anyway, introduce a small helper that filters out the commit headers before comparing, and remove them from the expected outputs. Signed-off-by: Thomas Rast --- t/t3301-notes.sh | 141 ++++++++++++++++++------------------------------------ 1 files changed, 46 insertions(+), 95 deletions(-) diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index 90178f9..3fec7ae 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -7,6 +7,15 @@ test_description='Test commit notes' . ./test-lib.sh +strip_header () { + sed -i '/^$commit\|tree\|parent$ /d' "$@" +} + +strip_then_cmp () { + strip_header "$2" + test_cmp "$@" +} + cat > fake_editor.sh << \EOF echo "$MSG" > "$1" echo "$MSG" >& 2 @@ -20,14 +29,8 @@ test_expect_success 'cannot annotate non-existing HEAD' ' ' test_expect_success setup ' - : > a1 && - git add a1 && - test_tick && - git commit -m 1st && - : > a2 && - git add a2 && - test_tick && - git commit -m 2nd + test_commit 1st && + test_commit 2nd ' test_expect_success 'need valid notes ref' ' @@ -92,7 +95,6 @@ test_expect_success 'can overwrite existing note with "git notes add -f"' ' ' cat > expect << EOF -commit 268048bfb8a1fb38e703baceb8ab235421bf80c5 Author: A U Thor Date: Thu Apr 7 15:14:13 2005 -0700 @@ -105,21 +107,17 @@ EOF test_expect_success 'show notes' ' ! (git cat-file commit HEAD | grep b1) && git log -1 > output && - test_cmp expect output + strip_then_cmp expect output ' test_expect_success 'create multi-line notes (setup)' ' - : > a3 && - git add a3 && - test_tick && - git commit -m 3rd && + test_commit 3rd && MSG="b3 c3c3c3c3 d3d3d3" git notes add ' cat > expect-multiline << EOF -commit 1584215f1d29c65e99c6c6848626553fdd07fd75 Author: A U Thor Date: Thu Apr 7 15:15:13 2005 -0700 @@ -136,19 +134,15 @@ cat expect >> expect-multiline test_expect_success 'show multi-line notes' ' git log -2 > output && - test_cmp expect-multiline output + strip_then_cmp expect-multiline output ' test_expect_success 'create -F notes (setup)' ' - : > a4 && - git add a4 && - test_tick && - git commit -m 4th && + test_commit 4th && echo "xyzzy" > note5 && git notes add -F note5 ' cat > expect-F << EOF -commit 15023535574ded8b1a89052b32673f84cf9582b8 Author: A U Thor Date: Thu Apr 7 15:16:13 2005 -0700 @@ -163,13 +157,10 @@ cat expect-multiline >> expect-F test_expect_success 'show -F notes' ' git log -3 > output && - test_cmp expect-F output + strip_then_cmp expect-F output ' cat >expect << EOF -commit 15023535574ded8b1a89052b32673f84cf9582b8 -tree e070e3af51011e47b183c33adf9736736a525709 -parent 1584215f1d29c65e99c6c6848626553fdd07fd75 author A U Thor 1112912173 -0700 committer C O Mitter 1112912173 -0700 @@ -177,7 +168,7 @@ committer C O Mitter 1112912173 -0700 EOF test_expect_success 'git log --pretty=raw does not show notes' ' git log -1 --pretty=raw >output && - test_cmp expect output + strip_then_cmp expect output ' cat >>expect <output && - test_cmp expect output + strip_then_cmp expect output ' test_expect_success 'git log --no-notes' ' @@ -220,10 +211,7 @@ do done test_expect_success 'create -m notes (setup)' ' - : > a5 && - git add a5 && - test_tick && - git commit -m 5th && + test_commit 5th && git notes add -m spam -m "foo bar baz" @@ -231,7 +219,6 @@ baz" whitespace=" " cat > expect-m << EOF -commit bd1753200303d0a0344be813e504253b3d98e74d Author: A U Thor Date: Thu Apr 7 15:17:13 2005 -0700 @@ -250,7 +237,7 @@ cat expect-F >> expect-m test_expect_success 'show -m notes' ' git log -4 > output && - test_cmp expect-m output + strip_then_cmp expect-m output ' test_expect_success 'remove note with add -f -F /dev/null (setup)' ' @@ -258,7 +245,6 @@ test_expect_success 'remove note with add -f -F /dev/null (setup)' ' ' cat > expect-rm-F << EOF -commit bd1753200303d0a0344be813e504253b3d98e74d Author: A U Thor Date: Thu Apr 7 15:17:13 2005 -0700 @@ -270,7 +256,7 @@ cat expect-F >> expect-rm-F test_expect_success 'verify note removal with -F /dev/null' ' git log -4 > output && - test_cmp expect-rm-F output && + strip_then_cmp expect-rm-F output && ! git notes show ' @@ -280,7 +266,7 @@ test_expect_success 'do not create empty note with -m "" (setup)' ' test_expect_success 'verify non-creation of note with -m ""' ' git log -4 > output && - test_cmp expect-rm-F output && + strip_then_cmp expect-rm-F output && ! git notes show ' @@ -301,7 +287,7 @@ test_expect_success 'create note with combination of -m and -F' ' echo "zyxxy" > note_b && git notes add -m "foo" -F note_a -m "bar" -F note_b -m "baz" && git notes show > output && - test_cmp expect-combine_m_and_F output + strip_then_cmp expect-combine_m_and_F output ' test_expect_success 'remove note with "git notes remove" (setup)' ' @@ -310,13 +296,11 @@ test_expect_success 'remove note with "git notes remove" (setup)' ' ' cat > expect-rm-remove << EOF -commit bd1753200303d0a0344be813e504253b3d98e74d Author: A U Thor Date: Thu Apr 7 15:17:13 2005 -0700 5th -commit 15023535574ded8b1a89052b32673f84cf9582b8 Author: A U Thor Date: Thu Apr 7 15:16:13 2005 -0700 @@ -328,13 +312,13 @@ cat expect-multiline >> expect-rm-remove test_expect_success 'verify note removal with "git notes remove"' ' git log -4 > output && - test_cmp expect-rm-remove output && + strip_then_cmp expect-rm-remove output && ! git notes show HEAD^ ' cat > expect << EOF -c18dc024e14f08d18d14eea0d747ff692d66d6a3 1584215f1d29c65e99c6c6848626553fdd07fd75 -c9c6af7f78bc47490dbf3e822cf2f3c24d4b9061 268048bfb8a1fb38e703baceb8ab235421bf80c5 +c9c6af7f78bc47490dbf3e822cf2f3c24d4b9061 7a4ca6ee52a974a66cbaa78e33214535dff1d691 +c18dc024e14f08d18d14eea0d747ff692d66d6a3 d07d62e5208f22eb5695e7eb47667dc8b9860290 EOF test_expect_success 'list notes with "git notes list"' ' @@ -401,15 +385,11 @@ test_expect_success 'appending empty string to non-existing note does not create ' test_expect_success 'create other note on a different notes ref (setup)' ' - : > a6 && - git add a6 && - test_tick && - git commit -m 6th && + test_commit 6th && GIT_NOTES_REF="refs/notes/other" git notes add -m "other note" ' cat > expect-other << EOF -commit 387a89921c73d7ed72cd94d179c1c7048ca47756 Author: A U Thor Date: Thu Apr 7 15:18:13 2005 -0700 @@ -420,7 +400,6 @@ Notes: EOF cat > expect-not-other << EOF -commit 387a89921c73d7ed72cd94d179c1c7048ca47756 Author: A U Thor Date: Thu Apr 7 15:18:13 2005 -0700 @@ -429,23 +408,23 @@ EOF test_expect_success 'Do not show note on other ref by default' ' git log -1 > output && - test_cmp expect-not-other output + strip_then_cmp expect-not-other output ' test_expect_success 'Do show note when ref is given in GIT_NOTES_REF' ' GIT_NOTES_REF="refs/notes/other" git log -1 > output && - test_cmp expect-other output + strip_then_cmp expect-other output ' test_expect_success 'Do show note when ref is given in core.notesRef config' ' git config core.notesRef "refs/notes/other" && git log -1 > output && - test_cmp expect-other output + strip_then_cmp expect-other output ' test_expect_success 'Do not show note when core.notesRef is overridden' ' GIT_NOTES_REF="refs/notes/wrong" git log -1 > output && - test_cmp expect-not-other output + strip_then_cmp expect-not-other output ' test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' ' @@ -466,7 +445,6 @@ test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' ' ' cat > expect << EOF -commit 2ede89468182a62d0bde2583c736089bcf7d7e92 Author: A U Thor Date: Thu Apr 7 15:19:13 2005 -0700 @@ -477,27 +455,20 @@ Notes: EOF test_expect_success 'create note from other note with "git notes add -C"' ' - : > a7 && - git add a7 && - test_tick && - git commit -m 7th && + test_commit 7th && git notes add -C $(git notes list HEAD^) && git log -1 > actual && - test_cmp expect actual && + strip_then_cmp expect actual && test "$(git notes list HEAD)" = "$(git notes list HEAD^)" ' test_expect_success 'create note from non-existing note with "git notes add -C" fails' ' - : > a8 && - git add a8 && - test_tick && - git commit -m 8th && + test_commit 8th && test_must_fail git notes add -C deadbeef && test_must_fail git notes list HEAD ' cat > expect << EOF -commit 016e982bad97eacdbda0fcbd7ce5b0ba87c81f1b Author: A U Thor Date: Thu Apr 7 15:21:13 2005 -0700 @@ -508,26 +479,19 @@ Notes: EOF test_expect_success 'create note from other note with "git notes add -c"' ' - : > a9 && - git add a9 && - test_tick && - git commit -m 9th && + test_commit 9th && MSG="yet another note" git notes add -c $(git notes list HEAD^^) && git log -1 > actual && - test_cmp expect actual + strip_then_cmp expect actual ' test_expect_success 'create note from non-existing note with "git notes add -c" fails' ' - : > a10 && - git add a10 && - test_tick && - git commit -m 10th && + test_commit 10th && test_must_fail MSG="yet another note" git notes add -c deadbeef && test_must_fail git notes list HEAD ' cat > expect << EOF -commit 016e982bad97eacdbda0fcbd7ce5b0ba87c81f1b Author: A U Thor Date: Thu Apr 7 15:21:13 2005 -0700 @@ -542,11 +506,10 @@ EOF test_expect_success 'append to note from other note with "git notes append -C"' ' git notes append -C $(git notes list HEAD^) HEAD^ && git log -1 HEAD^ > actual && - test_cmp expect actual + strip_then_cmp expect actual ' cat > expect << EOF -commit ffed603236bfa3891c49644257a83598afe8ae5a Author: A U Thor Date: Thu Apr 7 15:22:13 2005 -0700 @@ -559,11 +522,10 @@ EOF test_expect_success 'create note from other note with "git notes append -c"' ' MSG="other note" git notes append -c $(git notes list HEAD^) && git log -1 > actual && - test_cmp expect actual + strip_then_cmp expect actual ' cat > expect << EOF -commit ffed603236bfa3891c49644257a83598afe8ae5a Author: A U Thor Date: Thu Apr 7 15:22:13 2005 -0700 @@ -578,11 +540,10 @@ EOF test_expect_success 'append to note from other note with "git notes append -c"' ' MSG="yet another note" git notes append -c $(git notes list HEAD) && git log -1 > actual && - test_cmp expect actual + strip_then_cmp expect actual ' cat > expect << EOF -commit 6352c5e33dbcab725fe0579be16aa2ba8eb369be Author: A U Thor Date: Thu Apr 7 15:23:13 2005 -0700 @@ -595,25 +556,21 @@ $whitespace EOF test_expect_success 'copy note with "git notes copy"' ' - : > a11 && - git add a11 && - test_tick && - git commit -m 11th && + test_commit 11th && git notes copy HEAD^ HEAD && git log -1 > actual && - test_cmp expect actual && + strip_then_cmp expect actual && test "$(git notes list HEAD)" = "$(git notes list HEAD^)" ' test_expect_success 'prevent overwrite with "git notes copy"' ' test_must_fail git notes copy HEAD~2 HEAD && git log -1 > actual && - test_cmp expect actual && + strip_then_cmp expect actual && test "$(git notes list HEAD)" = "$(git notes list HEAD^)" ' cat > expect << EOF -commit 6352c5e33dbcab725fe0579be16aa2ba8eb369be Author: A U Thor Date: Thu Apr 7 15:23:13 2005 -0700 @@ -628,19 +585,13 @@ EOF test_expect_success 'allow overwrite with "git notes copy -f"' ' git notes copy -f HEAD~2 HEAD && git log -1 > actual && - test_cmp expect actual && + strip_then_cmp expect actual && test "$(git notes list HEAD)" = "$(git notes list HEAD~2)" ' test_expect_success 'cannot copy note from object without notes' ' - : > a12 && - git add a12 && - test_tick && - git commit -m 12th && - : > a13 && - git add a13 && - test_tick && - git commit -m 13th && + test_commit 12th && + test_commit 13th && test_must_fail git notes copy HEAD^ HEAD ' -- 1.7.0.53.g5c2e6.dirty ``` ## Thomas Rast, 2010-02-16 23:26 Subject: [RFC PATCH v2 06/11] notes: implement 'git notes copy --stdin' Message-ID: URL: https://gitlist.dev/e/d8649cbda00586a9dfbd5085bb7338a5824134b1.1266361759.git.trast%40student.ethz.ch In-Reply-To: ``` This implements a mass-copy command that takes a sequence of lines in the format SP [ SP ] LF on stdin, and copies each 's notes to the . The is ignored. The intent, of course, is that this can read the same input that the 'post-rewrite' hook gets. The copy_note() function is exposed for everyone's and in particular the next commit's use. Signed-off-by: Thomas Rast --- Documentation/git-notes.txt | 12 ++++++++- builtin-notes.c | 57 ++++++++++++++++++++++++++++++++++++++++++- notes.c | 21 ++++++++++++++++ notes.h | 9 +++++++ t/t3301-notes.sh | 32 ++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 2 deletions(-) diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt index 14f73b9..f67cb6a 100644 --- a/Documentation/git-notes.txt +++ b/Documentation/git-notes.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git notes' [list [