Retrieving a file at a before a specified commit
Hi all, I have a commit like this: commit 4d77a3cee01db0412956d40875c79f51ac745acc tree 3443c9f633114c3bd2e015453a8c55a171e62b53 parent 340d808ade8a79857bec40770f0eb4f98224c53d author committer . which modifies file A/B/C (ie specifically does not add, but changes an existing file in the repo). I would then like to retrive the version of the file A/B/C before commit 4d77a3cee by using the parent commit 340d808a: git show 340d808ade8a79857bec40770f0eb4f98224c53d:A/B/C which works for most files/commits I try this with, but doesn't work in one particular case. Questions: - Is my understanding of the above git command incorrect? - Is this a corrupt repo? Is there some way to check? - Is there some explaination of why I can't get the previous version of that file? Appreciate any light that could be shed on this. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/7] Rebase topology test
After way too long, here is finally a new version of the tests I sent at: http://thread.gmane.org/gmane.comp.version-control.git/205796. I have split the test up into two files. They stil take quite some time to run. Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 t/lib-rebase.sh | 32 t/t3400-rebase.sh | 53 +- t/t3401-rebase-partial.sh | 69 t/t3404-rebase-interactive.sh | 10 +- t/t3406-rebase-message.sh | 50 +++--- t/t3409-rebase-preserve-merges.sh | 53 -- t/t3420-rebase-topology-linear.sh | 350 ++ t/t3425-rebase-topology-merges.sh | 250 +++ 8 files changed, 664 insertions(+), 203 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3420-rebase-topology-linear.sh create mode 100755 t/t3425-rebase-topology-merges.sh -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/7] add tests for rebasing with patch-equivalence present
--- t/lib-rebase.sh | 17 t/t3420-rebase-topology-linear.sh | 85 +++ 2 files changed, 102 insertions(+) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 62b3887..16eeb1c 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -80,3 +80,20 @@ reset_rebase () { git reset --hard && git clean -f } + +cherry_pick () { + git cherry-pick -n "$2" && + git commit -m "$1" && + git tag "$1" +} + +revert () { + git revert -n "$2" && + git commit -m "$1" && + git tag "$1" +} + +make_empty () { + git commit --allow-empty -m "$1" && + git tag "$1" +} diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index c4b32db..1152491 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -75,4 +75,89 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# f +# / +# a---b---c---g---h +# \ +# d---G---i +# +# uppercase = cherry-picked +# h = reverted g +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for range selection tests' ' + git checkout c && + test_commit g && + revert h g && + git checkout d && + cherry_pick G g && + test_commit i && + git checkout b && + test_commit f +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* drops patches in upstream" " + reset_rebase && + git rebase $* h i && + test_cmp_rev h HEAD~2 && + test_linear_range 'd i' h.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* can drop last patch if in upstream" " + reset_rebase && + git rebase $* h G && + test_cmp_rev h HEAD^ && + test_linear_range 'd' h.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --onto does not lose patches in upstream" " + reset_rebase && + git rebase $* --onto f h i && + test_cmp_rev f HEAD~3 && + test_linear_range 'd G i' f.. + " +} +test_run_rebase failure '' +test_run_rebase success -m +test_run_rebase failure -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --onto drops patches in onto" " + reset_rebase && + git rebase $* --onto h f i && + test_cmp_rev h HEAD~2 && + test_linear_range 'd i' h.. + " +} +test_run_rebase failure '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + test_done -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/7] tests: move test for rebase messages from t3400 to t3406
t3406 is supposed to test "messages from rebase operation", so let's move tests in t3400 that fit that description into 3406. Most of the functionality they tested, except for the messages, has now been subsumed by t3420. --- t/t3400-rebase.sh | 22 -- t/t3406-rebase-message.sh | 22 ++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b436ef4..45a55e9 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -59,28 +59,6 @@ test_expect_success 'rebase against master' ' git rebase master ' -test_expect_success 'rebase against master twice' ' - git rebase master >out && - test_i18ngrep "Current branch my-topic-branch is up to date" out -' - -test_expect_success 'rebase against master twice with --force' ' - git rebase --force-rebase master >out && - test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out -' - -test_expect_success 'rebase against master twice from another branch' ' - git checkout my-topic-branch^ && - git rebase master my-topic-branch >out && - test_i18ngrep "Current branch my-topic-branch is up to date" out -' - -test_expect_success 'rebase fast-forward to master' ' - git checkout my-topic-branch^ && - git rebase my-topic-branch >out && - test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out -' - test_expect_success 'the rebase operation should not have destroyed author information' ' ! (git log | grep "Author:" | grep "<>") ' diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index fe8c27f..0392e36 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -30,6 +30,28 @@ test_expect_success 'rebase -m' ' test_cmp expect actual ' +test_expect_success 'rebase against master twice' ' + git rebase master >out && + test_i18ngrep "Current branch topic is up to date" out +' + +test_expect_success 'rebase against master twice with --force' ' + git rebase --force-rebase master >out && + test_i18ngrep "Current branch topic is up to date, rebase forced" out +' + +test_expect_success 'rebase against master twice from another branch' ' + git checkout topic^ && + git rebase master topic >out && + test_i18ngrep "Current branch topic is up to date" out +' + +test_expect_success 'rebase fast-forward to master' ' + git checkout topic^ && + git rebase topic >out && + test_i18ngrep "Fast-forwarded HEAD to topic" out +' + test_expect_success 'rebase --stat' ' git reset --hard start && git rebase --stat master >diffstat.txt && -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/7] add tests for rebasing merged history
--- t/t3400-rebase.sh | 31 + t/t3401-rebase-partial.sh | 45 --- t/t3404-rebase-interactive.sh | 10 +- t/t3409-rebase-preserve-merges.sh | 53 t/t3425-rebase-topology-merges.sh | 250 ++ 5 files changed, 252 insertions(+), 137 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3425-rebase-topology-merges.sh diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b58fa1a..b436ef4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' ' echo Side >>C && git add C && git commit -m "Add C" && - git checkout -b nonlinear my-topic-branch && - echo Edit >>B && - git add B && - git commit -m "Modify B" && - git merge side && - git checkout -b upstream-merged-nonlinear && - git merge master && git checkout -f my-topic-branch && git tag topic ' @@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' ' git rebase master ' -test_expect_success 'rebase after merge master' ' - git checkout --detach refs/tags/topic && - git branch -D topic && - git reset --hard topic && - git merge master && - git rebase master && - ! (git show | grep "^Merge:") -' - -test_expect_success 'rebase of history with merges is linearized' ' - git checkout nonlinear && - test 4 = $(git rev-list master.. | wc -l) && - git rebase master && - test 3 = $(git rev-list master.. | wc -l) -' - -test_expect_success 'rebase of history with merges after upstream merge is linearized' ' - git checkout upstream-merged-nonlinear && - test 5 = $(git rev-list master.. | wc -l) && - git rebase master && - test 3 = $(git rev-list master.. | wc -l) -' - test_expect_success 'rebase a single mode change' ' git checkout master && + git branch -D topic && echo 1 >X && git add X && test_tick && diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh deleted file mode 100755 index 7ba1797..000 --- a/t/t3401-rebase-partial.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland -# - -test_description='git rebase should detect patches integrated upstream - -This test cherry-picks one local change of two into master branch, and -checks that git rebase succeeds with only the second patch in the -local branch. -' -. ./test-lib.sh - -test_expect_success 'prepare repository with topic branch' ' - test_commit A && - git checkout -b my-topic-branch && - test_commit B && - test_commit C && - git checkout -f master && - test_commit A2 A.t -' - -test_expect_success 'pick top patch from topic branch into master' ' - git cherry-pick C && - git checkout -f my-topic-branch -' - -test_debug ' - git cherry master && - git format-patch -k --stdout --full-index master >/dev/null && - gitk --all & sleep 1 -' - -test_expect_success 'rebase topic branch against new master and check git am did not get halted' ' - git rebase master && - test_path_is_missing .git/rebase-apply -' - -test_expect_success 'rebase --merge topic branch that was partially merged upstream' ' - git reset --hard C && - git rebase --merge master && - test_path_is_missing .git/rebase-merge -' - -test_done diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a58406d..ffcaf02 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' ' test $one = $(git rev-parse HEAD~2) ' -test_expect_success 'ignore patch if in upstream' ' - HEAD=$(git rev-parse HEAD) && - git checkout -b has-cherry-picked HEAD^ && +test_expect_success '--continue tries to commit, even for "edit"' ' echo unrelated > file7 && git add file7 && test_tick && git commit -m "unrelated change" && - git cherry-pick $HEAD && - EXPECT_COUNT=1 git rebase -i $HEAD && - test $HEAD = $(git rev-parse HEAD^) -' - -test_expect_success '--continue tries to commit, even for "edit"' ' parent=$(git rev-parse HEAD^) && test_tick && FAKE_LINES="edit 1" git rebase -i HEAD^ && diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index 6de4e22..2e0c364 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -11,14 +11,6 @@ Run "git rebase -p" and check that merges are properly carried along GIT_AUTHOR_EMAIL=bogus_email_address export GIT_AUTHOR_EMAIL -# Clone 1 (trivial merge): -# -# A1--A2 <-- origin/master -# \ \ -# B1--M <-- topic -#\ -# B2 <-- origin/topic -# #
[PATCH v2 1/7] add simple tests of consistency across rebase types
Helped-by: Johannes Sixt --- t/lib-rebase.sh | 15 t/t3420-rebase-topology-linear.sh | 78 +++ 2 files changed, 93 insertions(+) create mode 100755 t/t3420-rebase-topology-linear.sh diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 6ccf797..62b3887 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -65,3 +65,18 @@ EOF test_set_editor "$(pwd)/fake-editor.sh" chmod a+x fake-editor.sh } + +# checks that the revisions in "$2" represent a linear range with the +# subjects in "$1" +test_linear_range () { + ! { git log --format=%p "$2" | sane_grep " " ;} && + expected=$1 + set -- $(git log --reverse --format=%s "$2") + test "$expected" = "$*" +} + +reset_rebase () { + git rebase --abort # may fail; ignore exit code + git reset --hard && + git clean -f +} diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh new file mode 100755 index 000..c4b32db --- /dev/null +++ b/t/t3420-rebase-topology-linear.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +test_description='basic rebase topology tests' +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-rebase.sh + +# a---b---c +# \ +# d---e +test_expect_success 'setup' ' + test_commit a && + test_commit b && + test_commit c && + git checkout b && + test_commit d && + test_commit e +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result "simple rebase $*" " + reset_rebase && + git rebase $* c e && + test_cmp_rev c HEAD~2 && + test_linear_range 'd e' c.. + " +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* is no-op if upstream is an ancestor" " + reset_rebase && + git rebase $* b e && + test_cmp_rev e HEAD + " +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" " + reset_rebase && + git rebase $* -f b e && + ! test_cmp_rev e HEAD && + test_cmp_rev b HEAD~2 && + test_linear_range 'd e' b.. + " +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" " + reset_rebase && + git rebase $* e b && + test_cmp_rev e HEAD + " +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_done -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/7] add tests for rebasing root
--- t/t3420-rebase-topology-linear.sh | 129 ++ 1 file changed, 129 insertions(+) diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index 40fe264..2429aa8 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -218,4 +218,133 @@ test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +# m +# / +# a---b---c---g +# +# x---y---B +# +# uppercase = cherry-picked +# m = reverted b +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for test involving root' ' + git checkout b && + revert m b && + git checkout --orphan disjoint && + git rm -rf . && + test_commit x && + test_commit y && + cherry_pick B b +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --onto --root" " + reset_rebase && + git rebase $* --onto c --root y && + test_cmp_rev c HEAD~2 && + test_linear_range 'x y' c.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* without --onto --root with disjoint history" " + reset_rebase && + git rebase $* c y && + test_cmp_rev c HEAD~2 && + test_linear_range 'x y' c.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --onto --root drops patch in onto" " + reset_rebase && + git rebase $* --onto m --root B && + test_cmp_rev m HEAD~2 && + test_linear_range 'x y' m.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --onto --root with merge-base does not go to root" " + reset_rebase && + git rebase $* --onto m --root g && + test_cmp_rev m HEAD~2 && + test_linear_range 'c g' m.. + " +} + +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* without --onto --root with disjoint history drops patch in onto" " + reset_rebase && + git rebase $* m B && + test_cmp_rev m HEAD~2 && + test_linear_range 'x y' m.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --root on linear history is a no-op" " + reset_rebase && + git rebase $* --root c && + test_cmp_rev c HEAD + " +} +test_run_rebase failure '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* -f --root on linear history causes re-write" " + reset_rebase && + git rebase $* -f --root c && + ! test_cmp_rev a HEAD~2 && + test_linear_range 'a b c' HEAD + " +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/7] t3406: modernize style
Update the following: - Quote 'setup' - Remove blank lines within test case body - Use test_commit instead of custom quick_one - Create branch "topic" from tag created by test_commit --- t/t3406-rebase-message.sh | 30 +- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index e6a9a0d..fe8c27f 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -4,27 +4,17 @@ test_description='messages from rebase operation' . ./test-lib.sh -quick_one () { - echo "$1" >"file$1" && - git add "file$1" && - test_tick && - git commit -m "$1" -} +test_expect_success 'setup' ' + test_commit O fileO && + test_commit X fileX && + test_commit A fileA && + test_commit B fileB && + test_commit Y fileY && -test_expect_success setup ' - quick_one O && - git branch topic && - quick_one X && - quick_one A && - quick_one B && - quick_one Y && - - git checkout topic && - quick_one A && - quick_one B && - quick_one Z && + git checkout -b topic O && + git cherry-pick A B && + test_commit Z fileZ && git tag start - ' cat >expect <<\EOF @@ -34,12 +24,10 @@ Committed: 0003 Z EOF test_expect_success 'rebase -m' ' - git rebase -m master >report && sed -n -e "/^Already applied: /p" \ -e "/^Committed: /p" report >actual && test_cmp expect actual - ' test_expect_success 'rebase --stat' ' -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/7] add tests for rebasing of empty commits
--- t/t3401-rebase-partial.sh | 24 t/t3420-rebase-topology-linear.sh | 58 +++ 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 58f4823..7ba1797 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr test_path_is_missing .git/rebase-merge ' -test_expect_success 'rebase ignores empty commit' ' - git reset --hard A && - git commit --allow-empty -m empty && - test_commit D && - git rebase C && - test "$(git log --format=%s C..)" = "D" -' - -test_expect_success 'rebase --keep-empty' ' - git reset --hard D && - git rebase --keep-empty C && - test "$(git log --format=%s C..)" = "D -empty" -' - -test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' - git reset --hard A && - git commit --allow-empty -m also-empty && - git rebase --keep-empty D && - test "$(git log --format=%s A..)" = "also-empty -D -empty" -' - test_done diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index 1152491..40fe264 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -160,4 +160,62 @@ test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +# a---b---c---j! +# \ +# d---k!--l +# +# ! = empty +test_expect_success 'setup of linear history for empty commit tests' ' + git checkout c && + make_empty j && + git checkout d && + make_empty k && + test_commit l +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* drops empty commit" " + reset_rebase && + git rebase $* c l && + test_cmp_rev c HEAD~2 && + test_linear_range 'd l' c.. + " +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --keep-empty" " + reset_rebase && + git rebase $* --keep-empty c l && + test_cmp_rev c HEAD~3 && + test_linear_range 'd k l' c.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* --keep-empty keeps empty even if already in upstream" " + reset_rebase && + git rebase $* --keep-empty j l && + test_cmp_rev j HEAD~3 && + test_linear_range 'd k l' j.. + " +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + test_done -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: zsh: improve bash script loading
On Wed, May 29, 2013 at 1:17 AM, Johannes Sixt wrote: > Am 5/29/2013 5:24, schrieb Felipe Contreras: >> +if [ -z "$script" ]; then >> + local -a locations >> + locations=( >> + '/etc/bash_completion.d/git' # fedora, old debian >> + '/usr/share/bash-completion/completions/git' # arch, ubuntu, >> new debian >> + '/usr/share/bash-completion/git' # gentoo >> + $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash >> + ) > > Won't you need > > local e > > here, or does it not matter? You are right, otherwise it would be in the user's shell. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] prompt: fix for simple rebase
When we are rebasing without options ('am' mode), the head rebased lives in '$g/rebase-apply/head-name', so lets use that information so it's reported the same way as if we were doing other rebases (-i or -m). Signed-off-by: Felipe Contreras --- contrib/completion/git-prompt.sh | 2 ++ t/t9903-bash-prompt.sh | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index eaf5c36..bbf7657 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -279,6 +279,7 @@ __git_ps1 () step=$(cat "$g/rebase-apply/next") total=$(cat "$g/rebase-apply/last") if [ -f "$g/rebase-apply/rebasing" ]; then + b="$(cat "$g/rebase-apply/head-name")" r="|REBASE" elif [ -f "$g/rebase-apply/applying" ]; then r="|AM" @@ -295,6 +296,7 @@ __git_ps1 () r="|BISECTING" fi + test -n "$b" || b="$(git symbolic-ref HEAD 2>/dev/null)" || { detached=yes b="$( diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 083b319..15521cc 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -276,7 +276,7 @@ test_expect_success 'prompt - rebase merge' ' ' test_expect_success 'prompt - rebase' ' - printf " ((t2)|REBASE 1/3)" > expected && + printf " (b2|REBASE 1/3)" > expected && git checkout b2 && test_when_finished "git checkout master" && test_must_fail git rebase b1 b2 && -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] patch-ids: check modified paths before calculating diff
On Sun, May 19, 2013 at 02:17:35PM +0100, John Keeping wrote: > When using "git cherry" or "git log --cherry-pick" we often have a small > number of commits on one side and a large number on the other. In > revision.c::cherry_pick_list we store the patch IDs for the small side > before comparing the large side to this. > > In this case, it is likely that only a small number of paths are touched > by the commits on the smaller side of the range and by storing these we > can ignore many commits on the other side before unpacking blobs and > diffing them. There are two observations that make your scheme work: 1. For something like --cherry-pick, we do not even care about the actual patch-ids, only whether there are matches from the left and right sides. 2. Comparing the sets of paths touched by two commits is often sufficient to realize they do not have the same patch-ids. But I think those observations mean we can do even better than calculating the real patch-id for the small side at all. Imagine we have both "loose" and "strict" patch-ids, where the loose one is much faster to compute, and has that property that a match _might_ mean we have the same strict patch-id, but a non-match means we definitely do not have the same strict patch-id. I think such a loose patch-id could just be a hash of the filenames that were changed by the patch (e.g., the first 32-bits of the sha1 of the concatenated filenames). Computing that should be about as expensive as a tree-diff. Per observation 2 above, if two commits do not have the same loose id, we know that they cannot possibly have the same strict id. Then we can forget about the smaller-side and bigger-side entirely, and just do something like: 1. Make a sorted list (or hash table) of loose ids for one side. 2. For each commit on the other side, calculate its loose id and look that up in the sorted list. If no hits, we know that there is no match. For any hits, lazily calculate (and cache) the strict patch id for both sides and compare as usual. In the best case, we compute no patch-ids at all. And even for the average case, I'd expect our lazy calculation to only have to compute a handful of ids. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: zsh: improve bash script loading
Am 5/29/2013 5:24, schrieb Felipe Contreras: > +if [ -z "$script" ]; then > + local -a locations > + locations=( > + '/etc/bash_completion.d/git' # fedora, old debian > + '/usr/share/bash-completion/completions/git' # arch, ubuntu, > new debian > + '/usr/share/bash-completion/git' # gentoo > + $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash > + ) Won't you need local e here, or does it not matter? > + for e in $locations; do > + test -f $e && script="$e" && break > + done > +fi -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
:-) On Tue, May 28, 2013 at 11:05 PM, Felipe Contreras wrote: > On Wed, May 29, 2013 at 12:51 AM, Martin von Zweigbergk > wrote: >> On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras >> wrote: > >>> One change splits, the other change fixes, what's wrong with that? >> >> I didn't say there was anything wrong. I was asking if the bug was >> there before (and I didn't see an answer when Junio asked). > > Why wouldn't it be before? Did I mention a commit that introduced a > problem? No. Did any patch in this series introduce a problem? No. > > All we've done in this series is 1) reorganize the code without > introducing *ANY* functional changes, and 2) fix a bug. > > If you see 1) introducing a problem, or 2) introducing a problem, then > mention that in *those* patches. If there is no problem with 1) or 2) > then it follows the problem already exists. > > -- > Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
On Wed, May 29, 2013 at 12:51 AM, Martin von Zweigbergk wrote: > On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras > wrote: >> One change splits, the other change fixes, what's wrong with that? > > I didn't say there was anything wrong. I was asking if the bug was > there before (and I didn't see an answer when Junio asked). Why wouldn't it be before? Did I mention a commit that introduced a problem? No. Did any patch in this series introduce a problem? No. All we've done in this series is 1) reorganize the code without introducing *ANY* functional changes, and 2) fix a bug. If you see 1) introducing a problem, or 2) introducing a problem, then mention that in *those* patches. If there is no problem with 1) or 2) then it follows the problem already exists. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] rebase: fix cherry-pick invocations
Junio C Hamano wrote: > Junio C Hamano writes: > > > Felipe Contreras writes: > > > >> So that all the tests pass. > >> > >> Signed-off-by: Felipe Contreras > >> --- > >> git-rebase--cherry.sh | 17 - > >> 1 file changed, 16 insertions(+), 1 deletion(-) > >> > >> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh > >> index ca78b1b..c3a2ac9 100644 > >> --- a/git-rebase--cherry.sh > >> +++ b/git-rebase--cherry.sh > >> @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root > >> mkdir "$state_dir" || die "Could not create temporary $state_dir" > >> : > "$state_dir"/cherry || die "Could not mark as cherry" > >> > >> +if test -n "$rebase_root" > >> +then > >> + revisions="$onto...$orig_head" > >> +else > >> + revisions="$upstream...$orig_head" > >> +fi > > > > "So that all the tests pass" needs a bit more explanation to say for > > cherry-pick codepath why and how two-dot range fails and why and how > > three-dot variant with --right-only fixes it. What are the problematic > > cases? > > Yikes, sorry, this was me being slow. Walking A...B range with > right-only and --cherry applied will filter the duplicates, which is > wat you want, I think, and walking A..B range will not do the > filtering for you. That's right. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] rebase: fix cherry-pick invocations
Junio C Hamano wrote: > Felipe Contreras writes: > > > So that all the tests pass. > > > > Signed-off-by: Felipe Contreras > > --- > > git-rebase--cherry.sh | 17 - > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh > > index ca78b1b..c3a2ac9 100644 > > --- a/git-rebase--cherry.sh > > +++ b/git-rebase--cherry.sh > > @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root > > mkdir "$state_dir" || die "Could not create temporary $state_dir" > > : > "$state_dir"/cherry || die "Could not mark as cherry" > > > > +if test -n "$rebase_root" > > +then > > + revisions="$onto...$orig_head" > > +else > > + revisions="$upstream...$orig_head" > > +fi > > "So that all the tests pass" needs a bit more explanation to say for > cherry-pick codepath why and how two-dot range fails and why and how > three-dot variant with --right-only fixes it. What are the problematic > cases? There's too many failures to count. We are blindingly applying a series of commits on top of another without checking if they are merges or if they already exist in the destination branch. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] rebase: fix sequence continuation
Junio C Hamano wrote: > Felipe Contreras writes: > > > We are not in am mode. > > That may make sense, but shouldn't this part be like so from the > very beginning? In other words, this looks like an "oops, 1/5 was > buggy and this is a hotfix". How can 1/5 be introducing a bug? It's merely splitting the code without introducing *ANY* functional changes. The bug is already there in 'master'. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras wrote: > On Wed, May 29, 2013 at 12:33 AM, Martin von Zweigbergk > wrote: >> As Junio asked in the previous iteration, shouldn't this have been in >> the first patch? > > No, the first patch is splitting the code without introducing any > functional changes. > > This is fixing a bug that already exists, we could fix it before the > split, or after, but mixing the split and the fix at the same time is > a no-no. Oh, now I remember. I ran into that bug once. > One change splits, the other change fixes, what's wrong with that? I didn't say there was anything wrong. I was asking if the bug was there before (and I didn't see an answer when Junio asked). Thanks -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 4/8] rebase: cherry-pick: fix abort of cherry mode
On Wed, May 29, 2013 at 12:35 AM, Martin von Zweigbergk wrote: > Same here: should this have been in the first patch? If not, do you > know for how long it has been broken (since which commit)? Since day 1 of --keep-empty: 90e1818 git-rebase: add keep_empty flag -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 2/8] rebase: cherry-pick: fix mode storage
On Wed, May 29, 2013 at 12:38 AM, Martin von Zweigbergk wrote: > Actually, are all of 2/8 - 7/8 fixes for things that broke in patch 1/8? No, everything is already broken. Try this: --- a/git-rebase.sh +++ b/git-rebase.sh @@ -78,7 +78,7 @@ state_dir= action= preserve_merges= autosquash= -keep_empty= +keep_empty=yes test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t read_basic_state () { And tell me what happens when you run the test suite: Hint: This[1] is what would happen; everything breaks, not only the tests that check for empty commits. [1] http://article.gmane.org/gmane.comp.version-control.git/225652 -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Has anyone tried to implement git grep --blame?
This would be so much more convenient if git-grep supported it natively: $ git grep -n 'if \(0\)' | perl -pe's/([^:]+):([^:]+).*/`git blame -L $2,$2 $1`/se' d18f76dc (Ævar Arnfjörð Bjarmason 2010-08-17 09:24:38 + 2278) if (0) 65648283 (David Brown 2007-12-25 19:56:29 -0800 433) if (0) { I.e. with all the coloring/pager interaction. Some Googling around reveals people piping things to git-blame like that, but has anyone made a stab at a smarter implementation (that would know to blame the whole file if it had lots of hits etc..). Don't know if I have time myself, but I'd be very pleased if someone hacked that up. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
On Wed, May 29, 2013 at 12:33 AM, Martin von Zweigbergk wrote: > As Junio asked in the previous iteration, shouldn't this have been in > the first patch? No, the first patch is splitting the code without introducing any functional changes. This is fixing a bug that already exists, we could fix it before the split, or after, but mixing the split and the fix at the same time is a no-no. One change splits, the other change fixes, what's wrong with that? This way it's easy to see what each patch does. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 2/8] rebase: cherry-pick: fix mode storage
Actually, are all of 2/8 - 7/8 fixes for things that broke in patch 1/8? On Tue, May 28, 2013 at 9:16 PM, Felipe Contreras wrote: > We don't use the 'rebase-apply'. > > Signed-off-by: Felipe Contreras > --- > git-rebase--cherrypick.sh | 4 > git-rebase.sh | 5 - > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh > index cbf80f9..51354af 100644 > --- a/git-rebase--cherrypick.sh > +++ b/git-rebase--cherrypick.sh > @@ -18,6 +18,9 @@ esac > > test -n "$rebase_root" && root_flag=--root > > +mkdir "$state_dir" || die "Could not create temporary $state_dir" > +: > "$state_dir"/cherrypick || die "Could not mark as cherrypick" > + > # we have to do this the hard way. git format-patch completely squashes > # empty commits and even if it didn't the format doesn't really lend > # itself well to recording empty patches. fortunately, cherry-pick > @@ -32,3 +35,4 @@ then > fi > > move_to_original_branch > +rm -rf "$state_dir" > diff --git a/git-rebase.sh b/git-rebase.sh > index f929ca3..76900a0 100755 > --- a/git-rebase.sh > +++ b/git-rebase.sh > @@ -174,6 +174,9 @@ then > then > type=interactive > interactive_rebase=explicit > + elif test -f "$merge_dir"/cherrypick > + then > + type=cherrypick > else > type=merge > fi > @@ -382,7 +385,7 @@ then > elif test -n "$keep_empty" > then > type=cherrypick > - state_dir="$apply_dir" > + state_dir="$merge_dir" > else > type=am > state_dir="$apply_dir" > -- > 1.8.3.rc3.312.g47657de > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 4/8] rebase: cherry-pick: fix abort of cherry mode
Same here: should this have been in the first patch? If not, do you know for how long it has been broken (since which commit)? On Tue, May 28, 2013 at 9:16 PM, Felipe Contreras wrote: > Signed-off-by: Felipe Contreras > --- > git-rebase.sh | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/git-rebase.sh b/git-rebase.sh > index 76900a0..9b5d78b 100755 > --- a/git-rebase.sh > +++ b/git-rebase.sh > @@ -335,6 +335,7 @@ skip) > run_specific_rebase > ;; > abort) > + test "$type" == "cherrypick" && git cherry-pick --abort > git rerere clear > read_basic_state > case "$head_name" in > -- > 1.8.3.rc3.312.g47657de > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
As Junio asked in the previous iteration, shouldn't this have been in the first patch? On Tue, May 28, 2013 at 9:16 PM, Felipe Contreras wrote: > We are not in am mode. > > Signed-off-by: Felipe Contreras > --- > git-rebase--cherrypick.sh | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh > index 51354af..2fa4993 100644 > --- a/git-rebase--cherrypick.sh > +++ b/git-rebase--cherrypick.sh > @@ -5,13 +5,15 @@ > > case "$action" in > continue) > - git am --resolved --resolvemsg="$resolvemsg" && > - move_to_original_branch > + git cherry-pick --continue && > + move_to_original_branch && > + rm -rf "$state_dir" > exit > ;; > skip) > - git am --skip --resolvemsg="$resolvemsg" && > - move_to_original_branch > + git cherry-pick --skip && > + move_to_original_branch && > + rm -rf "$state_dir" > exit > ;; > esac > -- > 1.8.3.rc3.312.g47657de > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #05; Mon, 20)
On Tue, May 21, 2013 at 09:19:22AM +0200, Thomas Rast wrote: > Junio C Hamano writes: > > > * tr/test-v-and-v-subtest-only (2013-05-16) 6 commits > > - test-lib: support running tests under valgrind in parallel > > - test-lib: allow prefixing a custom string before "ok N" etc. > > - test-lib: valgrind for only tests matching a pattern > > - test-lib: verbose mode for only tests matching a pattern > > - test-lib: refactor $GIT_SKIP_TESTS matching > > - test-lib: enable MALLOC_* for the actual tests > > > > Allows N instances of tests run in parallel, each running 1/N parts > > of the test suite under Valgrind, to speed things up. > > > > The tip one may be useful in practice but is a tad ugly ;-) > > I was hoping for some success stories ;-) Sorry, none yet, as I am just returning from vacation. Thanks very much for working on it, though. I'm looking forward to trying it out for real. > I think Peff (who I stupidly managed to not Cc in the series, there's > another git-send-email usability issue there) asked for the third from > the tip, which lets you run valgrind only on a certain test. (For > example, if you've already had two coffees while your computer found out > which test it was, this is a much faster way of seeing if the failure > disappeared.) Like Junio, I find the tip one a bit gross, and I do not expect to ever use it myself. If I want to run more than one test, I am much more likely to be running the whole suite, which already parallelizes nicely. The "run one test under valgrind" may be used when double-checking a failed test, as you mention. But the use case I had in mind was actually more like: 1. Notice a heisenbug/segfault. 2. Write a new test for it, that logically goes into t. It may become t.85. 3. Run t.85 under valgrind to confirm the bug. 4. Fix, and re-run t.85 to confirm the fix. This is the same procedure for a non-valgrind bugfix, and usually it is not too big a deal to run all of tests 1-84, because it only wastes a few seconds (and you need to run them anyway to create the proper state). But under valgrind, running 1-84 may take minutes. With your patches, it is no more painful than the non-valgrind case. I just posted some feedback as a reply to the series itself, but certainly I think the first four in the series (with that feedback addressed) would be great to have. The top two I do not care about, but I also do not mind if they are there for people to play with. > So one obvious way of going forward is cooking this for a while and > seeing whether people find the one-test-only or the massively-parallel > feature useful (or maybe both). One of the annoying things about test improvements is that you often want them while working on something else. In the scenario above, it does not help me when building a fix on "master" that the feature is cooking in "next" or on a topic branch. But since the feature should not hurt anybody who does not use it, and it involves no change to actual git code, only test scripts, I would be inclined to move it fairly quickly to master, and let it prove its worth there. We can always improve it or revert it as a failed experiment later if we wish, without worrying about compatibility issues. The only downside would be the potential for textual conflicts with other people improving the test scripts. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] test-lib: verbose mode for only tests matching a pattern
On Wed, May 29, 2013 at 01:00:00AM -0400, Jeff King wrote: > So we see 83 and 84 non-verbose, which is good. And we see the actual > output from 85 (the output from a "git checkout"). But we do not see the > "expecting success" for it. We see it for the _next_ test, which we > should not see at all. So I think your toggling is happening in the > wrong spot, but I haven't looked further than that. I think you want something more like: diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 5251009..75351f5 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -349,6 +349,7 @@ test_expect_failure () { export test_prereq if ! test_skip "$@" then + toggle_verbose say >&3 "checking known breakage: $2" if test_run_ "$2" expecting_failure then @@ -367,6 +368,7 @@ test_expect_success () { export test_prereq if ! test_skip "$@" then + toggle_verbose say >&3 "expecting success: $2" if test_run_ "$2" then diff --git a/t/test-lib.sh b/t/test-lib.sh index b4e81bc..165e84e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -400,7 +400,6 @@ setup_test_eval () { setup_test_eval () { setup_malloc_check toggle_valgrind - toggle_verbose } teardown_test_eval () { teardown_malloc_check However, I'm not sure the toggle is the right thing. However, the whole toggle thing seems weird to me, as there is a big "gap" between finishing test X and starting test X+1 where we inherit the verbosity (and valgrind) settings from X. In general we frown upon doing much at all outside of test_expect_*, but I would think that: test_expect_success 'one' '...' git foo test_expect_success 'two' '...' when run with "--valgrind-only=1" would not run the intermediate "git foo" with valgrind. I would have expected the implementation to be more like: maybe_turn_on_valgrind maybe_turn_on_verbose run_the_actual_test maybe_turn_off_verbose maybe_turn_off_valgrind rather than relying on the next test to return to normal. I doubt that it matters too much in practice, though. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] test-lib: verbose mode for only tests matching a pattern
On Thu, May 16, 2013 at 10:50:14PM +0200, Thomas Rast wrote: > With the new --verbose-only= option, one can enable --verbose > at a per-test granularity. The pattern is matched against the test > number, e.g. > > ./t-basic.sh --verbose-only='2[0-2]' > > to see only the full output of test 20-22, while showing the rest in the > one-liner format. > > This is arguably not *too* useful on its own, but makes the next patch > easier to follow. Hmm, I don't think this is quite right. Try: ./t4052-stat-output.sh --verbose-only=85 The script and test number aren't important; I just picked these at random, but they show the issue clearly. The output I get is: [...] ok 83 - log respects prefix greater than COLUMNS (big change) ok 84 - log --graph respects prefix greater than COLUMNS (big change) Switched to a new branch 'branch' ok 85 - merge --stat respects COLUMNS (big change) expecting success: COLUMNS=100 git merge --stat --no-ff master >output && grep " | " output >actual test_cmp expect actual ok 86 - merge --stat respects COLUMNS (long filename) So we see 83 and 84 non-verbose, which is good. And we see the actual output from 85 (the output from a "git checkout"). But we do not see the "expecting success" for it. We see it for the _next_ test, which we should not see at all. So I think your toggling is happening in the wrong spot, but I haven't looked further than that. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] --valgrind improvements
On Thu, May 16, 2013 at 10:50:11PM +0200, Thomas Rast wrote: > One open issue with the last patch that currently eludes me: if I > combine --valgrind-parallel with any --valgrind=*, there are lots of > errors as (apparently) the valgrind wrapper setups race against each > other. However, without any --valgrind=* (thus defaulting to > 'memcheck') this doesn't happen. I noticed two racy error messages. If you do: cd t && make clean && ./some-test --valgrind-parallel=8 you will get complaints from mkdir about existing directories, as we use mkdir as a poor man's O_EXCL to create lockfiles. These error messages are harmless (we loop and try again), and we should perhaps just squelch the stderr from mkdir. Although that might make weird situations hard to diagnose, like another error that prevents creating the lockfile, so maybe it is better to just live with the extra output (after the directory is built once, it does not happen at all). I also notice: $ ./t4052-* --valgrind-parallel=8 --valgrind=memcheck ... ./t4052-stat-output.sh: 572: ./test-lib.sh: cannot open /home/peff/compile/git/t/valgrind/bin/git-send-email.perl: No such file Line 572 is checking the "#!" line of the _source_ file. So it shouldn't be checking t/valgrind/bin in the first place. It looks like it comes from this loop: for path in $PATH do ls "$path"/git-* 2> /dev/null | while read file do make_valgrind_symlink "$file" done done as t/valgrind/bin seems to be in the $PATH of the parallel sub-processes. Hmm. It looks like you set up the valgrind dir in the parent test process, and _then_ call the sub-processes in parallel. We shouldn't need to do any valgrind setup at all in the subprocesses, should we? They would just be replicating what the parent already did. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?
On Wed, May 29, 2013 at 10:41 AM, Duy Nguyen wrote: > The changes in this area since 1.8.2.3 seem to be Karsten's (I'm not > blaming, just wanted to narrow down the problem). The patterns of > interest seem to be > > !/bin > /bin/* > !/bin/brew > > Without "!/bin" v1.8.3 seems to behave the same as v1.8.2.3. Karsten, the block "/* Abort if the directory is excluded */" in prep_exclude() seems to cause this. I think it goes through the exclude patterns, hits "!/bin", believes the patterns do not make sense in this context and throws all away. I think Øystein's case falls into the same path. Commenting out the block seems to gain the old behavior back (and probably breaks other stuff). Contrary to what Junio said, I'm clueless about this. I wanted to read your series through and eventually gave up. I think I now have the motivation to look at it again this weekend. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 8/8] rebase: use 'cherrypick' mode instead of 'am'
Unless any specific 'git am' options are used. Signed-off-by: Felipe Contreras --- contrib/completion/git-prompt.sh | 2 ++ git-rebase--am.sh | 1 + git-rebase.sh | 9 - t/t3407-rebase-abort.sh| 2 +- t/t5520-pull.sh| 2 +- t/t9106-git-svn-commit-diff-clobber.sh | 2 +- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index eaf5c36..f001463 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -271,6 +271,8 @@ __git_ps1 () total=$(cat "$g/rebase-merge/end") if [ -f "$g/rebase-merge/interactive" ]; then r="|REBASE-i" + elif [ -f "$g/rebase-merge/cherrypick" ]; then + r="|REBASE" else r="|REBASE-m" fi diff --git a/git-rebase--am.sh b/git-rebase--am.sh index ee1b1b9..7a978fd 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -51,6 +51,7 @@ then exit $? fi +test -n "$GIT_QUIET" && git_am_opt="$git_am_opt -q" git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" <"$GIT_DIR/rebased-patches" ret=$? diff --git a/git-rebase.sh b/git-rebase.sh index 9b5d78b..16cc91b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -251,7 +251,6 @@ do ;; -q) GIT_QUIET=t - git_am_opt="$git_am_opt -q" verbose= diffstat= ;; @@ -383,13 +382,13 @@ elif test -n "$do_merge" then type=merge state_dir="$merge_dir" -elif test -n "$keep_empty" +elif test -n "$git_am_opt" then - type=cherrypick - state_dir="$merge_dir" -else type=am state_dir="$apply_dir" +else + type=cherrypick + state_dir="$merge_dir" fi if test -z "$rebase_root" diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh index a6a6c40..2699b08 100755 --- a/t/t3407-rebase-abort.sh +++ b/t/t3407-rebase-abort.sh @@ -96,7 +96,7 @@ testrebase() { ' } -testrebase "" .git/rebase-apply +testrebase "" .git/rebase-merge testrebase " --merge" .git/rebase-merge test_done diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 6af6c63..ec2373b 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -244,7 +244,7 @@ test_expect_success 'setup for avoiding reapplying old patches' ' test_expect_success 'git pull --rebase does not reapply old patches' ' (cd dst && test_must_fail git pull --rebase && -test 1 = $(find .git/rebase-apply -name "000*" | wc -l) +test 1 = $(cat .git/sequencer/todo | wc -l) ) ' diff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh index f6d7ac7..b9cec33 100755 --- a/t/t9106-git-svn-commit-diff-clobber.sh +++ b/t/t9106-git-svn-commit-diff-clobber.sh @@ -92,7 +92,7 @@ test_expect_success 'multiple dcommit from git svn will not clobber svn' " test_expect_success 'check that rebase really failed' ' - test -d .git/rebase-apply + test -d .git/rebase-merge ' test_expect_success 'resolve, continue the rebase and dcommit' " -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 7/8] rebase: cherry-pick: automatically commit stage
When there's changes in the staging area. Just like the other rebase modes. Signed-off-by: Felipe Contreras --- git-rebase--cherrypick.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index ef3224d..0fcf2e1 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -8,6 +8,12 @@ export GIT_CHERRY_PICK_HELP case "$action" in continue) + # do we have anything to commit? + if ! git diff-index --cached --quiet HEAD -- + then + git commit --no-verify -e || + die "Could not commit staged changes." + fi git cherry-pick --continue && move_to_original_branch && rm -rf "$state_dir" -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 6/8] rebase: cherry-pick: fix status messages
Signed-off-by: Felipe Contreras --- git-rebase--cherrypick.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index ab892e6..ef3224d 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -3,6 +3,9 @@ # Copyright (c) 2010 Junio C Hamano. # +GIT_CHERRY_PICK_HELP="$resolvemsg" +export GIT_CHERRY_PICK_HELP + case "$action" in continue) git cherry-pick --continue && -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 4/8] rebase: cherry-pick: fix abort of cherry mode
Signed-off-by: Felipe Contreras --- git-rebase.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-rebase.sh b/git-rebase.sh index 76900a0..9b5d78b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -335,6 +335,7 @@ skip) run_specific_rebase ;; abort) + test "$type" == "cherrypick" && git cherry-pick --abort git rerere clear read_basic_state case "$head_name" in -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 5/8] rebase: cherry-pick: fix command invocations
So that all the tests pass. Signed-off-by: Felipe Contreras --- git-rebase--cherrypick.sh | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index 2fa4993..ab892e6 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root mkdir "$state_dir" || die "Could not create temporary $state_dir" : > "$state_dir"/cherrypick || die "Could not mark as cherrypick" +if test -n "$rebase_root" +then + revisions="$onto...$orig_head" +else + revisions="$upstream...$orig_head" +fi + # we have to do this the hard way. git format-patch completely squashes # empty commits and even if it didn't the format doesn't really lend # itself well to recording empty patches. fortunately, cherry-pick # makes this easy -git cherry-pick --allow-empty "$revisions" +if test -n "$keep_empty" +then + extra="--allow-empty" +else + extra="--skip-empty --cherry-pick" +fi +test -n "$GIT_QUIET" && extra="$extra -q" +test -z "$force_rebase" && extra="$extra --ff" +git cherry-pick --no-merges --right-only --topo-order --do-walk --copy-notes $extra "$revisions" ret=$? if test 0 != $ret -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
We are not in am mode. Signed-off-by: Felipe Contreras --- git-rebase--cherrypick.sh | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index 51354af..2fa4993 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -5,13 +5,15 @@ case "$action" in continue) - git am --resolved --resolvemsg="$resolvemsg" && - move_to_original_branch + git cherry-pick --continue && + move_to_original_branch && + rm -rf "$state_dir" exit ;; skip) - git am --skip --resolvemsg="$resolvemsg" && - move_to_original_branch + git cherry-pick --skip && + move_to_original_branch && + rm -rf "$state_dir" exit ;; esac -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 2/8] rebase: cherry-pick: fix mode storage
We don't use the 'rebase-apply'. Signed-off-by: Felipe Contreras --- git-rebase--cherrypick.sh | 4 git-rebase.sh | 5 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index cbf80f9..51354af 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -18,6 +18,9 @@ esac test -n "$rebase_root" && root_flag=--root +mkdir "$state_dir" || die "Could not create temporary $state_dir" +: > "$state_dir"/cherrypick || die "Could not mark as cherrypick" + # we have to do this the hard way. git format-patch completely squashes # empty commits and even if it didn't the format doesn't really lend # itself well to recording empty patches. fortunately, cherry-pick @@ -32,3 +35,4 @@ then fi move_to_original_branch +rm -rf "$state_dir" diff --git a/git-rebase.sh b/git-rebase.sh index f929ca3..76900a0 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -174,6 +174,9 @@ then then type=interactive interactive_rebase=explicit + elif test -f "$merge_dir"/cherrypick + then + type=cherrypick else type=merge fi @@ -382,7 +385,7 @@ then elif test -n "$keep_empty" then type=cherrypick - state_dir="$apply_dir" + state_dir="$merge_dir" else type=am state_dir="$apply_dir" -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 1/8] rebase: split the cherry-pick stuff
They do something completely different from 'git am', it belongs in a different file. Signed-off-by: Felipe Contreras --- .gitignore| 1 + Makefile | 1 + git-rebase--am.sh | 11 +-- git-rebase--cherrypick.sh | 34 ++ git-rebase.sh | 4 5 files changed, 41 insertions(+), 10 deletions(-) create mode 100644 git-rebase--cherrypick.sh diff --git a/.gitignore b/.gitignore index 6669bf0..a171533 100644 --- a/.gitignore +++ b/.gitignore @@ -113,6 +113,7 @@ /git-read-tree /git-rebase /git-rebase--am +/git-rebase--cherrypick /git-rebase--interactive /git-rebase--merge /git-receive-pack diff --git a/Makefile b/Makefile index 0f931a2..800e42d 100644 --- a/Makefile +++ b/Makefile @@ -469,6 +469,7 @@ SCRIPT_SH += git-web--browse.sh SCRIPT_LIB += git-mergetool--lib SCRIPT_LIB += git-parse-remote SCRIPT_LIB += git-rebase--am +SCRIPT_LIB += git-rebase--cherrypick SCRIPT_LIB += git-rebase--interactive SCRIPT_LIB += git-rebase--merge SCRIPT_LIB += git-sh-setup diff --git a/git-rebase--am.sh b/git-rebase--am.sh index f84854f..ee1b1b9 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -19,15 +19,7 @@ esac test -n "$rebase_root" && root_flag=--root ret=0 -if test -n "$keep_empty" -then - # we have to do this the hard way. git format-patch completely squashes - # empty commits and even if it didn't the format doesn't really lend - # itself well to recording empty patches. fortunately, cherry-pick - # makes this easy - git cherry-pick --allow-empty "$revisions" - ret=$? -else + rm -f "$GIT_DIR/rebased-patches" git format-patch -k --stdout --full-index --ignore-if-in-upstream \ @@ -63,7 +55,6 @@ else ret=$? rm -f "$GIT_DIR/rebased-patches" -fi if test 0 != $ret then diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh new file mode 100644 index 000..cbf80f9 --- /dev/null +++ b/git-rebase--cherrypick.sh @@ -0,0 +1,34 @@ +#!/bin/sh +# +# Copyright (c) 2010 Junio C Hamano. +# + +case "$action" in +continue) + git am --resolved --resolvemsg="$resolvemsg" && + move_to_original_branch + exit + ;; +skip) + git am --skip --resolvemsg="$resolvemsg" && + move_to_original_branch + exit + ;; +esac + +test -n "$rebase_root" && root_flag=--root + +# we have to do this the hard way. git format-patch completely squashes +# empty commits and even if it didn't the format doesn't really lend +# itself well to recording empty patches. fortunately, cherry-pick +# makes this easy +git cherry-pick --allow-empty "$revisions" +ret=$? + +if test 0 != $ret +then + test -d "$state_dir" && write_basic_state + exit $ret +fi + +move_to_original_branch diff --git a/git-rebase.sh b/git-rebase.sh index 2c692c3..f929ca3 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -379,6 +379,10 @@ elif test -n "$do_merge" then type=merge state_dir="$merge_dir" +elif test -n "$keep_empty" +then + type=cherrypick + state_dir="$apply_dir" else type=am state_dir="$apply_dir" -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 0/8] rebase: new cherry-pick mode
Hi, After the fixes I did to cherry-pick, it's now fully usable for 'git rebase' and can be used to replace 'git am' for most cases. We already rely on cherry-pick for the 'am' mode, but only when using the --keep-empty option, and when in such mode the behavior of 'git rebase' changes completely; more specifically; it's completely broken. Manually enabling --keep-empty to be the default and running the test-suite shows a huge lot of failures. After fixing the --keep-empty option by creating a new cherry-pick mode, this patch series uses this new mode instead of the 'am' mode, and everything works. There's only two tests that fail, one because the output of the shell prompt changes a bit, and the other I have not yet investigated. This brings us one step closer to replace scripts with C code. Felipe Contreras (8): rebase: split the cherry-pick stuff rebase: cherry-pick: fix mode storage rebase: cherry-pick: fix sequence continuation rebase: cherry-pick: fix abort of cherry mode rebase: cherry-pick: fix command invocations rebase: cherry-pick: fix status messages rebase: cherry-pick: automatically commit stage rebase: use 'cherrypick' mode instead of 'am' .gitignore | 1 + Makefile | 1 + contrib/completion/git-prompt.sh | 2 ++ git-rebase--am.sh | 12 ++- git-rebase--cherrypick.sh | 64 ++ git-rebase.sh | 11 -- t/t3407-rebase-abort.sh| 2 +- t/t5520-pull.sh| 2 +- t/t9106-git-svn-commit-diff-clobber.sh | 2 +- 9 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 git-rebase--cherrypick.sh -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 8/8] revert/cherry-pick: add --skip option
Akin to 'am --skip' and 'rebase --skip'. Signed-off-by: Felipe Contreras --- Documentation/git-cherry-pick.txt | 1 + Documentation/git-revert.txt | 1 + Documentation/sequencer.txt | 3 +++ builtin/revert.c | 6 ++ sequencer.c | 32 +--- sequencer.h | 3 ++- t/t3510-cherry-pick-sequence.sh | 12 7 files changed, 54 insertions(+), 4 deletions(-) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index da0bd81..d95c63c 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git cherry-pick' [-q] [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] ... 'git cherry-pick' --continue +'git cherry-pick' --skip 'git cherry-pick' --quit 'git cherry-pick' --abort diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt index 98a8e7a..52e146e 100644 --- a/Documentation/git-revert.txt +++ b/Documentation/git-revert.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git revert' [-q] [--[no-]edit] [-n] [-m parent-number] [-s] ... 'git revert' --continue +'git revert' --skip 'git revert' --quit 'git revert' --abort diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt index 5747f44..df2d355 100644 --- a/Documentation/sequencer.txt +++ b/Documentation/sequencer.txt @@ -3,6 +3,9 @@ '.git/sequencer'. Can be used to continue after resolving conflicts in a failed cherry-pick or revert. +--skip:: + Skip the current commit, and then continue. + --quit:: Forget about the current operation in progress. Can be used to clear the sequencer state after a failed cherry-pick or diff --git a/builtin/revert.c b/builtin/revert.c index d63b4a6..6afd990 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -99,11 +99,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) int remove_state = 0; int contin = 0; int rollback = 0; + int skip = 0; struct option options[] = { OPT__QUIET(&opts->quiet, N_("suppress progress reporting")), OPT_BOOLEAN(0, "quit", &remove_state, N_("end revert or cherry-pick sequence")), OPT_BOOLEAN(0, "continue", &contin, N_("resume revert or cherry-pick sequence")), OPT_BOOLEAN(0, "abort", &rollback, N_("cancel revert or cherry-pick sequence")), + OPT_BOOLEAN(0, "skip", &skip, N_("skip current commit in the sequence")), OPT_BOOLEAN('n', "no-commit", &opts->no_commit, N_("don't automatically commit")), OPT_BOOLEAN('e', "edit", &opts->edit, N_("edit the commit message")), OPT_NOOP_NOARG('r', NULL), @@ -160,6 +162,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) opts->subcommand = REPLAY_CONTINUE; else if (rollback) opts->subcommand = REPLAY_ROLLBACK; + else if (skip) + opts->subcommand = REPLAY_SKIP; else opts->subcommand = REPLAY_NONE; @@ -170,6 +174,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) this_operation = "--quit"; else if (opts->subcommand == REPLAY_CONTINUE) this_operation = "--continue"; + else if (opts->subcommand == REPLAY_SKIP) + this_operation = "--skip"; else { assert(opts->subcommand == REPLAY_ROLLBACK); this_operation = "--abort"; diff --git a/sequencer.c b/sequencer.c index b7391d6..46bbe29 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1182,7 +1182,9 @@ static int continue_single_pick(void) return run_command_v_opt(argv, RUN_GIT_CMD); } -static int sequencer_continue(struct replay_opts *opts) +static void add_rewritten(unsigned char *from, unsigned char *to); + +static int sequencer_continue(struct replay_opts *opts, int skip) { struct commit_list *todo_list = NULL; @@ -1201,7 +1203,7 @@ static int sequencer_continue(struct replay_opts *opts) } if (index_differs_from("HEAD", 0)) return error_dirty_index(opts); - { + if (!skip) { unsigned char to[20]; if (!read_ref("HEAD", to)) add_rewritten(todo_list->item->object.sha1, to); @@ -1210,6 +1212,28 @@ static int sequencer_continue(struct replay_opts *opts) return pick_commits(todo_list, opts); } +static int sequencer_skip(struct replay_opts *opts) +{ + const char *argv[4]; /* reset --hard HEAD + NULL */ + struct string_list merge_rr = STRING_LIST_INIT_DUP; + int ret; + + if (setup_rerere(&merge_rr, 0) >= 0) { + rerere_clear(&merge_rr); + string_list_clear(&m
[PATCH v2 7/8] revert/cherry-pick: add --quiet option
Signed-off-by: Felipe Contreras --- Documentation/git-cherry-pick.txt | 6 +- Documentation/git-revert.txt | 6 +- builtin/revert.c | 1 + sequencer.c | 9 ++--- sequencer.h | 1 + 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index fccd936..da0bd81 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -8,7 +8,7 @@ git-cherry-pick - Apply the changes introduced by some existing commits SYNOPSIS [verse] -'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] ... +'git cherry-pick' [-q] [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] ... 'git cherry-pick' --continue 'git cherry-pick' --quit 'git cherry-pick' --abort @@ -51,6 +51,10 @@ OPTIONS feed all ... arguments to a single revision walk (see a later example that uses 'maint master..next'). +-q:: +--quiet:: + Quiet, suppress feedback messages. + -e:: --edit:: With this option, 'git cherry-pick' will let you edit the commit diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt index f79c9d8..98a8e7a 100644 --- a/Documentation/git-revert.txt +++ b/Documentation/git-revert.txt @@ -8,7 +8,7 @@ git-revert - Revert some existing commits SYNOPSIS [verse] -'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] ... +'git revert' [-q] [--[no-]edit] [-n] [-m parent-number] [-s] ... 'git revert' --continue 'git revert' --quit 'git revert' --abort @@ -40,6 +40,10 @@ OPTIONS default, see linkgit:git-rev-list[1] and its '--no-walk' option. +-q:: +--quiet:: + Quiet, suppress feedback messages. + -e:: --edit:: With this option, 'git revert' will let you edit the commit diff --git a/builtin/revert.c b/builtin/revert.c index b977124..d63b4a6 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -100,6 +100,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) int contin = 0; int rollback = 0; struct option options[] = { + OPT__QUIET(&opts->quiet, N_("suppress progress reporting")), OPT_BOOLEAN(0, "quit", &remove_state, N_("end revert or cherry-pick sequence")), OPT_BOOLEAN(0, "continue", &contin, N_("resume revert or cherry-pick sequence")), OPT_BOOLEAN(0, "abort", &rollback, N_("cancel revert or cherry-pick sequence")), diff --git a/sequencer.c b/sequencer.c index cc9c2bc..b7391d6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -466,6 +466,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, argv_array_init(&array); argv_array_push(&array, "commit"); argv_array_push(&array, "-n"); + if (opts->quiet) + argv_array_push(&array, "-q"); if (opts->signoff) argv_array_push(&array, "-s"); @@ -705,9 +707,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts->skip_empty && is_index_unchanged() == 1) { - warning(_("skipping %s... %s"), - find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV), - msg.subject); + if (!opts->quiet) + warning(_("skipping %s... %s"), + find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV), + msg.subject); goto leave; } allow = allow_empty(opts, commit); diff --git a/sequencer.h b/sequencer.h index 6cc072c..41e19d0 100644 --- a/sequencer.h +++ b/sequencer.h @@ -37,6 +37,7 @@ struct replay_opts { int keep_redundant_commits; int skip_empty; int copy_notes; + int quiet; int mainline; -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/8] sequencer: run post-rewrite hook
As we should. Signed-off-by: Felipe Contreras --- sequencer.c | 45 - 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index c217716..3aa480e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -127,6 +127,37 @@ static void add_rewritten(unsigned char *from, unsigned char *to) rewritten.nr++; } +static void run_rewrite_hook(void) +{ + struct strbuf buf = STRBUF_INIT; + struct child_process proc; + const char *argv[3]; + int code, i; + + argv[0] = find_hook("post-rewrite"); + if (!argv[0]) + return; + + argv[1] = "rebase"; + 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; + for (i = 0; i < rewritten.nr; i++) { + struct rewritten_list_item *item = &rewritten.items[i]; + strbuf_addf(&buf, "%s %s\n", sha1_to_hex(item->from), sha1_to_hex(item->to)); + } + write_in_full(proc.in, buf.buf, buf.len); + close(proc.in); + finish_command(&proc); +} + static void remove_sequencer_state(void) { struct strbuf seq_dir = STRBUF_INIT; @@ -1099,6 +1130,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) } } + run_rewrite_hook(); + /* * Sequence of picks finished successfully; cleanup by * removing the .git/sequencer directory @@ -1136,14 +1169,24 @@ static int sequencer_continue(struct replay_opts *opts) } if (index_differs_from("HEAD", 0)) return error_dirty_index(opts); + { + unsigned char to[20]; + if (!read_ref("HEAD", to)) + add_rewritten(todo_list->item->object.sha1, to); + } todo_list = todo_list->next; return pick_commits(todo_list, opts); } static int single_pick(struct commit *cmit, struct replay_opts *opts) { + int ret; setenv(GIT_REFLOG_ACTION, action_name(opts), 0); - return do_pick_commit(cmit, opts); + ret = do_pick_commit(cmit, opts); + if (ret) + return ret; + run_rewrite_hook(); + return 0; } int sequencer_pick_revisions(struct replay_opts *opts) -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/8] cherry-pick: add support to copy notes
Signed-off-by: Felipe Contreras --- builtin/revert.c | 2 ++ sequencer.c | 33 - sequencer.h | 1 + t/t3500-cherry.sh | 32 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/builtin/revert.c b/builtin/revert.c index 0e5ce71..b977124 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -119,6 +119,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_END(), OPT_END(), OPT_END(), + OPT_END(), }; if (opts->action == REPLAY_PICK) { @@ -129,6 +130,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_BOOLEAN(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")), OPT_BOOLEAN(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")), OPT_BOOLEAN(0, "skip-empty", &opts->skip_empty, N_("skip empty commits")), + OPT_BOOLEAN(0, "copy-notes", &opts->copy_notes, N_("copy notes")), OPT_END(), }; if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra)) diff --git a/sequencer.c b/sequencer.c index 3aa480e..cc9c2bc 100644 --- a/sequencer.c +++ b/sequencer.c @@ -127,6 +127,27 @@ static void add_rewritten(unsigned char *from, unsigned char *to) rewritten.nr++; } +static void copy_notes(void) +{ + unsigned char note_commit[20]; + struct strbuf buf = STRBUF_INIT; + struct notes_tree *t = &default_notes_tree; + int i; + + init_notes(t, NULL, NULL, 0); + + for (i = 0; i < rewritten.nr; i++) { + struct rewritten_list_item *item = &rewritten.items[i]; + copy_note(t, item->from, item->to, 0, NULL); + } + + strbuf_addstr(&buf, "Notes added by 'git cherry-pick'\n"); + create_notes_commit(&default_notes_tree, NULL, &buf, note_commit); + strbuf_insert(&buf, 0, "notes: ", 7); + update_ref(buf.buf, t->ref, note_commit, NULL, 0, MSG_ON_ERR); + strbuf_release(&buf); +} + static void run_rewrite_hook(void) { struct strbuf buf = STRBUF_INIT; @@ -908,7 +929,10 @@ static int populate_opts_cb(const char *key, const char *value, void *data) else if (!strcmp(key, "options.strategy-option")) { ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc); opts->xopts[opts->xopts_nr++] = xstrdup(value); - } else + } + else if (!strcmp(key, "options.copy-notes")) + opts->copy_notes = git_config_bool_or_int(key, value, &error_flag); + else return error(_("Invalid key: %s"), key); if (!error_flag) @@ -1108,6 +1132,8 @@ static void save_opts(struct replay_opts *opts) "options.strategy-option", opts->xopts[i], "^$", 0); } + if (opts->copy_notes) + git_config_set_in_file(opts_file, "options.copy-notes", "true"); } static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) @@ -1132,6 +1158,9 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) run_rewrite_hook(); + if (opts->copy_notes) + copy_notes(); + /* * Sequence of picks finished successfully; cleanup by * removing the .git/sequencer directory @@ -1186,6 +1215,8 @@ static int single_pick(struct commit *cmit, struct replay_opts *opts) if (ret) return ret; run_rewrite_hook(); + if (opts->copy_notes) + copy_notes(); return 0; } diff --git a/sequencer.h b/sequencer.h index 84b9957..6cc072c 100644 --- a/sequencer.h +++ b/sequencer.h @@ -36,6 +36,7 @@ struct replay_opts { int allow_empty_message; int keep_redundant_commits; int skip_empty; + int copy_notes; int mainline; diff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh index f038f34..79c1219 100755 --- a/t/t3500-cherry.sh +++ b/t/t3500-cherry.sh @@ -55,4 +55,36 @@ test_expect_success \ expr "$(echo $(git cherry master my-topic-branch) )" : "+ [^ ]* - .*" ' +test_expect_success \ +'copy notes' \ +'git checkout master && +echo notes > C && +test_tick && +git commit -a -m "Update C" && +git notes add -m "a note" && +git checkout -b note-test HEAD^ && +git cherry-pick --copy-notes -x master && +test "a note" = "$(git notes show HEAD)" +' + +test_expect_success \ +'copy multiple notes' \ +'git checkout master && +echo "multiple notes" > C && +git commit -a -m "Update C again" && +git notes add -m "another note" && +git commit -
[PATCH v2 4/8] cherry-pick: store rewritten commits
Will be useful for the next commits. Signed-off-by: Felipe Contreras --- sequencer.c | 95 - sequencer.h | 1 + 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index d3c7d72..c217716 100644 --- a/sequencer.c +++ b/sequencer.c @@ -20,6 +20,18 @@ const char sign_off_header[] = "Signed-off-by: "; static const char cherry_picked_prefix[] = "(cherry picked from commit "; +struct rewritten_list_item { + unsigned char from[20]; + unsigned char to[20]; +}; + +struct rewritten_list { + struct rewritten_list_item *items; + unsigned int nr, alloc; +}; + +static struct rewritten_list rewritten; + static int is_rfc2822_line(const char *buf, int len) { int i; @@ -102,6 +114,19 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, return 1; } +static void add_rewritten(unsigned char *from, unsigned char *to) +{ + struct rewritten_list_item *item; + if (rewritten.nr + 1 >= rewritten.alloc) { + rewritten.alloc += 32; + rewritten.items = xrealloc(rewritten.items, rewritten.alloc * sizeof(*rewritten.items)); + } + item = &rewritten.items[rewritten.nr]; + hashcpy(item->from, from); + hashcpy(item->to, to); + rewritten.nr++; +} + static void remove_sequencer_state(void) { struct strbuf seq_dir = STRBUF_INIT; @@ -641,6 +666,14 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) if (!opts->no_commit) res = run_git_commit(defmsg, opts, allow); + if (!res) { + unsigned char to[20]; + + if (read_ref("HEAD", to)) + goto leave; + + add_rewritten(commit->object.sha1, to); + } leave: free_message(&msg); free(defmsg); @@ -786,6 +819,40 @@ static void read_populate_todo(struct commit_list **todo_list, die(_("Unusable instruction sheet: %s"), todo_file); } +static void read_populate_rewritten(void) +{ + const char *rewritten_file = git_path(SEQ_REWR_FILE); + struct strbuf buf = STRBUF_INIT; + char *p; + int fd; + + fd = open(rewritten_file, O_RDONLY); + if (fd < 0) + return; + if (strbuf_read(&buf, fd, 0) < 0) { + close(fd); + strbuf_release(&buf); + return; + } + close(fd); + + for (p = buf.buf; *p;) { + unsigned char from[20]; + unsigned char to[20]; + char *eol = strchrnul(p, '\n'); + if (eol - p != 81) + /* wrong size */ + break; + if (get_sha1_hex(p, from)) + break; + if (get_sha1_hex(p + 41, to)) + break; + add_rewritten(from, to); + p = *eol ? eol + 1 : eol; + } + strbuf_release(&buf); +} + static int populate_opts_cb(const char *key, const char *value, void *data) { struct replay_opts *opts = data; @@ -958,6 +1025,29 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts) strbuf_release(&buf); } +static void save_rewritten(void) +{ + const char *todo_file = git_path(SEQ_REWR_FILE); + static struct lock_file rewritten_lock; + struct strbuf buf = STRBUF_INIT; + int fd, i; + + fd = hold_lock_file_for_update(&rewritten_lock, todo_file, LOCK_DIE_ON_ERROR); + for (i = 0; i < rewritten.nr; i++) { + struct rewritten_list_item *item = &rewritten.items[i]; + strbuf_addf(&buf, "%s %s\n", sha1_to_hex(item->from), sha1_to_hex(item->to)); + } + if (write_in_full(fd, buf.buf, buf.len) < 0) { + strbuf_release(&buf); + die_errno(_("Could not write to %s"), todo_file); + } + if (commit_lock_file(&rewritten_lock) < 0) { + strbuf_release(&buf); + die(_("Error wrapping up %s."), todo_file); + } + strbuf_release(&buf); +} + static void save_opts(struct replay_opts *opts) { const char *opts_file = git_path(SEQ_OPTS_FILE); @@ -1003,8 +1093,10 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) for (cur = todo_list; cur; cur = cur->next) { save_todo(cur, opts); res = do_pick_commit(cur->item, opts); - if (res) + if (res) { + save_rewritten(); return res; + } } /* @@ -1033,6 +1125,7 @@ static int sequencer_continue(struct replay_opts *opts) return continue_single_pick(); read_populate_opts(&opts); read_populate_todo(&todo_list, opts); + read_populate_rewritten(); /* Verify that the con
[PATCH v2 3/8] cherry-pick: add --skip-empty option
Pretty much what it says on the tin. Signed-off-by: Felipe Contreras --- Documentation/git-cherry-pick.txt | 3 +++ builtin/revert.c| 2 ++ sequencer.c | 6 ++ sequencer.h | 1 + t/t3508-cherry-pick-many-commits.sh | 13 + 5 files changed, 25 insertions(+) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index c205d23..fccd936 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -129,6 +129,9 @@ effect to your index in a row. redundant commits are ignored. This option overrides that behavior and creates an empty commit object. Implies `--allow-empty`. +--skip-empty:: + Instead of failing, skip commits that are or become empty. + --strategy=:: Use the given merge strategy. Should only be used once. See the MERGE STRATEGIES section in linkgit:git-merge[1] diff --git a/builtin/revert.c b/builtin/revert.c index 0401fdb..0e5ce71 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -118,6 +118,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_END(), OPT_END(), OPT_END(), + OPT_END(), }; if (opts->action == REPLAY_PICK) { @@ -127,6 +128,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")), OPT_BOOLEAN(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")), OPT_BOOLEAN(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")), + OPT_BOOLEAN(0, "skip-empty", &opts->skip_empty, N_("skip empty commits")), OPT_END(), }; if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra)) diff --git a/sequencer.c b/sequencer.c index f7be7d8..d3c7d72 100644 --- a/sequencer.c +++ b/sequencer.c @@ -627,6 +627,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) goto leave; } + if (opts->skip_empty && is_index_unchanged() == 1) { + warning(_("skipping %s... %s"), + find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV), + msg.subject); + goto leave; + } allow = allow_empty(opts, commit); if (allow < 0) { res = allow; diff --git a/sequencer.h b/sequencer.h index 1fc22dc..3b04844 100644 --- a/sequencer.h +++ b/sequencer.h @@ -34,6 +34,7 @@ struct replay_opts { int allow_empty; int allow_empty_message; int keep_redundant_commits; + int skip_empty; int mainline; diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index 19c99d7..3dc19c6 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -187,4 +187,17 @@ test_expect_success 'cherry-pick --stdin works' ' check_head_differs_from fourth ' +test_expect_success 'cherry-pick skip empty' ' + git clean -fxd && + git checkout -b empty fourth && + git commit --allow-empty -m empty && + test_commit ontop && + git checkout -f master && + git reset --hard fourth && + git cherry-pick --skip-empty fourth..empty && + echo ontop > expected && + git log --format=%s fourth..HEAD > actual + test_cmp expected actual +' + test_done -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/8] sequencer: trivial fix
We should free objects before leaving. Signed-off-by: Felipe Contreras --- sequencer.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index b4989ba..f7be7d8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -628,8 +628,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } allow = allow_empty(opts, commit); - if (allow < 0) - return allow; + if (allow < 0) { + res = allow; + goto leave; + } if (!opts->no_commit) res = run_git_commit(defmsg, opts, allow); -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/8] cherry-pick: improvements
Hi, Here's a bunch of changes to make cherry-pick (and revert) more useful. In particular; this makes it more friendly for 'git rebase. Felipe Contreras (8): sequencer: remove useless indentation sequencer: trivial fix cherry-pick: add --skip-empty option cherry-pick: store rewritten commits sequencer: run post-rewrite hook cherry-pick: add support to copy notes revert/cherry-pick: add --quiet option revert/cherry-pick: add --skip option Documentation/git-cherry-pick.txt | 10 +- Documentation/git-revert.txt| 7 +- Documentation/sequencer.txt | 3 + builtin/revert.c| 11 ++ sequencer.c | 230 ++-- sequencer.h | 7 +- t/t3500-cherry.sh | 32 + t/t3508-cherry-pick-many-commits.sh | 13 ++ t/t3510-cherry-pick-sequence.sh | 12 ++ 9 files changed, 310 insertions(+), 15 deletions(-) -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/8] sequencer: remove useless indentation
By using good ol' goto. Signed-off-by: Felipe Contreras --- sequencer.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sequencer.c b/sequencer.c index ab6f8a7..b4989ba 100644 --- a/sequencer.c +++ b/sequencer.c @@ -474,7 +474,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) struct commit_message msg = { NULL, NULL, NULL, NULL, NULL }; char *defmsg = NULL; struct strbuf msgbuf = STRBUF_INIT; - int res, unborn = 0; + int res, unborn = 0, allow; if (opts->no_commit) { /* @@ -624,14 +624,16 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) msg.subject); print_advice(res == 1, opts); rerere(opts->allow_rerere_auto); - } else { - int allow = allow_empty(opts, commit); - if (allow < 0) - return allow; - if (!opts->no_commit) - res = run_git_commit(defmsg, opts, allow); + goto leave; } + allow = allow_empty(opts, commit); + if (allow < 0) + return allow; + if (!opts->no_commit) + res = run_git_commit(defmsg, opts, allow); + +leave: free_message(&msg); free(defmsg); -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?
On Wed, May 29, 2013 at 12:54 AM, Misty De Meo wrote: > Hi, > > Gitignore parsing no longer seems to work properly in git 1.8.3. > > One of my repositories has the following gitignore: > > /* > !/.gitignore > !/Library/ > !/CONTRIBUTING.md > !/README.md > !/SUPPORTERS.md > !/bin > /bin/* > !/bin/brew > !/share/man/man1/brew.1 > .DS_Store > /Library/LinkedKegs > /Library/PinnedKegs > /Library/Taps > /Library/Formula/.gitignore > > In 1.8.2.3 and earlier, this works as expected. However, in 1.8.3 I'm > seeing every file in /bin/ being marked as an untracked file. The changes in this area since 1.8.2.3 seem to be Karsten's (I'm not blaming, just wanted to narrow down the problem). The patterns of interest seem to be !/bin /bin/* !/bin/brew Without "!/bin" v1.8.3 seems to behave the same as v1.8.2.3. Can you verify it? > I asked about this in #git, and was told that the culprit was the > regex support; apparently recompiling without regex support fixes the > specific gitignore issue. However, this doesn't seem to have been > reported anywhere on the mailing list that I can see. I was also told > that the issue is OS X-specific, and doesn't happen on other > platforms. Puzzled. regex has nothing to do with gitignore (glob does). How do you recompile without regex support? Setting NO_REGEX? I'm on Linux btw, no chance of touching OS X. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] completion: zsh: improve bash script loading
It's better to check in multiple locations, so the user doesn't have to. And update the documentation. Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.zsh | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh index 2565d2e..9555cf8 100644 --- a/contrib/completion/git-completion.zsh +++ b/contrib/completion/git-completion.zsh @@ -4,18 +4,17 @@ # # Copyright (c) 2012-2013 Felipe Contreras # -# You need git's bash completion script installed somewhere, by default on the -# same directory as this script. +# You need git's bash completion script installed somewhere, by default it +# would be the location bash-completion uses. # -# If your script is on ~/.git-completion.sh instead, you can configure it on -# your ~/.zshrc: +# If your script is somewhere else, you can configure it on your ~/.zshrc: # # zstyle ':completion:*:*:git:*' script ~/.git-completion.sh # -# The recommended way to install this script is to copy to -# '~/.zsh/completion/_git', and then add the following to your ~/.zshrc file: +# The recommended way to install this script is to copy to '~/.zsh/_git', and +# then add the following to your ~/.zshrc file: # -# fpath=(~/.zsh/completion $fpath) +# fpath=(~/.zsh $fpath) complete () { @@ -27,7 +26,18 @@ zstyle -T ':completion:*:*:git:*' tag-order && \ zstyle ':completion:*:*:git:*' tag-order 'common-commands' zstyle -s ":completion:*:*:git:*" script script -test -z "$script" && script="$(dirname ${funcsourcetrace[1]%:*})"/git-completion.bash +if [ -z "$script" ]; then + local -a locations + locations=( + '/etc/bash_completion.d/git' # fedora, old debian + '/usr/share/bash-completion/completions/git' # arch, ubuntu, new debian + '/usr/share/bash-completion/git' # gentoo + $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash + ) + for e in $locations; do + test -f $e && script="$e" && break + done +fi ZSH_VERSION='' . "$script" __gitcomp () -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Towards a useable git-branch
On Tue, May 28, 2013 at 9:01 PM, Ramkumar Ramachandra wrote: > Also, a couple of minor annoyances: > > 1. When f-e-r is invoked with refs/tags, we get stray output. Atleast > it doesn't segfault, thanks to your ignore-commit patch. Maybe > printing stray output is the right thing to do, as opposed to erroring > out. What format did you use? > 2. %>(*) only works with f-e-r atoms, not with pretty-format atoms. > This is ofcourse obvious from the implementation, but isn't it a > little consistent? It is not. I think it's documented as well as a known implementation limitation. It's not hard to be fixed (we could call format_commit_message again for all entries, this time with a single placeholder, to collect the best width). If anybody does need %>(*)%H to work, we could do it. BTW, the way I implement get_atom_width() is not really optimal. For n lines, we call print_value() in get_atom_width n^2 times. We could cache the calculated width instead and reduce that to n times. > Should we start off a new pretty-ref-formats document, where we > explicitly exclude things like %ae (because of the hex overriding > thing)? I don't think it's a problem if documented properly. And this one is doucmented as well, I think. I'd really like to introduce a new --pretty option (or something) that does not accept %xx as a hex notion, so %ae and friends won't be hidden. It's also a good thing for compatibility because currently %H in --format produces %H. After this, %H produces something else. It's unlikely that anybody has done that. But it's even better if we avoid that possibility entirely with a new option. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] completion: avoid ls-remote in certain scenarios
It's _very_ slow in many cases, and there's really no point in fetching *everything* from the remote just for completion. In many cases it might be faster for the user to type the whole thing. If the user manually specifies 'refs/*', then the full ls-remote completion is triggered. Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 1c35eef..2ce4f7d 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -427,14 +427,8 @@ __git_refs () done ;; *) - git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \ - while read -r hash i; do - case "$i" in - *^{}) ;; - refs/*) echo "${i#refs/*/}" ;; - *) echo "$i" ;; - esac - done + echo "HEAD" + git for-each-ref --format="%(refname:short)" -- "refs/remotes/$dir/" | sed -e "s#^$dir/##" ;; esac } -- 1.8.3.rc3.312.g47657de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] rebase: fix 'cherry' mode storage
Junio C Hamano wrote: > Felipe Contreras writes: > > > We don't use the 'rebase-apply'. > > s/.$/; we will use rebase-merge instead./ I think. We could use 'rebase-apply' or any directory, but currently we don't use any, and 'rebase-apply' is for 'git am'. > > Signed-off-by: Felipe Contreras > > --- > > git-rebase--cherry.sh | 4 > > git-rebase.sh | 5 - > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh > > index cbf80f9..ab1f8b7 100644 > > --- a/git-rebase--cherry.sh > > +++ b/git-rebase--cherry.sh > > @@ -18,6 +18,9 @@ esac > > > > test -n "$rebase_root" && root_flag=--root > > > > +mkdir "$state_dir" || die "Could not create temporary $state_dir" > > +: > "$state_dir"/cherry || die "Could not mark as cherry" > > Style: > > >"$state_dir/cherry-pick" Fix that in git-rebase--interactive.sh then, where this code came from. > I am not sure if the user _cares_ that internally we use cherry-pick > when s/he asks us to do a keep-empty, and I suspect "mark as cherry" > incomprehensible. I do not even know what is going on at this point > (yet). It's the same we do in git-rebase--interactive.sh. > I _suspect_ that you are using cherry-pick when --keep-empty is given, > so it might even make sense to talk in the end-users' terms, > i.e. call this "$state_dir/keep-empty" (I am not sure if calling > this split half "git-rebase--keep-empty.sh" makes sense, yet). If git-rebase--cherry(-pick)?.sh does the same as 'git-rebase--am.sh', we can replace it, and make yet another step of replacing scripts (git am) with C code (git cherry-pick). I thought I explained that somewhow, maybe I didn't. It still makes sense to fix the broken --keep-empty behavior. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Towards a useable git-branch
On Tue, May 28, 2013 at 9:28 PM, Ramkumar Ramachandra wrote: > Ramkumar Ramachandra wrote: >> %>(N) doesn't work properly with f-e-r, and I'm not sure why. I'm not >> talking about your last patch where you compute * -- that works fine; >> it's just that %>(N) doesn't when N is a concrete number. > > Try this: > > %(refname:short)%>(30)%(upstream:short) > > (assuming that you have lots of branches). I'm noticing random > alignment problems. It's because you don't pad enough spaces after %(refname:short) so the starting point of %(upstream:short) on each line is already unaligned, I think. Try this: %<(*)%(refname:short)%>(30)%(upstream:short) or if you prefer at specific column (e.g. align upstream close to the 60th column, regardless of refname's length): %(refname:short)%>|(60)%(upstream:short) -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] rebase: improve the keep-empty
Martin von Zweigbergk wrote: > I think I have some patches at home that instead teach 'git am' the > --keep-empty flag. Does that make sense? It's been a while since I > looked at it, but I'll try to take a look tonight (PST). I think it does make sense. But I still would prefer 'git rebase' to rely on 'git cherry-pick' primariliy. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: What's cooking in git.git (May 2013, #08; Tue, 28)
Junio C Hamano wrote: > * fc/makefile (2013-05-26) 5 commits > - build: do not install git-remote-testpy > - build: add NO_INSTALL variable > - build: cleanup using $< > - build: cleanup using $^ > - build: trivial simplification > (this branch is used by fc/remote-helpers-use-specified-python.) No, these are independent. > * fc/remote-helpers-use-specified-python (2013-05-28) 4 commits > - remote-helpers: add exec-path links > - remote-helpers: allow direct test execution > - remote-helpers: rename tests > - remote-helpers: generate scripts > (this branch uses fc/makefile.) > > I do not particularly think the second from the bottom is a good > change, but it takes the remainder of the series hostage. > Will hopefully be rerolled without it. > * fc/remote-bzr (2013-05-28) 8 commits > - remote-bzr: add fallback check for a partial clone > - remote-bzr: reorganize the way 'wanted' works > - remote-bzr: trivial cleanups > - remote-bzr: change global repo > - remote-bzr: delay cloning/pulling > - remote-bzr: simplify get_remote_branch() > - remote-bzr: fix for files with spaces > - remote-bzr: recover from failed clones > > The ones near the tip conflicted with the hotfix for 1.8.3 so I > discarded them for now. > > Will merge to 'next'? Didn't I resend these with the conflict fixed? -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: contrib/git-normal-to-bare.sh
Zenaan Harkness wrote: > This question comes up every now and then - how to convert from normal > to bare, or vice versa. > > This is just a start to a basic script to go one way. Needs more tests > etc, but it's enough to get newbies (like me) off to a reasonable > start. > > PLEASE CC me, as I am not subscribed. You do not need to ask to be CC'ed. Any mailing list properly configued would not munge the Reply-To header, so anybody replying to this mail would reply to you. Git is a properly configured mailing list, just like all the mailing list in vger.kernel.org. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] remote-helpers: rename tests
Junio C Hamano wrote: > Felipe Contreras writes: > > > The .t extension is more standard for sharness tests. > > > > Signed-off-by: Felipe Contreras > > --- > > Is that "sharness" test the sh script testsuite forked from our > testsuite? > > I do not see how it makes sense to copy how they deviate from us > back to our codebase, especially if we plan to eventually move some > of these tests out of contrib/ area, but even without such a plan in > the future. They deviate from us, we deviate from them, whatever. We are a single project, what more than one project does is more standard. % man prove --ext Set the extension for tests (default '.t') -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Thomas Rast wrote: > Junio C Hamano writes: > > > Thomas Rast Cc'ed as he has been the primary force behind this line > > of "notes" usability. > > Thanks for pointing this out to me. > > > Felipe Contreras writes: > > > >> Signed-off-by: Felipe Contreras > >> --- > >> builtin/revert.c | 2 + > >> sequencer.c | 136 > >> -- > >> sequencer.h | 2 + > >> t/t3500-cherry.sh | 32 + > >> 4 files changed, 169 insertions(+), 3 deletions(-) > > > > "git cherry-pick" should help maintaining notes just like amend and > > rebase, but how should this interact with notes.rewrite., > > where the command is capable of doing this without an explicit > > option once you tell which notes need to be maintained? > > Since we already have the notes.rewrite. convention, it would > seem the obvious choice to line it up with the others. The main > bikeshedding opportunity is whether this should be an exception and > default to false (all other commands default it to true). > > Also: how does this interact with notes.rewriteRef and the corresponding > env vars? Why? > > How does it interact with 'cherry-pick -n' if this is done in sequence, > effectively squashing several commits (this use-case is actually > suggested by the manpage), if multiple source commits had notes? Should > it respect notes.rewriteMode (and by default concatenate)? (I don't > know if the sequencer state is expressive enough already to carry this > in a meaningful way across cherry-pick commands.) Feel free to implement that. I'm just interested in 'git cherry-pick' being usable for 'git rebase' purposes. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Junio C Hamano wrote: > Felipe Contreras writes: > > > Signed-off-by: Felipe Contreras > > --- > > builtin/revert.c | 2 + > > sequencer.c | 136 > > -- > > sequencer.h | 2 + > > t/t3500-cherry.sh | 32 + > > 4 files changed, 169 insertions(+), 3 deletions(-) > > "git cherry-pick" should help maintaining notes just like amend and > rebase, but how should this interact with notes.rewrite., > where the command is capable of doing this without an explicit > option once you tell which notes need to be maintained? > > Thomas Rast Cc'ed as he has been the primary force behind this line > of "notes" usability. > > It probably is not sensible to carry over note from a reverted > commit to its revert, but I didn't immediately spot how the patch > does this only for cherry-pick but not for revert (the codepath in > do_pick_commit() is shared between them, no?). The option is enabled from the UI, and only 'git cherry-pick' has the --copy-notes option. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] test: improve rebase -q test
Junio C Hamano wrote: > Felipe Contreras writes: > > > Let's show the output so it's clear why it failed. > > > > Signed-off-by: Felipe Contreras > > --- > > t/t3400-rebase.sh | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh > > index b58fa1a..fb39531 100755 > > --- a/t/t3400-rebase.sh > > +++ b/t/t3400-rebase.sh > > @@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when > > upstream arg is missing' ' > > test_expect_success 'rebase -q is quiet' ' > > git checkout -b quiet topic && > > git rebase -q master >output.out 2>&1 && > > + cat output.out && > > test ! -s output.out > > ' > > It is one thing to avoid squelching output that naturally comes out > of command being tested unnecessarily, so that "./t-*.sh -v" > output can be used for debugging. I however am not sure if adding > "cat" to random places like this is a productive direction for us to > go in. > > A more preferrable alternative may be adding something like this to > test-lib.sh and call it from here and elsewhere (there are about 50 > places that do "test ! -s "), perhaps? > > test_must_be_an_empty_file () { > if test -s "$1" > then > cat "$1" > false > fi > } Perhaps, but I'm not interested. I'm tired of obvious fixes getting rejected for hypothetical "ideal" situations that we'll never reach. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
Junio C Hamano wrote: > Neil Horman writes: > > > On Mon, May 27, 2013 at 11:52:18AM -0500, Felipe Contreras wrote: > >> We should free objects before leaving. > >> > >> Signed-off-by: Felipe Contreras > >> --- > >> sequencer.c | 7 +-- > >> 1 file changed, 5 insertions(+), 2 deletions(-) > >> > >> diff --git a/sequencer.c b/sequencer.c > >> index ab6f8a7..7eeae2f 100644 > >> --- a/sequencer.c > >> +++ b/sequencer.c > >> @@ -626,12 +626,15 @@ static int do_pick_commit(struct commit *commit, > >> struct replay_opts *opts) > >>rerere(opts->allow_rerere_auto); > >>} else { > >>int allow = allow_empty(opts, commit); > >> - if (allow < 0) > >> - return allow; > >> + if (allow < 0) { > >> + res = allow; > >> + goto leave; > >> + } > >>if (!opts->no_commit) > >>res = run_git_commit(defmsg, opts, allow); > >>} > >> > >> +leave: > >>free_message(&msg); > >>free(defmsg); > >> > >> -- > >> 1.8.3.rc3.312.g47657de > >> > >> > > Acked-by: Neil Horman > > This is better done without "goto" in general. > > The other patch 2/2/ adds one more "we need to exit from the middle > of the flow" and makes it look handier to add an exit label here, > but it would be even better to express the logic of that patch as a > normal cascade of if/else if/..., which is small enough and we do > not need the "leave:" label. Linux kernel developers would disagree. In C 'goto' is quite of then the only sane option, and you can see 'goto' used in the Linux kernel all over the place for that reason. In this particular case it also makes perfect sense. > It probably is better to fold this patch into the other one when it > is rerolled to correct the option name gotcha "on the tin". Why? This patch is standalone and fixes an issue that is independent of the other patch. Why squash two patches that do *two* different things? Anyway, I'll happily drop this patch if you want this memory leak to remain. But then I'll do the same in the other patch. This mantra of avodiing 'goto' is not helping anybody. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] remote-bzr: reorganize the way 'wanted' works
Junio C Hamano wrote: > Felipe Contreras writes: > > > +wanted = get_config('remote-bzr.branches').rstrip().split(', ') > > Two minor nits and one design suggestion: > > - Why rstrip() not strip()? The purpose of the strip is to remove the _single_ "\n" at the end that subprocess communicate. Maybe get_config() should do that. > It appears that this only is helping >an end-user "mistake" like this: > > git config remote-bzr.branches 'trunk, devel, test ' > >without helping people who have done this: > > git config remote-bzr.branches 'trunk, devel, test' No, that's tnot it. > - Is > > git config remote-bzr.branches trunk,devel,test > >a grave sin? > >In other words, wouldn't we want something like this instead? > > map(lambda s: s.strip(), get_config('...').split(',')) Yeah, that might make sense. > - Doesn't allowing multi-valued variable, e.g. > > [remote-bzr] > branches = trunk > branches = devel > branches = test > >make it easier for the user to manage this configuration by >e.g. selectively removing or adding tracked branches? How would the 'git config' command look like? Either way, that's orthogonal to this patch. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix segfault with git log -c --follow
Clemens Buchacher writes: >> I wonder, just like we force recursive and disable external on the >> copy before we use it to call diff_tree_sha1(), if we should disable >> follow-renames on it. "--follow" is an option that is given to the >> history traversal part and it should not play any role in getting the >> pairwise diff with all parents diff_tree_combined() does. > > Can't parse that last sentence. > > In any case, I don't think disabling diff_tree_sha1 is a solution. The > bug is in diff_tree_sha1 and its subfunctions, because they manipulate a > data structures such that it becomes corrupt. And they do so in an > obfuscated and clearly unintentional manner. So we should not blame the > user for calling diff_tree_sha1 in such a way that it causes corruption. > >> Besides, >> >> - "--follow" hack lets us keep track of only one path; and > > Ok. Good to know it is considered a hack. The code is quite strange > indeed. The problem with --follow is that it only tracks one path globally. In a history like this, suppose that a path X long time ago was renamed to Y at commit B: ---o---A---B---C---o HEAD and you start digging with "log --follow -c HEAD -- Y". When looking at C, because it and its parent B both have path Y, the try-to-follow hack does not kick in, and when trying to show C, we will show the change in Y (because that is the pathspec). Then we look at B. Because B has path we are following, i.e. Y, and its parent A does not, try-to-follow hack kicks in, and it mangles the pathspec that is used globally for history traversal to X while showing the difference between A's X and B's Y. Then we dig further to find A; at this point the global pathspec is swapped and now it is X. That makes --follow a working hack for a simplest single strand of pearls. But if you have a mergy history, e.g. ---o---A---B---C---o HEAD \ / D---E---F---G---H it can break in interesting ways. We are likely to have looked at H before looking at B and used pathspec Y while inspecting H, but after looking at B, the global pathspec is swapped to X, and then we try to look at G, F, E and D, none of which may have renamed the original X, so you would likely miss the change to the path Y you wanted to follow. To fix this, we would need to keep "what path are we following" not in the global revs->pathspec, but per the traversal paths that are currently active (e.g. when we look at C and H, it is Y, when we look at B, it is X, when we look at G, that is inherited from H and still Y, not affected by the rename at B. And then when we look at A (we need topo-order traversal to do this), it needs to notice that one child (i.e. B) has been following X while the other (i.e. D) Y, and merge the "I've been following this path" information in a sensible way (e.g. look at its own tree and see what is available, in this case X). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix segfault with git log -c --follow
On Tue, May 28, 2013 at 10:22:17AM -0700, Junio C Hamano wrote: > Clemens Buchacher writes: > > > In diff_tree_combined we make a copy of diffopts. In > > try_to_follow_renames, called via diff_tree_sha1, we free and > > re-initialize diffopts->pathspec->items. Since we did not make a deep > > copy of diffopts in diff_tree_combined, the original diffopts does not > > get the update. By the time we return from diff_tree_combined, > > rev->diffopt->pathspec->items points to an invalid memory address. We > > get a segfault next time we try to access that pathspec. > > I am not quite sure if I follow. Do you mean > > diff_tree_combined() > - makes a shallow copy of rev->diffopt > - calls diff_tree_sha1() > diff_tree_sha1() > - tries to follow rename and clobbers diffopt Right. > - tries to use the shallow copy of original rev->diffopt > that no longer is valid, which is a problem diff_tree_combined does not try to use it right away. It does return, but rev->diffopt is now invalid and the next time we do any kind of diff with it, we have a problem. > I wonder, just like we force recursive and disable external on the > copy before we use it to call diff_tree_sha1(), if we should disable > follow-renames on it. "--follow" is an option that is given to the > history traversal part and it should not play any role in getting the > pairwise diff with all parents diff_tree_combined() does. Can't parse that last sentence. In any case, I don't think disabling diff_tree_sha1 is a solution. The bug is in diff_tree_sha1 and its subfunctions, because they manipulate a data structures such that it becomes corrupt. And they do so in an obfuscated and clearly unintentional manner. So we should not blame the user for calling diff_tree_sha1 in such a way that it causes corruption. > Besides, > > - "--follow" hack lets us keep track of only one path; and Ok. Good to know it is considered a hack. The code is quite strange indeed. > - "-c" and "--cc" make sense only when dealing with a merge commit >and the path in the child may have come from different path in >parents, Sorry, I don't get it. > so I am not sure if allowing combination of "--follow -c/--cc" makes > much sense in the first place. My use-case is came up with this history: 1. Code gets added to file A. 2. File A gets renamed to B in a different branch. 3. The branches get merged, and code from (1) is removed in the merge. Later I wonder why code from (1) is gone from B even though I felt certain it had been added before. I also remember that B was renamed at some point. So I do git log -p --follow B, and it nicely shows that diff where the code was added, but no diff where the code is removed. The reason is of course, that the code was removed in the merge and that diff is not shown. And -c is usually what I do to enable showing diffs in merge commits. And if the pairwise diff can also deal with file renames, I think it absolutely does make sense to show also a three-way diff. I can't tell far away the code is from supporting anything like that. Cheers, Clemens -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] rebase: fix cherry-pick invocations
Junio C Hamano writes: > Felipe Contreras writes: > >> So that all the tests pass. >> >> Signed-off-by: Felipe Contreras >> --- >> git-rebase--cherry.sh | 17 - >> 1 file changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh >> index ca78b1b..c3a2ac9 100644 >> --- a/git-rebase--cherry.sh >> +++ b/git-rebase--cherry.sh >> @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root >> mkdir "$state_dir" || die "Could not create temporary $state_dir" >> : > "$state_dir"/cherry || die "Could not mark as cherry" >> >> +if test -n "$rebase_root" >> +then >> +revisions="$onto...$orig_head" >> +else >> +revisions="$upstream...$orig_head" >> +fi > > "So that all the tests pass" needs a bit more explanation to say for > cherry-pick codepath why and how two-dot range fails and why and how > three-dot variant with --right-only fixes it. What are the problematic > cases? Yikes, sorry, this was me being slow. Walking A...B range with right-only and --cherry applied will filter the duplicates, which is wat you want, I think, and walking A..B range will not do the filtering for you. >> # we have to do this the hard way. git format-patch completely squashes >> # empty commits and even if it didn't the format doesn't really lend >> # itself well to recording empty patches. fortunately, cherry-pick >> # makes this easy >> -git cherry-pick --allow-empty "$revisions" >> +if test -n "$keep_empty" >> +then >> +extra="--allow-empty" >> +else >> +extra="--skip-empty --cherry-pick" >> +fi >> +test -n "$GIT_QUIET" && extra="$extra -q" >> +test -z "$force_rebase" && extra="$extra --ff" >> +git cherry-pick --no-merges --right-only --topo-order --do-walk >> --copy-notes $extra "$revisions" >> ret=$? >> >> if test 0 != $ret -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] rebase: fix cherry-pick invocations
Felipe Contreras writes: > So that all the tests pass. > > Signed-off-by: Felipe Contreras > --- > git-rebase--cherry.sh | 17 - > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh > index ca78b1b..c3a2ac9 100644 > --- a/git-rebase--cherry.sh > +++ b/git-rebase--cherry.sh > @@ -23,11 +23,26 @@ test -n "$rebase_root" && root_flag=--root > mkdir "$state_dir" || die "Could not create temporary $state_dir" > : > "$state_dir"/cherry || die "Could not mark as cherry" > > +if test -n "$rebase_root" > +then > + revisions="$onto...$orig_head" > +else > + revisions="$upstream...$orig_head" > +fi "So that all the tests pass" needs a bit more explanation to say for cherry-pick codepath why and how two-dot range fails and why and how three-dot variant with --right-only fixes it. What are the problematic cases? > # we have to do this the hard way. git format-patch completely squashes > # empty commits and even if it didn't the format doesn't really lend > # itself well to recording empty patches. fortunately, cherry-pick > # makes this easy > -git cherry-pick --allow-empty "$revisions" > +if test -n "$keep_empty" > +then > + extra="--allow-empty" > +else > + extra="--skip-empty --cherry-pick" > +fi > +test -n "$GIT_QUIET" && extra="$extra -q" > +test -z "$force_rebase" && extra="$extra --ff" > +git cherry-pick --no-merges --right-only --topo-order --do-walk --copy-notes > $extra "$revisions" > ret=$? > > if test 0 != $ret -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] rebase: fix sequence continuation
Felipe Contreras writes: > We are not in am mode. That may make sense, but shouldn't this part be like so from the very beginning? In other words, this looks like an "oops, 1/5 was buggy and this is a hotfix". > > Signed-off-by: Felipe Contreras > --- > git-rebase--cherry.sh | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh > index ab1f8b7..ca78b1b 100644 > --- a/git-rebase--cherry.sh > +++ b/git-rebase--cherry.sh > @@ -5,13 +5,15 @@ > > case "$action" in > continue) > - git am --resolved --resolvemsg="$resolvemsg" && > - move_to_original_branch > + git cherry-pick --continue && > + move_to_original_branch && > + rm -rf "$state_dir" > exit > ;; > skip) > - git am --skip --resolvemsg="$resolvemsg" && > - move_to_original_branch > + git cherry-pick --skip && > + move_to_original_branch && > + rm -rf "$state_dir" > exit > ;; > esac -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] rebase: fix 'cherry' mode storage
Felipe Contreras writes: > We don't use the 'rebase-apply'. s/.$/; we will use rebase-merge instead./ I think. > Signed-off-by: Felipe Contreras > --- > git-rebase--cherry.sh | 4 > git-rebase.sh | 5 - > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh > index cbf80f9..ab1f8b7 100644 > --- a/git-rebase--cherry.sh > +++ b/git-rebase--cherry.sh > @@ -18,6 +18,9 @@ esac > > test -n "$rebase_root" && root_flag=--root > > +mkdir "$state_dir" || die "Could not create temporary $state_dir" > +: > "$state_dir"/cherry || die "Could not mark as cherry" Style: >"$state_dir/cherry-pick" I am not sure if the user _cares_ that internally we use cherry-pick when s/he asks us to do a keep-empty, and I suspect "mark as cherry" incomprehensible. I do not even know what is going on at this point (yet). I _suspect_ that you are using cherry-pick when --keep-empty is given, so it might even make sense to talk in the end-users' terms, i.e. call this "$state_dir/keep-empty" (I am not sure if calling this split half "git-rebase--keep-empty.sh" makes sense, yet). > + > # we have to do this the hard way. git format-patch completely squashes > # empty commits and even if it didn't the format doesn't really lend > # itself well to recording empty patches. fortunately, cherry-pick > @@ -32,3 +35,4 @@ then > fi > > move_to_original_branch > +rm -rf "$state_dir" > diff --git a/git-rebase.sh b/git-rebase.sh > index 2754985..b7759d5 100755 > --- a/git-rebase.sh > +++ b/git-rebase.sh > @@ -174,6 +174,9 @@ then > then > type=interactive > interactive_rebase=explicit > + elif test -f "$merge_dir"/cherry > + then > + type=cherry > else > type=merge > fi > @@ -382,7 +385,7 @@ then > elif test -n "$keep_empty" > then > type=cherry > - state_dir="$apply_dir" > + state_dir="$merge_dir" > else > type=am > state_dir="$apply_dir" -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] rebase: split the cherry-pick stuff
Felipe Contreras writes: > They do something completely different from 'git am', it belongs in a > different file. I would prefer to see it called --cherry-pick, not --cherry, as they are different commands (the latter may be useful when deciding which one to use the former). > > Signed-off-by: Felipe Contreras > --- > .gitignore| 1 + > Makefile | 1 + > git-rebase--am.sh | 65 > ++- > git-rebase--cherry.sh | 34 +++ > git-rebase.sh | 4 > 5 files changed, 68 insertions(+), 37 deletions(-) > create mode 100644 git-rebase--cherry.sh > > diff --git a/.gitignore b/.gitignore > index 6669bf0..284fc8f 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -113,6 +113,7 @@ > /git-read-tree > /git-rebase > /git-rebase--am > +/git-rebase--cherry > /git-rebase--interactive > /git-rebase--merge > /git-receive-pack > diff --git a/Makefile b/Makefile > index 0f931a2..a3cd4bc 100644 > --- a/Makefile > +++ b/Makefile > @@ -469,6 +469,7 @@ SCRIPT_SH += git-web--browse.sh > SCRIPT_LIB += git-mergetool--lib > SCRIPT_LIB += git-parse-remote > SCRIPT_LIB += git-rebase--am > +SCRIPT_LIB += git-rebase--cherry > SCRIPT_LIB += git-rebase--interactive > SCRIPT_LIB += git-rebase--merge > SCRIPT_LIB += git-sh-setup > diff --git a/git-rebase--am.sh b/git-rebase--am.sh > index f84854f..ee1b1b9 100644 > --- a/git-rebase--am.sh > +++ b/git-rebase--am.sh > @@ -19,52 +19,43 @@ esac > test -n "$rebase_root" && root_flag=--root > > ret=0 > +rm -f "$GIT_DIR/rebased-patches" > > - cat >&2 <<-EOF > +git format-patch -k --stdout --full-index --ignore-if-in-upstream \ > + --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \ > + $root_flag "$revisions" >"$GIT_DIR/rebased-patches" > +ret=$? > > +if test 0 != $ret > +then > + rm -f "$GIT_DIR/rebased-patches" > + case "$head_name" in > + refs/heads/*) > + git checkout -q "$head_name" > + ;; > + *) > + git checkout -q "$orig_head" > + ;; > + esac > > - $revisions > + cat >&2 <<-EOF > > + git encountered an error while preparing the patches to replay > + these revisions: > > + $revisions > > + As a result, git cannot rebase them. > + EOF > + exit $? > fi > > +git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" > <"$GIT_DIR/rebased-patches" > +ret=$? > + > +rm -f "$GIT_DIR/rebased-patches" > + > if test 0 != $ret > then > test -d "$state_dir" && write_basic_state OK, this part seems a straightforward reindentation of "else" side. > diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh > new file mode 100644 > index 000..cbf80f9 > --- /dev/null > +++ b/git-rebase--cherry.sh > @@ -0,0 +1,34 @@ > +#!/bin/sh > +# > +# Copyright (c) 2010 Junio C Hamano. > +# > + > +case "$action" in > +continue) > + git am --resolved --resolvemsg="$resolvemsg" && > + move_to_original_branch > + exit > + ;; > +skip) > + git am --skip --resolvemsg="$resolvemsg" && > + move_to_original_branch > + exit > + ;; > +esac > + > +test -n "$rebase_root" && root_flag=--root Hmm, do we want to duplicate this part between the two? Perhaps it will be unified in a later patch---I'll read on. > +# we have to do this the hard way. git format-patch completely squashes > +# empty commits and even if it didn't the format doesn't really lend > +# itself well to recording empty patches. fortunately, cherry-pick > +# makes this easy > +git cherry-pick --allow-empty "$revisions" > +ret=$? > + > +if test 0 != $ret > +then > + test -d "$state_dir" && write_basic_state > + exit $ret > +fi > + > +move_to_original_branch OK. This is the "then" side of the original, with clean-up phase copied. > diff --git a/git-rebase.sh b/git-rebase.sh > index 2c692c3..2754985 100755 > --- a/git-rebase.sh > +++ b/git-rebase.sh > @@ -379,6 +379,10 @@ elif test -n "$do_merge" > then > type=merge > state_dir="$merge_dir" > +elif test -n "$keep_empty" > +then > + type=cherry > + state_dir="$apply_dir" > else > type=am > state_dir="$apply_dir" -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?
Øystein Walle writes: > Misty De Meo brew.sh> writes: > >> >> Hi, >> >> Gitignore parsing no longer seems to work properly in git 1.8.3. >> >> One of my repositories has the following gitignore: >> >> /* >> !/.gitignore >> !/Library/ >> !/CONTRIBUTING.md >> !/README.md >> !/SUPPORTERS.md >> !/bin >> /bin/* >> !/bin/brew >> !/share/man/man1/brew.1 >> .DS_Store >> /Library/LinkedKegs >> /Library/PinnedKegs >> /Library/Taps >> /Library/Formula/.gitignore >> >> In 1.8.2.3 and earlier, this works as expected. However, in 1.8.3 I'm >> seeing every file in /bin/ being marked as an untracked file. >> >> I asked about this in #git, and was told that the culprit was the >> regex support; apparently recompiling without regex support fixes the >> specific gitignore issue. However, this doesn't seem to have been >> reported anywhere on the mailing list that I can see. I was also told >> that the issue is OS X-specific, and doesn't happen on other >> platforms. >> >> Thanks, >> Misty De Meo >> > > I see a similar problem using e.g. the following .gitignore to exclude > everything except C source files and header files: > > * > !*/ > !*.c > !*.h > > In Git 1.8.3 'git status' will show other files as untracked while in > Git 1.8.2.3 I don't have that problem. I bisected to find that the > offending commit is v1.8.2.1-402-g95c6f27. > > I am not on OSX, however, but on Linux (Ubuntu 12.04 and RHEL 5.8) so > this may be a separate issue. I've also gotten the impression that this > is intentional. In any case I cannot create a .gitignore that achieves > the same for both older and newer versions of Git. Thanks; 95c6f27 is from Karsten and I think Duy is the most clueful one in this area in the discussion that had this change; I am forwarding this message to them. Cf. http://thread.gmane.org/gmane.comp.version-control.git/218440/focus=221289 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (May 2013, #08; Tue, 28)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. There are topics that are still in 'next', not because they needed more testing, but only because we already were in the -rc period. Now that 1.8.3 is out, we will start merging them to 'master' (and some to 'maint' for 1.8.3.1) probably tomorrow (i.e. after waiting for a few days to make sure we can have a clean start of the post 1.8.3 maint branch with brown paper bag fixes and nothing else). The post 1.8.3 cycle will start early next month, at which time the 'next' branch will be rewound and rebuilt. Until then, I expect that my patch queue may stay slow and leaky while I take a bit of break. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * nd/urls-doc-no-file-hyperlink-fix (2013-05-24) 1 commit - urls.txt: avoid auto converting to hyperlink Will merge to 'next'. * cb/log-follow-with-combined (2013-05-28) 1 commit - fix segfault with git log -c --follow Will merge to 'next'. * fc/cleanups (2013-05-28) 3 commits - test: rebase: fix --interactive test - test: trivial cleanups - remote: trivial style cleanup Will merge to 'next'. * fc/makefile (2013-05-26) 5 commits - build: do not install git-remote-testpy - build: add NO_INSTALL variable - build: cleanup using $< - build: cleanup using $^ - build: trivial simplification (this branch is used by fc/remote-helpers-use-specified-python.) Will merge to 'next'. * fc/remote-helpers-use-specified-python (2013-05-28) 4 commits - remote-helpers: add exec-path links - remote-helpers: allow direct test execution - remote-helpers: rename tests - remote-helpers: generate scripts (this branch uses fc/makefile.) I do not particularly think the second from the bottom is a good change, but it takes the remainder of the series hostage. Will hopefully be rerolled without it. * fc/send-email-chainreplyto-warning (2013-05-28) 1 commit - send-email: remove warning about unset chainreplyto An overdue removal od "behaviour changed at 1.7.0; if you were living in a cave, here is what you can adjust to it" message. Will merge to 'next'. * ks/difftool-dirdiff-copy-all (2013-05-28) 1 commit - difftool --dir-diff: always use identical working tree file "difftool --dir-diff" populates a temporary directory with files, inviting the user to modify them in place, but in some cases we weren't expecting the user to modify them and did not take the change back to the working tree. The log message has room for clarification, but the change looked sane. * nd/prune-packed-dryrun-verbose (2013-05-28) 1 commit - prune-packed: avoid implying "1" is DRY_RUN in prune_packed_objects() Will merge to 'next'. * rj/mingw-compat-st-mode-bits (2013-05-28) 1 commit - path: Fix a sparse warning Will merge to 'next'. * rs/commit-m-no-edit (2013-05-28) 1 commit - commit: don't start editor if empty message is given with -m Will merge to 'next'. * xq/credential-osxkeychain (2013-05-28) 1 commit - credential-osxkeychain: support more protocols Will merge to 'next'. * jc/core-checkstat (2013-05-06) 1 commit - deprecate core.statinfo at Git 2.0 boundary (this branch is used by jc/core-checkstat-2.0.) Will merge to 'next'. -- [Stalled] * rr/rebase-autostash (2013-05-12) 7 commits - rebase: implement --[no-]autostash and rebase.autostash - rebase --merge: return control to caller, for housekeeping - rebase -i: return control to caller, for housekeeping - am: return control to caller, for housekeeping - rebase: prepare to do generic housekeeping - rebase -i: don't error out if $state_dir already exists - am: tighten a conditional that checks for $dotest This is from v3, but after a "Fixed" message in $gmane/224111 we haven't seen a reroll yet. Also there was an attempt for a follow-up, but it was never completed. * rj/mingw-cygwin (2013-05-08) 2 commits - cygwin: Remove the CYGWIN_V15_WIN32API build variable - mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE Update build for Cygwin 1.[57]. There seems to have been some discussion but did anything concrete come out of it??? * mh/multimail (2013-04-21) 1 commit - git-multimail: a replacement for post-receive-email Waiting for the initial history to pull from. $gmane/223564 * jc/format-patch (2013-04-22) 2 commits - format-patch: --inline-single - format-patch: rename "no_inline" field A new option to send a single patch to the standard output to be appended at the bottom of a message. I personally have no need for this, but it was easy enough to cobble together. Tests, docs and stripping out more MIMEy stuff are left as exercises to interested parties. Not ready
Re: 1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?
Misty De Meo brew.sh> writes: > > Hi, > > Gitignore parsing no longer seems to work properly in git 1.8.3. > > One of my repositories has the following gitignore: > > /* > !/.gitignore > !/Library/ > !/CONTRIBUTING.md > !/README.md > !/SUPPORTERS.md > !/bin > /bin/* > !/bin/brew > !/share/man/man1/brew.1 > .DS_Store > /Library/LinkedKegs > /Library/PinnedKegs > /Library/Taps > /Library/Formula/.gitignore > > In 1.8.2.3 and earlier, this works as expected. However, in 1.8.3 I'm > seeing every file in /bin/ being marked as an untracked file. > > I asked about this in #git, and was told that the culprit was the > regex support; apparently recompiling without regex support fixes the > specific gitignore issue. However, this doesn't seem to have been > reported anywhere on the mailing list that I can see. I was also told > that the issue is OS X-specific, and doesn't happen on other > platforms. > > Thanks, > Misty De Meo > I see a similar problem using e.g. the following .gitignore to exclude everything except C source files and header files: * !*/ !*.c !*.h In Git 1.8.3 'git status' will show other files as untracked while in Git 1.8.2.3 I don't have that problem. I bisected to find that the offending commit is v1.8.2.1-402-g95c6f27. I am not on OSX, however, but on Linux (Ubuntu 12.04 and RHEL 5.8) so this may be a separate issue. I've also gotten the impression that this is intentional. In any case I cannot create a .gitignore that achieves the same for both older and newer versions of Git. Best regards, Øystein Walle -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #04; Wed, 15)
Duy Nguyen writes: > Point taken. I guess the message would be something like this? > > Refname '%.*s' is ignored. It may be created by mistake. > > Or should we be more elaborate? I dunno; with s/may be/may have been/, I think it is better than "refname is ambiguous". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: contrib/git-normal-to-bare.sh
Zenaan Harkness writes: > I needed this quite a bit in the last few days, basic script but > serves my need. I think it would be useful for other beginners if in > $git/contrib/ source dir. > > Just a start to a basic script. Needs more tests etc, but it's enough > to get newbies (like me) off to a reasonable start. Handles multiple > input dirs. > > PLEASE CC me, as I am not subscribed. > > (some SMTP server rejected attachment, so pasting below instead) > > Thanks, > Zenaan > > > #!/bin/bash I do not think you need (nor used) any bash-ism in this script. Saying "#!/bin/sh" here is cleaner. > > # Change one or more normal repos into bare repos: > # See also > https://git.wiki.kernel.org/index.php/GitFaq#How_do_I_make_existing_non-bare_repository_bare.3F > > for i in "$@"; do for i do You do not have to say 'in "$@"'; it is implied. >echo; echo "--" >echo Processing $i Forgot to dq? > >repo="$i" >repo="`basename $i`" >tmp_repo="${repo}.git" ># Insert here: may be exit if any spaces in repo fqn ># Insert here: check for non-existent repo/.git dir ># Insert here: check that we are not inside the repo ># Insert here: add exit/do-nothing if fail to mv dirs etc > >mv $repo/.git $tmp_repo Forgot to dq? i.e. mv "$repo/.git" "$tmp_repo" The same for all the variable references in the remainder of the script. More importantly, "mv" would fail if $repo is given as a full pathname elsewhere in the filesystem that is different from your current directory where you create $tmp_repo. >git --git-dir=$tmp_repo config core.bare true >mv $repo ${repo}.bak >mv $tmp_repo $repo > done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git clone does not understand insteadOf URLs
Andreas Schwab writes: > Gioele Barabucci writes: > >> Simple, I keep all my projects on the same server, so I would like to >> refer to that server + path using 'remote-repo'. >> >> "git+ssh://git.example.org//users/gioele/projects" insteadOf "remote-repo" > > You can use "remote-repo:" instead. That "with colon" is pretty much how it was designed to be used, but the code did not insist that the insteadof has at least the scheme part, and by mistake allowed something like this [url "k.org:/pub/scm/git"] insteadOf = g to be used with "g/foo" to expand to "k.org:/pub/scm/git/foo". But if "git fetch g/foo" with such a configuration happily goes to "k.org:/pub/scm/git/foo" without problems, then I think it is very reasonable to delay the "is that a local filename?" short-cut "git clone" does and let the insteadof URL expand take place before it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] remote-helpers: add exec-path links
Felipe Contreras writes: > This way we don't have to modify the PATH ourselves and it's easier to > test without 'make'. > > Signed-off-by: Felipe Contreras > --- > .gitignore | 2 ++ > contrib/remote-helpers/Makefile | 13 + > 2 files changed, 11 insertions(+), 4 deletions(-) If we teach the top-level .gitignore that some contrib/ stuff might contaminate the top-level directory (which is a good idea), we should also teach the top-level Makefile how to get rid of the built cruft upon "make clean", no? > diff --git a/.gitignore b/.gitignore > index 10aee94..a79b412 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -119,11 +119,13 @@ > /git-reflog > /git-relink > /git-remote > +/git-remote-bzr > /git-remote-http > /git-remote-https > /git-remote-ftp > /git-remote-ftps > /git-remote-fd > +/git-remote-hg > /git-remote-ext > /git-remote-testgit > /git-remote-testpy > diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile > index 55abf0b..98150b4 100644 > --- a/contrib/remote-helpers/Makefile > +++ b/contrib/remote-helpers/Makefile > @@ -1,9 +1,9 @@ > TESTS := $(wildcard test-*.t) > SCRIPTS := $(wildcard git-remote-*.py) > +LINKS := $(addprefix ../../,$(patsubst %.py,%,$(SCRIPTS))) > > export T := $(addprefix $(CURDIR)/,$(TESTS)) > export MAKE := $(MAKE) -e > -export PATH := $(CURDIR):$(PATH) > export TEST_LINT := test-lint-executable test-lint-shell-syntax > export TEST_DIRECTORY := $(CURDIR)/../../t > > @@ -15,10 +15,15 @@ all: $(SCRIPTS) > install: > $(MAKE) -C ../.. install-python-script > > -test: all > +links: all $(LINKS) > + > +test: links > $(MAKE) -C ../../t $@ > > -$(TESTS): all > +$(LINKS): > + ln -sf contrib/remote-helpers/$(notdir $@) ../.. > + > +$(TESTS): links > $(MAKE) -C ../../t $(CURDIR)/$@ > > -.PHONY: all install test $(TESTS) > +.PHONY: all install test links $(TESTS) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] remote-helpers: rename tests
Felipe Contreras writes: > The .t extension is more standard for sharness tests. > > Signed-off-by: Felipe Contreras > --- Is that "sharness" test the sh script testsuite forked from our testsuite? I do not see how it makes sense to copy how they deviate from us back to our codebase, especially if we plan to eventually move some of these tests out of contrib/ area, but even without such a plan in the future. > contrib/remote-helpers/Makefile| 2 +- > contrib/remote-helpers/{test-bzr.sh => test-bzr.t} | 0 > contrib/remote-helpers/{test-hg-bidi.sh => test-hg-bidi.t} | 0 > contrib/remote-helpers/{test-hg-hg-git.sh => test-hg-hg-git.t} | 0 > contrib/remote-helpers/{test-hg.sh => test-hg.t} | 0 > 5 files changed, 1 insertion(+), 1 deletion(-) > rename contrib/remote-helpers/{test-bzr.sh => test-bzr.t} (100%) > rename contrib/remote-helpers/{test-hg-bidi.sh => test-hg-bidi.t} (100%) > rename contrib/remote-helpers/{test-hg-hg-git.sh => test-hg-hg-git.t} (100%) > rename contrib/remote-helpers/{test-hg.sh => test-hg.t} (100%) > > diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile > index d9b3515..2c91ec6 100644 > --- a/contrib/remote-helpers/Makefile > +++ b/contrib/remote-helpers/Makefile > @@ -1,4 +1,4 @@ > -TESTS := $(wildcard test*.sh) > +TESTS := $(wildcard test-*.t) > SCRIPTS := $(wildcard git-remote-*.py) > > export T := $(addprefix $(CURDIR)/,$(TESTS)) > diff --git a/contrib/remote-helpers/test-bzr.sh > b/contrib/remote-helpers/test-bzr.t > similarity index 100% > rename from contrib/remote-helpers/test-bzr.sh > rename to contrib/remote-helpers/test-bzr.t > diff --git a/contrib/remote-helpers/test-hg-bidi.sh > b/contrib/remote-helpers/test-hg-bidi.t > similarity index 100% > rename from contrib/remote-helpers/test-hg-bidi.sh > rename to contrib/remote-helpers/test-hg-bidi.t > diff --git a/contrib/remote-helpers/test-hg-hg-git.sh > b/contrib/remote-helpers/test-hg-hg-git.t > similarity index 100% > rename from contrib/remote-helpers/test-hg-hg-git.sh > rename to contrib/remote-helpers/test-hg-hg-git.t > diff --git a/contrib/remote-helpers/test-hg.sh > b/contrib/remote-helpers/test-hg.t > similarity index 100% > rename from contrib/remote-helpers/test-hg.sh > rename to contrib/remote-helpers/test-hg.t -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool --dir-diff: always use identical working tree file
John Keeping writes: >> - When comparing two revisions, e.g. "--dir-diff HEAD^^ HEAD^", >>that checks out (via $rsha1 to "checkout -f" codepath) a blob >>that does not match what is in the working tree of HEAD to the >>temporary directory, we still allow modifications to the copy in >>the temporary directory, but what can the user do with these >>changes that are _not_ based on HEAD, short of checking out HEAD^ >>and apply the difference first? >> >> I still cannot shake this nagging feeling that giving a writable >> temporary directory might have been a mistake in the first place. >> Perhaps it may be a better design to make the ones that the user >> shouldn't touch (or will lead to the above confusion) read-only, >> while the ones that match the working tree read-write? > > My ideal scenario would be that we only allow users to edit files when > they are comparing against the working tree, but that would require > git-difftool to fully understand all git-diff options since it just > passes through any it doesn't recognise. I don't think there's an easy > way to do that, which leaves us with this confusing situation. Not necessarily. Let's assume that changing files in "diff" tool is a sensible thing to do, as long as we make sure such a change is not lost (which I may not 100% agree with, but let's put it aside for now). When you are viewing a file F in "--dir-diff HEAD^^ HEAD^", if there is no change for F in between HEAD^ and HEAD and you notice a typo that may or may not be related to the differences between HEAD^^ and HEAD^, it would be tempting to fix that right there. And as long as F in the working tree matches that of HEAD^ and the modification you make in the temporary directory gets copied back to the working tree, your typofix will end up to be in the working tree. Which I _think_ is what people, who want to change files in "diff" tool, want to do. Of course, your working tree may have been in the middle of doing something entirely different and you may have to "add [-p]" to separate such a typofix with other changes you were working on, but that is a separate issue. And for that to work, the only think you need is "does the blob we show on the RHS temporary tree match what is in the working tree?" check. You do not need to know or care if you are comparing two old revisions, etc. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool --dir-diff: always use identical working tree file
On Tue, May 28, 2013 at 11:57:08AM -0700, Junio C Hamano wrote: > John Keeping writes: > > > Yeah, the commit message is still quite focused on the end effect of > > copying files back. But that's not what's being changed here. > > > > In my suggested commit message I tried to make it clear that we're > > changing when we decide to copy a file across to the temporary tree. > > This has the beneficial (side-)effect of changing the set of files we > > consider for copying back into the working tree after the diff tool has > > been run. > > I actually think the effect of copying files back _is_ the primary > motivation of this change, and stressing that end effect is a much > better description. After all, if the working tree files do not > have any difference from the RHS of the comparison, copying from the > working tree and stuffing the $rsha1 to the RHS temporary index and > running "checkout -f" should produce identical temporary directory > for the user to start viewing. > > The _only_ difference in behaviour before and after this patch that > matters to the end user is if that path is in @working_tree, which > is returned to @worktree of dir_diff sub to be later copied back, > no? I would view this change as a mere means, an implementation > detail, to achieve that end of stuffing more paths in the @worktree > array. I agree with this, but like you I found it confusing that the patch touched code seemingly unrelated to copying files back. I went toward describing the patch more literally and giving the motivation in the final paragraph. Your message below is better, but I think it needs to say that the set of files considered for copying back is the set that is copied across to begin with. > Perhaps > > difftool --dir-diff: allow changing any clean working tree file > > The temporary directory prepared by "difftool --dir-diff" to > show the result of a change can be modified by the user via > the tree diff program, and we try hard not to lose changes > to them after tree diff program returns to us. > > However, the set of files to be copied back is computed > differently between --symlinks and --no-symlinks modes. The > former checks all paths that start out as identical to the > working tree file, while the latter checks paths that > already had a local modification in the working tree, > allowing changes made in the tree diff program to paths that > did not have any local change to be lost. > > or something. This invites a few questions, though. > > - By allowing these files in the temporary directory to be >modified, aren't we making the user's life harder by forcing them >to handle "working tree file was already modified, made different >changes in the temporary directory, now these changes need to be >consolidated" case? > > - When comparing two revisions, e.g. "--dir-diff HEAD^^ HEAD^", >that checks out (via $rsha1 to "checkout -f" codepath) a blob >that does not match what is in the working tree of HEAD to the >temporary directory, we still allow modifications to the copy in >the temporary directory, but what can the user do with these >changes that are _not_ based on HEAD, short of checking out HEAD^ >and apply the difference first? > > I still cannot shake this nagging feeling that giving a writable > temporary directory might have been a mistake in the first place. > Perhaps it may be a better design to make the ones that the user > shouldn't touch (or will lead to the above confusion) read-only, > while the ones that match the working tree read-write? My ideal scenario would be that we only allow users to edit files when they are comparing against the working tree, but that would require git-difftool to fully understand all git-diff options since it just passes through any it doesn't recognise. I don't think there's an easy way to do that, which leaves us with this confusing situation. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool --dir-diff: always use identical working tree file
John Keeping writes: > Yeah, the commit message is still quite focused on the end effect of > copying files back. But that's not what's being changed here. > > In my suggested commit message I tried to make it clear that we're > changing when we decide to copy a file across to the temporary tree. > This has the beneficial (side-)effect of changing the set of files we > consider for copying back into the working tree after the diff tool has > been run. I actually think the effect of copying files back _is_ the primary motivation of this change, and stressing that end effect is a much better description. After all, if the working tree files do not have any difference from the RHS of the comparison, copying from the working tree and stuffing the $rsha1 to the RHS temporary index and running "checkout -f" should produce identical temporary directory for the user to start viewing. The _only_ difference in behaviour before and after this patch that matters to the end user is if that path is in @working_tree, which is returned to @worktree of dir_diff sub to be later copied back, no? I would view this change as a mere means, an implementation detail, to achieve that end of stuffing more paths in the @worktree array. Perhaps difftool --dir-diff: allow changing any clean working tree file The temporary directory prepared by "difftool --dir-diff" to show the result of a change can be modified by the user via the tree diff program, and we try hard not to lose changes to them after tree diff program returns to us. However, the set of files to be copied back is computed differently between --symlinks and --no-symlinks modes. The former checks all paths that start out as identical to the working tree file, while the latter checks paths that already had a local modification in the working tree, allowing changes made in the tree diff program to paths that did not have any local change to be lost. or something. This invites a few questions, though. - By allowing these files in the temporary directory to be modified, aren't we making the user's life harder by forcing them to handle "working tree file was already modified, made different changes in the temporary directory, now these changes need to be consolidated" case? - When comparing two revisions, e.g. "--dir-diff HEAD^^ HEAD^", that checks out (via $rsha1 to "checkout -f" codepath) a blob that does not match what is in the working tree of HEAD to the temporary directory, we still allow modifications to the copy in the temporary directory, but what can the user do with these changes that are _not_ based on HEAD, short of checking out HEAD^ and apply the difference first? I still cannot shake this nagging feeling that giving a writable temporary directory might have been a mistake in the first place. Perhaps it may be a better design to make the ones that the user shouldn't touch (or will lead to the above confusion) read-only, while the ones that match the working tree read-write? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool --dir-diff: always use identical working tree file
On Tue, May 28, 2013 at 11:06:13AM -0700, Junio C Hamano wrote: > Kenichi Saita writes: > > > When deciding whether or not we should link a working tree file into > > the temporary right-hand directory for a directory diff, we > > currently behave differently in the --symlink and --no-symlink > > cases. If using symlinks any identical files are linked across but > > with --no-symlink only files that contain unstaged changes are > > copied back into the working tree. > > I may have missed an earlier discussion, but I do not follow the > last sentence. The former part (i.e. symlinks) talks about what is > done to populate the temporary tree (i.e. everything is linked), but > the latter part (i.e. not symlinks) only talks about what is copied > back, i.e. it is not a contrast between the behaviour of symlink vs > no-symlink case wrt how the temporary tree is populated. > > Confused... Yeah, the commit message is still quite focused on the end effect of copying files back. But that's not what's being changed here. In my suggested commit message I tried to make it clear that we're changing when we decide to copy a file across to the temporary tree. This has the beneficial (side-)effect of changing the set of files we consider for copying back into the working tree after the diff tool has been run. > > Change this so that identical files are copied back as well. This > > is beneficial because it widens the set of circumstances in which we > > copy changes made by the user back into the working tree. > > Ah, OK, you meant that the set of files we keep in @working_tree > array for later copying back are different between the two. > > > Signed-off-by: Kenichi Saita > > --- > > git-difftool.perl |9 ++--- > > t/t7800-difftool.sh | 19 +++ > > 2 files changed, 21 insertions(+), 7 deletions(-) > > > > diff --git a/git-difftool.perl b/git-difftool.perl > > index 8a75205..e57d3d1 100755 > > --- a/git-difftool.perl > > +++ b/git-difftool.perl > > @@ -85,13 +85,9 @@ sub exit_cleanup > > > > sub use_wt_file > > { > > - my ($repo, $workdir, $file, $sha1, $symlinks) = @_; > > + my ($repo, $workdir, $file, $sha1) = @_; > > my $null_sha1 = '0' x 40; > > > > - if ($sha1 ne $null_sha1 and not $symlinks) { > > - return 0; > > - } > > - > > if (! -e "$workdir/$file") { > > # If the file doesn't exist in the working tree, we cannot > > # use it. > > @@ -213,8 +209,7 @@ EOF > > > > if ($rmode ne $null_mode) { > > my ($use, $wt_sha1) = use_wt_file($repo, $workdir, > > - $dst_path, $rsha1, > > - $symlinks); > > + $dst_path, $rsha1); > > if ($use) { > > push @working_tree, $dst_path; > > $wtindex .= "$rmode $wt_sha1\t$dst_path\0"; > > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh > > index d46f041..2418528 100755 > > --- a/t/t7800-difftool.sh > > +++ b/t/t7800-difftool.sh > > @@ -385,6 +385,25 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff > > --symlink without unstage > > test_cmp actual expect > > ' > > > > +write_script modify-right-file <<\EOF > > +echo "new content" >"$2/file" > > +EOF > > + > > +run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged > > change' ' > > + test_when_finished git reset --hard && > > + echo "orig content" >file && > > + git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch && > > + echo "new content" >expect && > > + test_cmp expect file > > +' > > + > > +run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged > > change' ' > > + test_when_finished git reset --hard && > > + git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch && > > + echo "new content" >expect && > > + test_cmp expect file > > +' > > + > > write_script modify-file <<\EOF > > echo "new content" >file > > EOF > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails
On Thu, May 23, 2013 at 10:05:03PM +0200, Matthieu Moy wrote: > My use-case is an invalid SSL certificate. Pulling from the wiki with a > recent version of libwww-perl fails, and git-remote-mediawiki gave no > clue about the reason. Give the mediawiki API detailed error message, and > since it is not so informative, hint the user about an invalid SSL > certificate on https:// urls. This is definitely an improvement, but it seems like it just the tip of the iceberg. The call in get_mw_tracked_categories already uses die() with the MW error code and detailed string. Good. The call you fix here prints a guess to stderr and exits 1, and your patch improves that. But the call in get_mw_first_pages does the same broken thing, and is not fixed. Ditto for get_all_mediafiles. Other calls do not even seem to error check the result at all, and assume the result is not undef (which I suspect would produce a perl runtime error). I wonder if we can do something like: our $mw_operation; $mediawiki->{config}->{on_error} = sub { my $err = "fatal: "; if (defined $mw_operation) { $err .= "unable to $mw_operation: "; } err .= $mediawiki->{error}->{details}; die "$err\n"; }; and then callers do not have to worry about error-checking at all. If they want a nicer human-readable indication of where the error occured, they can do: local $mw_operation = "get the list of remote wiki pages"; my $mw_pages = $mediawiki->list(...); -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool --dir-diff: always use identical working tree file
Kenichi Saita writes: > When deciding whether or not we should link a working tree file into > the temporary right-hand directory for a directory diff, we > currently behave differently in the --symlink and --no-symlink > cases. If using symlinks any identical files are linked across but > with --no-symlink only files that contain unstaged changes are > copied back into the working tree. I may have missed an earlier discussion, but I do not follow the last sentence. The former part (i.e. symlinks) talks about what is done to populate the temporary tree (i.e. everything is linked), but the latter part (i.e. not symlinks) only talks about what is copied back, i.e. it is not a contrast between the behaviour of symlink vs no-symlink case wrt how the temporary tree is populated. Confused... > Change this so that identical files are copied back as well. This > is beneficial because it widens the set of circumstances in which we > copy changes made by the user back into the working tree. Ah, OK, you meant that the set of files we keep in @working_tree array for later copying back are different between the two. > Signed-off-by: Kenichi Saita > --- > git-difftool.perl |9 ++--- > t/t7800-difftool.sh | 19 +++ > 2 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/git-difftool.perl b/git-difftool.perl > index 8a75205..e57d3d1 100755 > --- a/git-difftool.perl > +++ b/git-difftool.perl > @@ -85,13 +85,9 @@ sub exit_cleanup > > sub use_wt_file > { > - my ($repo, $workdir, $file, $sha1, $symlinks) = @_; > + my ($repo, $workdir, $file, $sha1) = @_; > my $null_sha1 = '0' x 40; > > - if ($sha1 ne $null_sha1 and not $symlinks) { > - return 0; > - } > - > if (! -e "$workdir/$file") { > # If the file doesn't exist in the working tree, we cannot > # use it. > @@ -213,8 +209,7 @@ EOF > > if ($rmode ne $null_mode) { > my ($use, $wt_sha1) = use_wt_file($repo, $workdir, > - $dst_path, $rsha1, > - $symlinks); > + $dst_path, $rsha1); > if ($use) { > push @working_tree, $dst_path; > $wtindex .= "$rmode $wt_sha1\t$dst_path\0"; > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh > index d46f041..2418528 100755 > --- a/t/t7800-difftool.sh > +++ b/t/t7800-difftool.sh > @@ -385,6 +385,25 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff > --symlink without unstage > test_cmp actual expect > ' > > +write_script modify-right-file <<\EOF > +echo "new content" >"$2/file" > +EOF > + > +run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged change' ' > + test_when_finished git reset --hard && > + echo "orig content" >file && > + git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch && > + echo "new content" >expect && > + test_cmp expect file > +' > + > +run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged > change' ' > + test_when_finished git reset --hard && > + git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch && > + echo "new content" >expect && > + test_cmp expect file > +' > + > write_script modify-file <<\EOF > echo "new content" >file > EOF -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Junio C Hamano writes: > Thomas Rast Cc'ed as he has been the primary force behind this line > of "notes" usability. Thanks for pointing this out to me. > Felipe Contreras writes: > >> Signed-off-by: Felipe Contreras >> --- >> builtin/revert.c | 2 + >> sequencer.c | 136 >> -- >> sequencer.h | 2 + >> t/t3500-cherry.sh | 32 + >> 4 files changed, 169 insertions(+), 3 deletions(-) > > "git cherry-pick" should help maintaining notes just like amend and > rebase, but how should this interact with notes.rewrite., > where the command is capable of doing this without an explicit > option once you tell which notes need to be maintained? Since we already have the notes.rewrite. convention, it would seem the obvious choice to line it up with the others. The main bikeshedding opportunity is whether this should be an exception and default to false (all other commands default it to true). Also: how does this interact with notes.rewriteRef and the corresponding env vars? Why? How does it interact with 'cherry-pick -n' if this is done in sequence, effectively squashing several commits (this use-case is actually suggested by the manpage), if multiple source commits had notes? Should it respect notes.rewriteMode (and by default concatenate)? (I don't know if the sequencer state is expressive enough already to carry this in a meaningful way across cherry-pick commands.) A commit message and some docs would be a nice idea, too. > diff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh > index f038f34..79c1219 100755 > --- a/t/t3500-cherry.sh > +++ b/t/t3500-cherry.sh This file starts out with test_description='git cherry should detect patches integrated upstream This test cherry-picks one local change of two into master branch, and checks that git cherry only returns the second patch in the local branch ' So either your tests should go to a different file or the description becomes stale and needs to be updated. -- Thomas Rast trast@{inf,student}.ethz.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?
Hi, Gitignore parsing no longer seems to work properly in git 1.8.3. One of my repositories has the following gitignore: /* !/.gitignore !/Library/ !/CONTRIBUTING.md !/README.md !/SUPPORTERS.md !/bin /bin/* !/bin/brew !/share/man/man1/brew.1 .DS_Store /Library/LinkedKegs /Library/PinnedKegs /Library/Taps /Library/Formula/.gitignore In 1.8.2.3 and earlier, this works as expected. However, in 1.8.3 I'm seeing every file in /bin/ being marked as an untracked file. I asked about this in #git, and was told that the culprit was the regex support; apparently recompiling without regex support fixes the specific gitignore issue. However, this doesn't seem to have been reported anywhere on the mailing list that I can see. I was also told that the issue is OS X-specific, and doesn't happen on other platforms. Thanks, Misty De Meo -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git hangs on pthread_join
On Thu, May 23, 2013 at 03:01:43PM +0200, Ian Kumlien wrote: > git 1.8.2.1 is started by xinetd > [...] > I have found "git receive-pack"s that has been running for days/weeks > without terminating > > Attaching gdb and doing a trace results in: > #0 0x003261207b35 in pthread_join () from /lib64/libpthread.so.0 > #1 0x004ce58b in finish_async () > #2 0x0045744b in cmd_receive_pack () > #3 0x00404851 in handle_internal_command () > #4 0x00404c9d in main () I recently fixed a deadlock that could happen in receive-pack when clients hung up before sending a valid pack header. The fix is commit 49ecfa1, and it's in git v1.8.2.2. The stack trace for the deadlock fixed by 49ecfa1 would have unpack_with_sideband between #1 and #2 above, but it is entirely possible that it is simply inlined in your build of git, depending on the -O level of your build (it is a static function that is only called from one place). So it seems likely that it is the culprit. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] test: improve rebase -q test
Jonathan Nieder writes: > Junio C Hamano wrote: > >> A more preferrable alternative may be adding something like this to >> test-lib.sh and call it from here and elsewhere (there are about 50 >> places that do "test ! -s "), perhaps? >> >> test_must_be_an_empty_file () { >> if test -s "$1" >> then >> cat "$1" >> false >> fi >> } > > I generally just use the two-liner > > >empty && > test_cmp empty output > > directly in cases like this. That would work, too. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix segfault with git log -c --follow
Clemens Buchacher writes: > In diff_tree_combined we make a copy of diffopts. In > try_to_follow_renames, called via diff_tree_sha1, we free and > re-initialize diffopts->pathspec->items. Since we did not make a deep > copy of diffopts in diff_tree_combined, the original diffopts does not > get the update. By the time we return from diff_tree_combined, > rev->diffopt->pathspec->items points to an invalid memory address. We > get a segfault next time we try to access that pathspec. I am not quite sure if I follow. Do you mean diff_tree_combined() - makes a shallow copy of rev->diffopt - calls diff_tree_sha1() diff_tree_sha1() - tries to follow rename and clobbers diffopt - tries to use the shallow copy of original rev->diffopt that no longer is valid, which is a problem I wonder, just like we force recursive and disable external on the copy before we use it to call diff_tree_sha1(), if we should disable follow-renames on it. "--follow" is an option that is given to the history traversal part and it should not play any role in getting the pairwise diff with all parents diff_tree_combined() does. Besides, - "--follow" hack lets us keep track of only one path; and - "-c" and "--cc" make sense only when dealing with a merge commit and the path in the child may have come from different path in parents, so I am not sure if allowing combination of "--follow -c/--cc" makes much sense in the first place. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] test: improve rebase -q test
Junio C Hamano wrote: > A more preferrable alternative may be adding something like this to > test-lib.sh and call it from here and elsewhere (there are about 50 > places that do "test ! -s "), perhaps? > > test_must_be_an_empty_file () { > if test -s "$1" > then > cat "$1" > false > fi > } I generally just use the two-liner >empty && test_cmp empty output directly in cases like this. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] rebase: improve the keep-empty
Hi, I think I have some patches at home that instead teach 'git am' the --keep-empty flag. Does that make sense? It's been a while since I looked at it, but I'll try to take a look tonight (PST). Martin On Tue, May 28, 2013 at 6:29 AM, Felipe Contreras wrote: > Hi, > > I've been analyzing 'git rebase' and found that the --keep-empty option > triggers a very very different behavior. Here's a bunch of patches that make > it > behave like the 'am' does does for the most part. > > There's only a few minor changes, after which it might be possible to replace > the whole 'am' mode to use cherr-pick instead. > > Felipe Contreras (5): > rebase: split the cherry-pick stuff > rebase: fix 'cherry' mode storage > rebase: fix sequence continuation > rebase: fix abort of cherry mode > rebase: fix cherry-pick invocations > > .gitignore| 1 + > Makefile | 1 + > git-rebase--am.sh | 65 > ++- > git-rebase--cherry.sh | 55 +++ > git-rebase.sh | 8 +++ > 5 files changed, 93 insertions(+), 37 deletions(-) > create mode 100644 git-rebase--cherry.sh > > -- > 1.8.3.rc3.312.g47657de > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
where is the opensuse version for git?
Dear developer trying to find a version for suse 12.3 I was unable to locate a version. Please advise. downloading x-tags error messages. error message 504-gateway time out. Please advise what to do. regards Ludger -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Felipe Contreras writes: > Signed-off-by: Felipe Contreras > --- > builtin/revert.c | 2 + > sequencer.c | 136 > -- > sequencer.h | 2 + > t/t3500-cherry.sh | 32 + > 4 files changed, 169 insertions(+), 3 deletions(-) "git cherry-pick" should help maintaining notes just like amend and rebase, but how should this interact with notes.rewrite., where the command is capable of doing this without an explicit option once you tell which notes need to be maintained? Thomas Rast Cc'ed as he has been the primary force behind this line of "notes" usability. It probably is not sensible to carry over note from a reverted commit to its revert, but I didn't immediately spot how the patch does this only for cherry-pick but not for revert (the codepath in do_pick_commit() is shared between them, no?). > diff --git a/builtin/revert.c b/builtin/revert.c > index 0e5ce71..b977124 100644 > --- a/builtin/revert.c > +++ b/builtin/revert.c > @@ -119,6 +119,7 @@ static void parse_args(int argc, const char **argv, > struct replay_opts *opts) > OPT_END(), > OPT_END(), > OPT_END(), > + OPT_END(), > }; > > if (opts->action == REPLAY_PICK) { > @@ -129,6 +130,7 @@ static void parse_args(int argc, const char **argv, > struct replay_opts *opts) > OPT_BOOLEAN(0, "allow-empty-message", > &opts->allow_empty_message, N_("allow commits with empty messages")), > OPT_BOOLEAN(0, "keep-redundant-commits", > &opts->keep_redundant_commits, N_("keep redundant, empty commits")), > OPT_BOOLEAN(0, "skip-empty", &opts->skip_empty, > N_("skip empty commits")), > + OPT_BOOLEAN(0, "copy-notes", &opts->copy_notes, > N_("copy notes")), > OPT_END(), > }; > if (parse_options_concat(options, ARRAY_SIZE(options), > cp_extra)) > diff --git a/sequencer.c b/sequencer.c > index edf141d..35a84ad 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -20,6 +20,18 @@ > const char sign_off_header[] = "Signed-off-by: "; > static const char cherry_picked_prefix[] = "(cherry picked from commit "; > > +struct rewritten_list_item { > + unsigned char from[20]; > + unsigned char to[20]; > +}; > + > +struct rewritten_list { > + struct rewritten_list_item *items; > + unsigned int nr, alloc; > +}; > + > +static struct rewritten_list rewritten; > + > static int is_rfc2822_line(const char *buf, int len) > { > int i; > @@ -102,6 +114,40 @@ static int has_conforming_footer(struct strbuf *sb, > struct strbuf *sob, > return 1; > } > > +static void add_rewritten(unsigned char *from, unsigned char *to) > +{ > + struct rewritten_list_item *item; > + if (rewritten.nr + 1 >= rewritten.alloc) { > + rewritten.alloc += 32; > + rewritten.items = xrealloc(rewritten.items, rewritten.alloc * > sizeof(*rewritten.items)); > + } > + item = &rewritten.items[rewritten.nr]; > + hashcpy(item->from, from); > + hashcpy(item->to, to); > + rewritten.nr++; > +} > + > +static void copy_notes(void) > +{ > + unsigned char note_commit[20]; > + struct strbuf buf = STRBUF_INIT; > + struct notes_tree *t = &default_notes_tree; > + int i; > + > + init_notes(t, NULL, NULL, 0); > + > + for (i = 0; i < rewritten.nr; i++) { > + struct rewritten_list_item *item = &rewritten.items[i]; > + copy_note(t, item->from, item->to, 0, NULL); > + } > + > + strbuf_addstr(&buf, "Notes added by 'git cherry-pick'\n"); > + create_notes_commit(&default_notes_tree, NULL, &buf, note_commit); > + strbuf_insert(&buf, 0, "notes: ", 7); > + update_ref(buf.buf, t->ref, note_commit, NULL, 0, MSG_ON_ERR); > + strbuf_release(&buf); > +} > + > static void remove_sequencer_state(void) > { > struct strbuf seq_dir = STRBUF_INIT; > @@ -641,6 +687,14 @@ static int do_pick_commit(struct commit *commit, struct > replay_opts *opts) > res = run_git_commit(defmsg, opts, allow); > } > > + if (!res && opts->copy_notes) { > + unsigned char to[20]; > + > + if (read_ref("HEAD", to)) > + goto leave; > + > + add_rewritten(commit->object.sha1, to); > + } > leave: > free_message(&msg); > free(defmsg); > @@ -786,6 +840,40 @@ static void read_populate_todo(struct commit_list > **todo_list, > die(_("Unusable instruction sheet: %s"), todo_file); > } > > +static void read_populate_rewritten(void) > +{ > + const char *rewritten_file = git_path(SEQ_REWR_FILE); > + struct strbuf buf = STRBUF_INIT; > + char *p; > + int fd; > + > + fd = open(rewritten_file, O_RDONLY); > + if (fd < 0) > + return; > + if (strbuf_read(&buf, fd, 0) < 0) { > + close
Re: [PATCH 4/5] test: improve rebase -q test
Felipe Contreras writes: > Let's show the output so it's clear why it failed. > > Signed-off-by: Felipe Contreras > --- > t/t3400-rebase.sh | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh > index b58fa1a..fb39531 100755 > --- a/t/t3400-rebase.sh > +++ b/t/t3400-rebase.sh > @@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when upstream > arg is missing' ' > test_expect_success 'rebase -q is quiet' ' > git checkout -b quiet topic && > git rebase -q master >output.out 2>&1 && > + cat output.out && > test ! -s output.out > ' It is one thing to avoid squelching output that naturally comes out of command being tested unnecessarily, so that "./t-*.sh -v" output can be used for debugging. I however am not sure if adding "cat" to random places like this is a productive direction for us to go in. A more preferrable alternative may be adding something like this to test-lib.sh and call it from here and elsewhere (there are about 50 places that do "test ! -s "), perhaps? test_must_be_an_empty_file () { if test -s "$1" then cat "$1" false fi } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] remote: trivial style cleanup
Felipe Contreras writes: > Signed-off-by: Felipe Contreras > --- > remote.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/remote.c b/remote.c > index 68eb99b..e71f66d 100644 > --- a/remote.c > +++ b/remote.c > @@ -1474,8 +1474,7 @@ struct branch *branch_get(const char *name) > ret->remote = remote_get(ret->remote_name); > if (ret->merge_nr) { > int i; > - ret->merge = xcalloc(sizeof(*ret->merge), > - ret->merge_nr); > + ret->merge = xcalloc(ret->merge_nr, > sizeof(*ret->merge)); Yeah, calloc is nmemb first and then size, so this makes sense. > for (i = 0; i < ret->merge_nr; i++) { > ret->merge[i] = xcalloc(1, > sizeof(**ret->merge)); > ret->merge[i]->src = > xstrdup(ret->merge_name[i]); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git clone depth of 0 not possible.
Matthijs Kooijman writes: > Did you consider how to implement this? Looking at the code, it seems > the "deepen" parameter in the wire protocol now means: > - 0: Do not change anything about the shallowness (i.e., fetch >everything from the shallow root to the tip). > - > 0: Create new shallow commits at depth commits below the tip (so >depth == 1 means tip and one below). > - INFINITE_DEPTH (0x7fff): Remove all shallowness and fetch >complete history. > > Given this, I'm not sure how one can express "fetch the tip and nothing > below that", since depth == 0 already has a different meaning. Doing it "correctly" (in the shorter term) would involve: - adding a capability on the sending side "fixed-off-by-one-depth" to the protocol, and teaching the sending side to advertise the capability; - teaching the requestor that got --depth=N from the end user to pay attention to the new capability in such a way that: - when talking to an old sender (i.e. without the off-by-one fix), send N-1 for N greater than 1. Punt on N==1; - when talking to a fixed sender, ask to enable the capability, and send N as is (including N==1). - teaching the sending side to see if the new behaviour to fix off-by-one is asked by the requestor, and stop at the correct number of commits, not oversending one more. Otherwise retain the old behaviour. In the longer term, I think we should introduce a better deepening mechanism. Cf. http://thread.gmane.org/gmane.comp.version-control.git/212912/focus=212940 > Of course, one could using depth == 1 in this case to receive two > commits and then drop one, but this would seem a bit pointless to me > (especially if the commit below the tip is very different from the tip > leading to a lot of useless data transfer). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND v2] path: Fix a sparse warning
Ramsay Jones writes: > On MinGW, sparse issues an "'get_st_mode_bits' not declared. Should > it be static?" warning. The MinGW and MSVC builds do not see the > declaration of this function, within git-compat-util.h, due to its > placement within an preprocessor conditional. > > In order to suppress the warning, we simply move the declaration to > the top level of the header. > > Signed-off-by: Ramsay Jones > --- > > Hi Junio, > > Now that v1.8.3 is out, I note that this patch seems to have been > dropped (or did I miss something?). > > This used to be > > [PATCH 2/6] path: Make the 'get_st_mode_bits' symbol a file static > > but the change in implementation required a change in title. > This version simply moves the declaration so that the MinGW and > MSVC builds can see it. Will queue. Can you tell me what the conclusion on the discussion on your two other patches on 'pu'? * rj/mingw-cygwin (2013-05-08) 2 commits - cygwin: Remove the CYGWIN_V15_WIN32API build variable - mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE I stopped keeping track of the discussion and my vague recollection was that it is OK for 1.5 but not verified on 1.7 or something? > > ATB, > Ramsay Jones > > git-compat-util.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/git-compat-util.h b/git-compat-util.h > index e955bb5..0e5e4f8 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -163,7 +163,6 @@ > typedef long intptr_t; > typedef unsigned long uintptr_t; > #endif > -int get_st_mode_bits(const char *path, int *mode); > #if defined(__CYGWIN__) > #undef _XOPEN_SOURCE > #include > @@ -176,6 +175,8 @@ int get_st_mode_bits(const char *path, int *mode); > #endif > #endif > > +extern int get_st_mode_bits(const char *path, int *mode); > + > /* used on Mac OS X */ > #ifdef PRECOMPOSE_UNICODE > #include "compat/precompose_utf8.h" -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
Neil Horman writes: > On Mon, May 27, 2013 at 11:52:18AM -0500, Felipe Contreras wrote: >> We should free objects before leaving. >> >> Signed-off-by: Felipe Contreras >> --- >> sequencer.c | 7 +-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/sequencer.c b/sequencer.c >> index ab6f8a7..7eeae2f 100644 >> --- a/sequencer.c >> +++ b/sequencer.c >> @@ -626,12 +626,15 @@ static int do_pick_commit(struct commit *commit, >> struct replay_opts *opts) >> rerere(opts->allow_rerere_auto); >> } else { >> int allow = allow_empty(opts, commit); >> -if (allow < 0) >> -return allow; >> +if (allow < 0) { >> +res = allow; >> +goto leave; >> +} >> if (!opts->no_commit) >> res = run_git_commit(defmsg, opts, allow); >> } >> >> +leave: >> free_message(&msg); >> free(defmsg); >> >> -- >> 1.8.3.rc3.312.g47657de >> >> > Acked-by: Neil Horman This is better done without "goto" in general. The other patch 2/2/ adds one more "we need to exit from the middle of the flow" and makes it look handier to add an exit label here, but it would be even better to express the logic of that patch as a normal cascade of if/else if/..., which is small enough and we do not need the "leave:" label. It probably is better to fold this patch into the other one when it is rerolled to correct the option name gotcha "on the tin". Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] remote-bzr: reorganize the way 'wanted' works
Felipe Contreras writes: > +wanted = get_config('remote-bzr.branches').rstrip().split(', ') Two minor nits and one design suggestion: - Why rstrip() not strip()? It appears that this only is helping an end-user "mistake" like this: git config remote-bzr.branches 'trunk, devel, test ' without helping people who have done this: git config remote-bzr.branches 'trunk, devel, test' - Is git config remote-bzr.branches trunk,devel,test a grave sin? In other words, wouldn't we want something like this instead? map(lambda s: s.strip(), get_config('...').split(',')) - Doesn't allowing multi-valued variable, e.g. [remote-bzr] branches = trunk branches = devel branches = test make it easier for the user to manage this configuration by e.g. selectively removing or adding tracked branches? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html