From 572a7c52bb12e22b3b8471133253b9884643a4e2 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 26 Jun 2012 07:51:54 -0700 Subject: rebase: don't source git-sh-setup twice The git-sh-setup script is already sourced in git-rebase.sh before calling into git-rebase--(am|interactive|merge).sh. There are no other callers of these scripts. It is therefore unnecessary to source git-sh-setup again in them. Signed-off-by: Junio C Hamano --- git-rebase--am.sh | 2 -- git-rebase--interactive.sh | 4 +--- git-rebase--merge.sh | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 04d89415fe..392ebc9790 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -3,8 +3,6 @@ # Copyright (c) 2010 Junio C Hamano. # -. git-sh-setup - case "$action" in continue) git am --resolved --resolvemsg="$resolvemsg" && diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 0c19b7c753..a5b018d4ac 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -9,9 +9,7 @@ # # The original idea comes from Eric W. Biederman, in # http://article.gmane.org/gmane.comp.version-control.git/22407 - -. git-sh-setup - +# # The file containing rebase commands, comments, and empty lines. # This file is created by "git rebase -i" then edited by the user. As # the lines are processed, they are removed from the front of this diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh index dc599077f0..b10f2cf21b 100644 --- a/git-rebase--merge.sh +++ b/git-rebase--merge.sh @@ -3,8 +3,6 @@ # Copyright (c) 2010 Junio C Hamano. # -. git-sh-setup - prec=4 read_state () { -- cgit v1.2.1 From f2b6a19907ccb366790c7ec7b640f9111ac7ad32 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 26 Jun 2012 07:51:55 -0700 Subject: rebase --root: print usage on too many args Just like git rebase --onto newbase upstream branch error displays the usage message, so should clearly git rebase --onto newbase --root branch error , but it doesn't. Instead, it ignores both "branch" and "error" and rebases the current HEAD. This is because we try to match the number of remainging arguments "$#", which fails to match "1" argument and matches the "*" that really should have been a "0". Make sure we display usage information when too many arguments are given. Also fail-fast in case of similar bugs in the future by matching on exactly 0 arguments and failing on unknown numbers. Signed-off-by: Junio C Hamano --- git-rebase.sh | 6 +++++- t/t3412-rebase-root.sh | 8 +++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index e616737444..6df06c4008 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -400,6 +400,7 @@ else test -z "$onto" && die "You must specify --onto when using --root" unset upstream_name unset upstream + test $# -gt 1 && usage upstream_arg=--root fi @@ -450,7 +451,7 @@ case "$#" in die "fatal: no such branch: $1" fi ;; -*) +0) # Do not need to switch branches, we are already on it. if branch_name=`git symbolic-ref -q HEAD` then @@ -462,6 +463,9 @@ case "$#" in fi orig_head=$(git rev-parse --verify "${branch_name}^0") || exit ;; +*) + die "BUG: unexpected number of arguments left to parse" + ;; esac require_clean_work_tree "rebase" "Please commit or stash them." diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh index 086c91c7b4..1e9d1a737c 100755 --- a/t/t3412-rebase-root.sh +++ b/t/t3412-rebase-root.sh @@ -23,9 +23,15 @@ test_expect_success 'prepare repository' ' ' test_expect_success 'rebase --root expects --onto' ' + git checkout -B fail other && test_must_fail git rebase --root ' +test_expect_success 'rebase --root fails with too many args' ' + git checkout -B fail other && + test_must_fail git rebase --onto master --root fail fail +' + test_expect_success 'setup pre-rebase hook' ' mkdir -p .git/hooks && cat >.git/hooks/pre-rebase < expect <' ' - git checkout -b work && + git checkout -b work other && git rebase --root --onto master && git log --pretty=tformat:"%s" > rebased && test_cmp expect rebased -- cgit v1.2.1 From a230949409f4a650c6a1a9a5879e2a8b993ba695 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 26 Jun 2012 07:51:56 -0700 Subject: am --rebasing: get patch body from commit, not from mailbox Rebasing a commit that contains a diff in the commit message results in a failure with output such as First, rewinding head to replay your work on top of it... Applying: My cool patch. fatal: sha1 information is lacking or useless (app/controllers/settings_controller.rb). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 My cool patch. The reason is that 'git rebase' without -p/-i/-m internally calls 'git format-patch' and pipes the output to 'git am --rebasing', which has no way of knowing what is a real patch and what is a commit message that contains a patch. Make 'git am' while in --rebasing mode get the patch body from the commit object instead of extracting it from the mailbox. Patch by Junio, test case and commit log message by Martin. Reported-by: anikey Helped-by: Junio C Hamano Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- git-am.sh | 1 + t/t3405-rebase-malformed.sh | 32 ++++++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/git-am.sh b/git-am.sh index f8b7a0cb60..ec8fde102c 100755 --- a/git-am.sh +++ b/git-am.sh @@ -683,6 +683,7 @@ To restore the original branch and stop patching run \"\$cmdline --abort\"." sed -e '1,/^$/d' >"$dotest/msg-clean" echo "$commit" > "$dotest/original-commit" get_author_ident_from_commit "$commit" > "$dotest/author-script" + git diff-tree --root --binary "$commit" >"$dotest/patch" else { sed -n '/^Subject/ s/Subject: //p' "$dotest/info" diff --git a/t/t3405-rebase-malformed.sh b/t/t3405-rebase-malformed.sh index e5ad67c643..19eddadcf7 100755 --- a/t/t3405-rebase-malformed.sh +++ b/t/t3405-rebase-malformed.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='rebase should not insist on git message convention' +test_description='rebase should handle arbitrary git message' . ./test-lib.sh @@ -12,6 +12,11 @@ It has two paragraphs, but its first paragraph is not friendly to oneline summary format. EOF +cat >G <<\EOF +commit log message containing a diff +EOF + + test_expect_success setup ' >file1 && @@ -19,8 +24,9 @@ test_expect_success setup ' git add file1 file2 && test_tick && git commit -m "Initial commit" && + git branch diff-in-message - git checkout -b side && + git checkout -b multi-line-subject && cat F >file2 && git add file2 && test_tick && @@ -28,6 +34,17 @@ test_expect_success setup ' git cat-file commit HEAD | sed -e "1,/^\$/d" >F0 && + git checkout diff-in-message && + echo "commit log message containing a diff" >G && + echo "" >>G + cat G >file2 && + git add file2 && + git diff --cached >>G && + test_tick && + git commit -F G && + + git cat-file commit HEAD | sed -e "1,/^\$/d" >G0 && + git checkout master && echo One >file1 && @@ -36,13 +53,20 @@ test_expect_success setup ' git commit -m "Second commit" ' -test_expect_success rebase ' +test_expect_success 'rebase commit with multi-line subject' ' - git rebase master side && + git rebase master multi-line-subject && git cat-file commit HEAD | sed -e "1,/^\$/d" >F1 && test_cmp F0 F1 && test_cmp F F0 ' +test_expect_success 'rebase commit with diff in message' ' + git rebase master diff-in-message && + git cat-file commit HEAD | sed -e "1,/^$/d" >G1 && + test_cmp G0 G1 && + test_cmp G G0 +' + test_done -- cgit v1.2.1 From 0fbb95dc83c80fa12f731e703e88e61d9391abd5 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 26 Jun 2012 07:51:57 -0700 Subject: am: don't call mailinfo if $rebasing Since 5e835ca (rebase: do not munge commit log message, 2008-04-16), 'git am --rebasing' no longer gets the commit log message from the patch, but reads it from the commit identified by the "From " header line. From 43c2325 (am: use get_author_ident_from_commit instead of mailinfo when rebasing, 2010-06-16), it also gets the author name, email and date from the commit. Now that the final part of the patch -- the patch body itself -- is also read from the commit, there is no longer a need to call 'git mailinfo' to extract any of these parts while --rebasing. Sugested-by: Junio C Hamano Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- git-am.sh | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/git-am.sh b/git-am.sh index ec8fde102c..b6a530018e 100755 --- a/git-am.sh +++ b/git-am.sh @@ -260,7 +260,7 @@ check_patch_format () { split_patches () { case "$patch_format" in mbox) - if test -n "$rebasing" || test t = "$keepcr" + if test t = "$keepcr" then keep_cr=--keep-cr else @@ -413,7 +413,7 @@ do --abort) abort=t ;; --rebasing) - rebasing=t threeway=t keep=t scissors=f no_inbody_headers=t ;; + rebasing=t threeway=t ;; -d|--dotest) die "$(gettext "-d option is no longer supported. Do not use.")" ;; @@ -658,33 +658,34 @@ do # by the user, or the user can tell us to do so by --resolved flag. case "$resume" in '') - git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \ - <"$dotest/$msgnum" >"$dotest/info" || - stop_here $this - - # skip pine's internal folder data - sane_grep '^Author: Mail System Internal Data$' \ - <"$dotest"/info >/dev/null && - go_next && continue - - test -s "$dotest/patch" || { - eval_gettextln "Patch is empty. Was it split wrong? -If you would prefer to skip this patch, instead run \"\$cmdline --skip\". -To restore the original branch and stop patching run \"\$cmdline --abort\"." - stop_here $this - } - rm -f "$dotest/original-commit" "$dotest/author-script" - if test -f "$dotest/rebasing" && + if test -f "$dotest/rebasing" + then commit=$(sed -e 's/^From \([0-9a-f]*\) .*/\1/' \ -e q "$dotest/$msgnum") && - test "$(git cat-file -t "$commit")" = commit - then + test "$(git cat-file -t "$commit")" = commit || + stop_here $this git cat-file commit "$commit" | sed -e '1,/^$/d' >"$dotest/msg-clean" - echo "$commit" > "$dotest/original-commit" - get_author_ident_from_commit "$commit" > "$dotest/author-script" + echo "$commit" >"$dotest/original-commit" + get_author_ident_from_commit "$commit" >"$dotest/author-script" git diff-tree --root --binary "$commit" >"$dotest/patch" else + git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \ + <"$dotest/$msgnum" >"$dotest/info" || + stop_here $this + + # skip pine's internal folder data + sane_grep '^Author: Mail System Internal Data$' \ + <"$dotest"/info >/dev/null && + go_next && continue + + test -s "$dotest/patch" || { + eval_gettextln "Patch is empty. Was it split wrong? +If you would prefer to skip this patch, instead run \"\$cmdline --skip\". +To restore the original branch and stop patching run \"\$cmdline --abort\"." + stop_here $this + } + rm -f "$dotest/original-commit" "$dotest/author-script" { sed -n '/^Subject/ s/Subject: //p' "$dotest/info" echo -- cgit v1.2.1