[PATCH v3 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 v3 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 v3 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 | 252 ++ 5 files changed, 254 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 v3 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 81e3d59..659a7b3 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 v3 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..75cc476 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 drops patches in upstream" "
+ reset_rebase &&
+ git rebase $* --onto f h i &&
+ test_cmp_rev f HEAD~2 &&
+ test_linear_range 'd i' f..
+ "
+}
+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 drop patches in onto" "
+ reset_rebase &&
+ git rebase $* --onto h f i &&
+ test_cmp_rev h HEAD~3 &&
+ test_linear_range 'd G i' h..
+ "
+}
+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 v3 0/7] Rebase topology test
Patches are now expected to be dropped iff they are on upstream. I've also followed all of Johannes's other suggestions except for the one about topo-order. 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 | 252 +++ 8 files changed, 666 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 v3 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 v3 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 75cc476..81e3d59 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase success -m
test_run_rebase success -i
test_run_rebase success -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 2/2] lookup_commit_reference_gently: do not read non-{tag,commit}
Thomas Rast wrote:
> diff --git a/commit.c b/commit.c
> index 888e02a..00e8d4a 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -31,8 +31,12 @@ static struct commit *check_commit(struct object *obj,
> struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
> int quiet)
> {
> - struct object *obj = deref_tag(parse_object(sha1), NULL, 0);
> -
> + struct object *obj;
> + int type = sha1_object_info(sha1, NULL);
> + /* If it's neither tag nor commit, parsing the object is wasted
> effort */
> + if (type != OBJ_TAG && type != OBJ_COMMIT)
> + return NULL;
> + obj = deref_tag(parse_object(sha1), NULL, 0);
> if (!obj)
> return NULL;
> return check_commit(obj, sha1, quiet);
As Jeff points out, you've introduced an extra sha1_object_info() call
in the common case of tag (which derefs into a commit anyway) and
commit slowing things down.
So, my main doubt centres around how sha1_object_info() determines the
type of the object without actually parsing it. You have to open up
the file and look at the fields near the top, no? (or fallback to blob
failing that). I am reading it:
1. It calls sha1_loose_object_info() or sha1_packed_object_info(),
depending on whether the particular file is in-pack or not. Lets see
what is common between them.
2. The loose counterpart seems to call unpack_sha1_header() after
mmap'ing the file. This ultimately ends up calling
unpack_object_header_buffer(), which is also what the packed
counterpart calls.
3. I didn't understand what unpack_object_header_buffer() is doing.
And'ing with some magic 0x80 and shifting by 4 bits iteratively? type
= (c >> 4) & 7?
In contrast, parse_object() first calls lookup_object() to look it up
in some hashtable to get the type -- the packfile idx, presumably?
Why don't you also do that instead of sha1_object_info()? Or, why
don't you wrap parse_object() in an API that doesn't go beyond the
first blob check (and not execute parse_object_buffer())?
Also, does this patch fix the bug Alex reported?
Apologies if I've misunderstood something horribly (which does seem to
be the case).
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: What's cooking in git.git (May 2013, #09; Wed, 29)
Junio C Hamano pobox.com> writes: > * kb/status-ignored-optim-2 (2013-05-29) 1 commit > - dir.c: fix ignore processing within not-ignored directories > > Fix 1.8.3 regressions in the .gitignore path exclusion logic. Hi, I see that the Tested-by line in kb/status-ignored-optim-2 (3973cbd) doesn't contain my e-mail address. I personally have no particular preference whether it's there or not, but I noticed that it usually is on Tested-by lines, so I'm wondering if I have slipped on some standard practice. Would be useful to know in case I get the chance to help out more :) Thanks, Øsse -- 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 5/7] add tests for rebasing merged history
On Wed, May 29, 2013 at 12:57 AM, Johannes Sixt wrote: > Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: >> +# a---b---c >> +# \ \ >> +# d---e \ >> +#\ \ \ >> +# n---o---w---v >> +# \ >> +# z > >> +#TODO: make all flavors of rebase use --topo-order >> +test_run_rebase success 'e n o' '' >> +test_run_rebase success 'e n o' -m >> +test_run_rebase success 'n o e' -i > > As test_commit offers predictable timestamps, I think you can work around > this discrepancy by generating commits n and o before e. (That is not a > solution--just a workaround that depends on the current > implementation--because the order in which parents of a merge are listed > is unspecified.) I actually liked it as documentation of the current inconsistency and with an explicit TODO. I have addressed the remainder of your comments in this and the next message. Thanks again. -- 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 v3] credential-osxkeychain: support more protocols
Add protocol imap, imaps, ftp and smtp for credential-osxkeychain.
Signed-off-by: Xidorn Quan
Acked-by: John Szakmeister
Acked-by: Jeff King
---
contrib/credential/osxkeychain/git-credential-osxkeychain.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
index 3940202..bcd3f57 100644
--- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
+++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
@@ -127,10 +127,20 @@ static void read_credential(void)
*v++ = '\0';
if (!strcmp(buf, "protocol")) {
- if (!strcmp(v, "https"))
+ if (!strcmp(v, "imap"))
+ protocol = kSecProtocolTypeIMAP;
+ else if (!strcmp(v, "imaps"))
+ protocol = kSecProtocolTypeIMAPS;
+ else if (!strcmp(v, "ftp"))
+ protocol = kSecProtocolTypeFTP;
+ else if (!strcmp(v, "ftps"))
+ protocol = kSecProtocolTypeFTPS;
+ else if (!strcmp(v, "https"))
protocol = kSecProtocolTypeHTTPS;
else if (!strcmp(v, "http"))
protocol = kSecProtocolTypeHTTP;
+ else if (!strcmp(v, "smtp"))
+ protocol = kSecProtocolTypeSMTP;
else /* we don't yet handle other protocols */
exit(0);
}
--
1.8.3
--
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 1/3] read-cache: plug a few leaks
On Thu, May 30, 2013 at 10:13 AM, René Scharfe wrote: > Am 30.05.2013 15:34, schrieb Felipe Contreras: >> We don't free 'istate->cache' properly. >> >> Apparently 'initialized' doesn't really mean initialized, but loaded, or >> rather 'not-empty', and the cache can be used even if it's not >> 'initialized', so we can't rely on this variable to keep track of the >> 'istate->cache'. >> >> So assume it always has data, and free it before overwriting it. >> >> Signed-off-by: Felipe Contreras >> --- >> read-cache.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/read-cache.c b/read-cache.c >> index 04ed561..e5dc96f 100644 >> --- a/read-cache.c >> +++ b/read-cache.c >> @@ -1449,6 +1449,7 @@ int read_index_from(struct index_state *istate, const >> char *path) >> istate->version = ntohl(hdr->hdr_version); >> istate->cache_nr = ntohl(hdr->hdr_entries); >> istate->cache_alloc = alloc_nr(istate->cache_nr); >> + free(istate->cache); > > With that change, callers of read_index_from need to set ->cache to > NULL for uninitialized (on-stack) index_state variables. They only had > to set ->initialized to 0 before in that situation. It this chunk safe > for all existing callers? Shouldn't the same free in discard_index > (added below) be enough? We can remove that line, but then if some code does this: discard_cache(); # add entries read_cache(); There will be a memory leak. -- 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] reflog: show committer date in verbose mode
2013/5/31 Jiang Xin :
> By default, reflog won't show committer date and for some cases won't
> show commit log either. It will be helpful to show them all by passing
> a more complicated pretty formatter to `git reflog` like this:
>
> $ git reflog show \
> --pretty="%Cred%h%Creset %gd: %gs%n >> %Cblue%ci (%cr)%Creset: %s"
>
For example:
$ git reflog show master
edca41 master@{0}: merge kernel/master: Fast-forward
5e49f master@{1}: merge kernel/master: Fast-forward
de3a5 master@{2}: merge kernel/master: Fast-forward
0c2b1 master@{3}: merge kernel/master: Fast-forward
b387c master@{4}: merge kernel/master: Fast-forward
9b795 master@{5}: merge kernel/master: Fast-forward
4dcdc master@{6}: merge jx/zh_CN: Fast-forward
a09ab0 master@{7}: merge de-ralfth/master: Fast-forward
674c5
$ git reflog show -v -v master
edca41 master@{0}: merge kernel/master: Fast-forward
>> Fri, 24 May 2013 11:34:46 -0700 (6 days ago), by Junio C
Hamano: Git 1.8.3
5e49f master@{1}: merge kernel/master: Fast-forward
>> Tue, 21 May 2013 09:33:24 -0700 (9 days ago), by Felipe
Contreras: remote-hg: fix order of configuration comments
de3a5 master@{2}: merge kernel/master: Fast-forward
>> Fri, 17 May 2013 12:19:20 -0700 (13 days ago), by Junio C
Hamano: Git 1.8.3-rc3
0c2b1 master@{3}: merge kernel/master: Fast-forward
>> Wed, 15 May 2013 14:58:56 -0700 (2 weeks ago), by Junio C
Hamano: Merge branch 'fc/remote-hg' (early part)
b387c master@{4}: merge kernel/master: Fast-forward
>> Thu, 9 May 2013 13:32:54 -0700 (3 weeks ago), by Junio C
Hamano: Sync with v1.8.2.3
9b795 master@{5}: merge kernel/master: Fast-forward
>> Tue, 7 May 2013 22:50:05 -0700 (3 weeks ago), by Junio C
Hamano: Update draft release notes for 1.8.3
4dcdc master@{6}: merge jx/zh_CN: Fast-forward
>> Wed, 8 May 2013 08:13:32 +0800 (3 weeks ago), by Jiang Xin:
l10n: zh_CN.po: translate 44 messages (2080t0f0u)
a09ab0 master@{7}: merge de-ralfth/master: Fast-forward
>> Tue, 7 May 2013 19:28:19 +0200 (3 weeks ago), by Ralf Thielow:
l10n: de.po: translate 44 new messages
674c5 :
>> Wed, 1 May 2013 19:49:18 +0800 (4 weeks ago), by Jiang Xin:
Merge remote-tracking branch 'vi-vnwildman/master'
--
蒋鑫
北京群英汇信息技术有限公司
邮件: worldhello@gmail.com
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 18601196889
--
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] reflog: show committer date in verbose mode
By default, reflog won't show committer date and for some cases won't
show commit log either. It will be helpful to show them all by passing
a more complicated pretty formatter to `git reflog` like this:
$ git reflog show \
--pretty="%Cred%h%Creset %gd: %gs%n >> %Cblue%ci (%cr)%Creset: %s"
It will be nice to add this pretty formatter automatically when run
`git reflog` in verbose mode. Also add new flag "verbose" to rev_info.
Signed-off-by: Jiang Xin
---
builtin/log.c | 31 +++
revision.c| 1 +
revision.h| 1 +
3 files changed, 33 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index dd3f10..fd213 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -615,6 +615,37 @@ int cmd_log_reflog(int argc, const char **argv, const char
*prefix)
rev.use_terminator = 1;
rev.always_show_header = 1;
cmd_log_init_finish(argc, argv, prefix, &rev, &opt);
+ if (rev.verbose && !rev.pretty_given) {
+ struct strbuf formatter = STRBUF_INIT;
+ rev.verbose_header = 1;
+ rev.pretty_given = 1;
+ strbuf_addf(&formatter, "%s%%h%s %%gd: %%gs%%n",
+ diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
+ diff_get_color_opt(&rev.diffopt, DIFF_RESET));
+ switch (rev.verbose) {
+ case 1:
+ strbuf_addf(&formatter, ">> %s%%ci%s (%s%%cr%s)",
+ diff_get_color_opt(&rev.diffopt,
DIFF_METAINFO),
+ diff_get_color_opt(&rev.diffopt,
DIFF_RESET),
+ diff_get_color_opt(&rev.diffopt,
DIFF_METAINFO),
+ diff_get_color_opt(&rev.diffopt,
DIFF_RESET));
+ strbuf_addstr(&formatter, ": %s");
+ break;
+ default:
+ strbuf_addf(&formatter, ">> %s%%cD%s (%s%%cr%s)",
+ diff_get_color_opt(&rev.diffopt,
DIFF_METAINFO),
+ diff_get_color_opt(&rev.diffopt,
DIFF_RESET),
+ diff_get_color_opt(&rev.diffopt,
DIFF_METAINFO),
+ diff_get_color_opt(&rev.diffopt,
DIFF_RESET));
+ strbuf_addf(&formatter, ", by %s%%an%s",
+ diff_get_color_opt(&rev.diffopt,
DIFF_METAINFO),
+ diff_get_color_opt(&rev.diffopt,
DIFF_RESET));
+ strbuf_addstr(&formatter, ": %s");
+ break;
+ }
+ get_commit_format(formatter.buf, &rev);
+ strbuf_release(&formatter);
+ }
return cmd_log_walk(&rev);
}
diff --git a/revision.c b/revision.c
index 518cd..f7483 100644
--- a/revision.c
+++ b/revision.c
@@ -1514,6 +1514,7 @@ static int handle_revision_opt(struct rev_info *revs, int
argc, const char **arg
revs->combine_merges = 1;
} else if (!strcmp(arg, "-v")) {
revs->verbose_header = 1;
+ revs->verbose++;
} else if (!strcmp(arg, "--pretty")) {
revs->verbose_header = 1;
revs->pretty_given = 1;
diff --git a/revision.h b/revision.h
index a313a..032ec 100644
--- a/revision.h
+++ b/revision.h
@@ -119,6 +119,7 @@ struct rev_info {
show_notes_given:1,
show_signature:1,
pretty_given:1,
+ verbose:4,
abbrev_commit:1,
abbrev_commit_given:1,
use_terminator:1,
--
1.8.3.466.g52d8e0f
--
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] git.txt: remove stale comment regarding GIT_WORK_TREE
Official support for specifying --work-tree/GIT_WORK_TREE without --git-dir/GIT_DIR was added with v1.7.4-rc3~2^2~2. Update description of GIT_WORK_TREE to reflect this. Signed-off-by: Chris Rorvick --- Commit ea472c1 made most of the relevant updates. Noticed this while troubleshooting a script that failed when using an old version of Git. Chris Documentation/git.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 68f1ee6..54b87d9 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -682,9 +682,7 @@ Git so take care if using Cogito etc. The '--git-dir' command-line option also sets this value. 'GIT_WORK_TREE':: - Set the path to the working tree. The value will not be - used in combination with repositories found automatically in - a .git directory (i.e. $GIT_DIR is not set). + Set the path to the root of the working tree. This can also be controlled by the '--work-tree' command line option and the core.worktree configuration variable. -- 1.8.3.450.gf3f2a46 -- 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/2] lookup_commit_reference_gently: do not read non-{tag,commit}
On Fri, May 31, 2013 at 4:22 AM, Jeff King wrote: > On Thu, May 30, 2013 at 10:00:23PM +0200, Thomas Rast wrote: > >> lookup_commit_reference_gently unconditionally parses the object given >> to it. This slows down git-describe a lot if you have a repository >> with large tagged blobs in it: parse_object() will read the entire >> blob and verify that its sha1 matches, only to then throw it away. >> >> Speed it up by checking the type with sha1_object_info() prior to >> unpacking. > > This would speed up the case where we do not end up looking at the > object at all, but it will slow down the (presumably common) case where > we will in fact find a commit and end up parsing the object anyway. Perhaps turn "quiet" into a bitmap and only let git-describe do this? -- 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 v2 24/25] register_ref(): make a copy of the bad reference SHA-1
From: "Michael Haggerty" Sent: Thursday, May 30, 2013 10:51 PM On 05/29/2013 06:53 PM, Junio C Hamano wrote: Michael Haggerty writes: [...] + current_bad_sha1 = xmalloc(20); + hashcpy(current_bad_sha1, sha1); This, together with 18/25, may hint that we want hashdup()? I thought about it, but was surprised that I could only find one or two other places in the existing code that would use such a function. But sure, I would be happy to submit a patch. I think hashdup() needn't be inline, so the definition can't go with its cousins in cache.h. There is no cache.c. So where would be the best place to define hashdup()? object.c? sha1_name.c? While I'm at it, I think it would be nice to have constants like #define SHA1_LEN 20 #define SHA1_HEX_LEN 40 and start using those instead of magic numbers. Any objections (or suggestions for better names)? The first named constant should be fully qualified to the same extent as the second, perhaps: #define SHA1_BYTE_LEN 20 and perhaps with an alternate of (though HEX is just as good): #define SHA1_CHAR_LEN 40 Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- Philip -- 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 FIXUP 22/25] fixup! string_list_add_refs_by_glob(): add a comment about memory management
Signed-off-by: Michael Haggerty
---
Junio, would you mind squashing this patch onto mh/reflife 22/25?
notes.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/notes.c b/notes.c
index 602d956..b69c0b8 100644
--- a/notes.c
+++ b/notes.c
@@ -932,6 +932,7 @@ static int string_list_add_one_ref(const char *refname,
const unsigned char *sha
*/
void string_list_add_refs_by_glob(struct string_list *list, const char *glob)
{
+ assert(list->strdup_strings);
if (has_glob_specials(glob)) {
for_each_glob_ref(string_list_add_one_ref, glob, list);
} else {
--
1.8.3
--
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 24/25] register_ref(): make a copy of the bad reference SHA-1
On 05/29/2013 06:53 PM, Junio C Hamano wrote: > Michael Haggerty writes: >> [...] >> +current_bad_sha1 = xmalloc(20); >> +hashcpy(current_bad_sha1, sha1); > > This, together with 18/25, may hint that we want hashdup()? I thought about it, but was surprised that I could only find one or two other places in the existing code that would use such a function. But sure, I would be happy to submit a patch. I think hashdup() needn't be inline, so the definition can't go with its cousins in cache.h. There is no cache.c. So where would be the best place to define hashdup()? object.c? sha1_name.c? While I'm at it, I think it would be nice to have constants like #define SHA1_LEN 20 #define SHA1_HEX_LEN 40 and start using those instead of magic numbers. Any objections (or suggestions for better names)? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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
Re: [PATCH 2/2] lookup_commit_reference_gently: do not read non-{tag,commit}
On Thu, May 30, 2013 at 10:00:23PM +0200, Thomas Rast wrote: > lookup_commit_reference_gently unconditionally parses the object given > to it. This slows down git-describe a lot if you have a repository > with large tagged blobs in it: parse_object() will read the entire > blob and verify that its sha1 matches, only to then throw it away. > > Speed it up by checking the type with sha1_object_info() prior to > unpacking. This would speed up the case where we do not end up looking at the object at all, but it will slow down the (presumably common) case where we will in fact find a commit and end up parsing the object anyway. Have you measured the impact of this on normal operations? During a traversal, we spend a measurable amount of time looking up commits in packfiles, and this would presumably double it. This is not the first time I have seen this tradeoff in git. It would be nice if our object access was structured to do incremental examination of the objects (i.e., store the packfile index lookup or partial unpack of a loose object header, and then use that to complete the next step of actually getting the contents). -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 11/25] object_array_remove_duplicates(): rewrite to reduce copying
On 05/29/2013 06:18 PM, Junio C Hamano wrote:
> Michael Haggerty writes:
>
>> The old version copied one entry to its destination position, then
>> deleted any matching entries from the tail of the array. This
>> required the tail of the array to be copied multiple times. It didn't
>> affect the complexity of the algorithm because the whole tail has to
>> be searched through anyway. But all the copying was unnecessary.
>>
>> Instead, check for the existence of an entry with the same name in the
>> *head* of the list before copying an entry to its final position.
>> This way each entry has to be copied at most one time.
>>
>> Extract a helper function contains_name() to do a bit of the work.
>>
>> Signed-off-by: Michael Haggerty
>> ---
>> object.c | 32 +---
>> object.h | 6 +-
>> 2 files changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/object.c b/object.c
>> index fcd4a82..10b5349 100644
>> --- a/object.c
>> +++ b/object.c
>> @@ -294,22 +294,32 @@ void object_array_filter(struct object_array *array,
>> array->nr = dst;
>> }
>>
>> +/*
>> + * Return true iff array already contains an entry with name.
>> + */
>> +static int contains_name(struct object_array *array, const char *name)
>> +{
>> +unsigned nr = array->nr, i;
>> +struct object_array_entry *object = array->objects;
>> +
>> +for (i = 0; i < nr; i++, object++)
>> +if (!strcmp(object->name, name))
>> +return 1;
>> +return 0;
>> +}
>
> Because some codepaths (e.g. patch 14/25) stuff NULL in the name
> field, we may want to be more careful with this.
>
> This is not a new problem, and I think the longer term solution is
> to get rid of object_array_remove_duplicates(), so it is perfectly
> fine to leave this function broken with respect to NULL input as-is.
You make a good point about NULL names, but I agree to leave it for now
since it needs work anyway.
> The only caller of remove-duplicates is bundle.c, which gets many
> starting points and end points from the command line and tries to be
> nice by removing obvious duplicates, e.g.
>
> git bundle create t.bundle master master
>
> but I think its logic of deduping is wrong. It runs dwim_ref() on
> the incoming refs after the remove-duplicates call, so
>
> git bundle create t.bundle master heads/mater
>
> will end up with two copies of refs/heads/master. To fix it, the
> code must dedup the result of running dwim_ref(), and at that point,
> there is no reason to call object_array_remove_duplicates().
That sounds reasonable.
I poked around this code a bit to understand what is going on, and it
occurred to me that the object_array can include both positive and
negative references, right? And yet object_array_remove_duplicates()
only considers names, not flags. So it seems to me that if the deduping
code would see the same reference twice, once positive and once
negative, then it would throw an arbitrary one of them out, which would
be wrong.
But I couldn't provoke this situation, so perhaps setup_revisions()
already specially treats combinations like "master ^master"? (If that's
true then why? and wouldn't it get confused by "master ^heads/master"?)
I suppose someday I will have to dig into these functions and maybe even
document them.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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 1/2] sha1_file: silence sha1_loose_object_info
sha1_object_info() returns -1 (OBJ_BAD) if it cannot find the object
for some reason, which suggests that it wants the _caller_ to report
this error. However, part of its work happens in
sha1_loose_object_info, which _does_ report errors itself. This is
doubly strange because:
* packed_object_info(), which is the other half of the duo, does _not_
report this.
* In the event that an object is packed and pruned while
sha1_object_info_extended() goes looking for it, we would
erroneously show the error -- even though the code of the latter
function purports to handle this case gracefully.
* A caller might invoke sha1_object_info() to find the type of an
object even if that object is not known to exist.
Silence this error. The others remain untouched as a corrupt object
is a much more grave error than it merely being absent.
Signed-off-by: Thomas Rast
---
sha1_file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index 67e815b..c0f6a0e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2348,7 +2348,7 @@ static int sha1_loose_object_info(const unsigned char
*sha1, unsigned long *size
map = map_sha1_file(sha1, &mapsize);
if (!map)
- return error("unable to find %s", sha1_to_hex(sha1));
+ return -1;
if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
status = error("unable to unpack %s header",
sha1_to_hex(sha1));
--
1.8.3.506.g4fdeee5
--
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 2/2] lookup_commit_reference_gently: do not read non-{tag,commit}
lookup_commit_reference_gently unconditionally parses the object given
to it. This slows down git-describe a lot if you have a repository
with large tagged blobs in it: parse_object() will read the entire
blob and verify that its sha1 matches, only to then throw it away.
Speed it up by checking the type with sha1_object_info() prior to
unpacking.
The reason that deref_tag() does not need the same fix is a bit
subtle: parse_tag_buffer() does not fill the 'tagged' member of the
tag struct if the tagged object is a blob.
Reported-by: Alex Bennée
Signed-off-by: Thomas Rast
---
commit.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/commit.c b/commit.c
index 888e02a..00e8d4a 100644
--- a/commit.c
+++ b/commit.c
@@ -31,8 +31,12 @@ static struct commit *check_commit(struct object *obj,
struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
int quiet)
{
- struct object *obj = deref_tag(parse_object(sha1), NULL, 0);
-
+ struct object *obj;
+ int type = sha1_object_info(sha1, NULL);
+ /* If it's neither tag nor commit, parsing the object is wasted effort
*/
+ if (type != OBJ_TAG && type != OBJ_COMMIT)
+ return NULL;
+ obj = deref_tag(parse_object(sha1), NULL, 0);
if (!obj)
return NULL;
return check_commit(obj, sha1, quiet);
--
1.8.3.506.g4fdeee5
--
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 00/25] Remove assumptions about each_ref_fn arg lifetimes
On 05/29/2013 10:25 AM, Thomas Rast wrote: > Michael Haggerty writes: > I read the entire series on Monday, and give it an Ack at maybe 90% > confidence level -- sorry, I was short on caffeine and sleep ;-) Thanks very much. I'll buy you a coffee the next time I see you :-) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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
Re: Poor performance of git describe in big repos
On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote:
> Alex Bennée writes:
>
> > On 30 May 2013 16:33, Thomas Rast wrote:
> >> Alex Bennée writes:
> >>
> >>> 41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3
> >>> 33.62% git libz.so.1.2.3.4 [.] inflate_fast
> >>> 10.39% git libz.so.1.2.3.4 [.] adler32
> >>> 2.03% git [kernel.kallsyms] [k] clear_page_c
> >>
> >> Do you have any large blobs in the repo that are referenced directly by
> >> a tag?
> >
> > Most probably. I've certainly done a bunch of releases (which are tagged)
> > were
> > the last thing that was updated was an FPGA image.
> [...]
> >> git-describe should probably be fixed to avoid loading blobs, though I'm
> >> not sure off hand if we have any infrastructure to infer the type of a
> >> loose object without inflating it. (This could probably be added by
> >> inflating only the first block.) We do have this for packed objects, so
> >> at least for packed repos there's a speedup to be had.
> >
> > Will it be loading the blob for every commit it traverses or just ones that
> > hit
> > a tag? Why does it need to load the blob at all? Surely the commit
> > tree state doesn't
> > need to be walked down?
>
> No, my theory is that you tagged *the blobs*. Git supports this.
You can see if that is the case by doing something like this:
eval $(git for-each-ref --shell --format '
test $(git cat-file -t %(objectname)^{}) = commit ||
echo %(refname);')
That will print out the name of any ref that doesn't point at a commit.
--
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 22/25] string_list_add_refs_by_glob(): add a comment about memory management
On 05/29/2013 10:21 AM, Thomas Rast wrote:
> Michael Haggerty writes:
>
>> Since string_list_add_one_ref() adds refname to the string list, but
>> the lifetime of refname is limited, it is important that the
>> string_list passed to string_list_add_one_ref() has strdup_strings
>> set. Document this fact.
>>
>> All current callers do the right thing.
> [...]
>> +/*
>> + * The list argument must have strdup_strings set on it.
>> + */
>> void string_list_add_refs_by_glob(struct string_list *list, const char
>> *glob)
>> {
>> if (has_glob_specials(glob)) {
>
> Maybe add an assert() so that this is bulletproof?
Good idea. Will be added in v3.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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
Re: What's cooking in git.git (May 2013, #09; Wed, 29)
Am 30.05.2013 01:58, schrieb Junio C Hamano: > * jk/submodule-subdirectory-ok (2013-04-24) 3 commits > (merged to 'next' on 2013-04-24 at 6306b29) > + submodule: fix quoting in relative_path() > (merged to 'next' on 2013-04-22 at f211e25) > + submodule: drop the top-level requirement > + rev-parse: add --prefix option > > Allow various subcommands of "git submodule" to be run not from the > top of the working tree of the superproject. The summary and status commands are looking good in this version (they are now showing the submodule directory paths relative to the current directory). Apart from that my other remarks from gmane $221575 still seem to apply. And this series has only tests for status, summary and add (and that just with an absolute URL), I'd rather like to see a test for each submodule command (and a relative add to) to document the desired behavior. But I'm not sure if it's better to have another iteration of this series or to address the open issues a follow-up series. Having status, summary and add - at least with absolute URLs - lose the toplevel requirement is already a huge improvement IMO. Opinions? -- 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, #09; Wed, 29)
Am 30.05.2013 01:58, schrieb Junio C Hamano:
> * jl/submodule-mv (2013-04-23) 5 commits
> (merged to 'next' on 2013-04-23 at c04f574)
> + submodule.c: duplicate real_path's return value
> (merged to 'next' on 2013-04-19 at 45ae3c9)
> + rm: delete .gitmodules entry of submodules removed from the work tree
> + Teach mv to update the path entry in .gitmodules for moved submodules
> + Teach mv to move submodules using a gitfile
> + Teach mv to move submodules together with their work trees
>
> "git mv A B" when moving a submodule A does "the right thing",
> inclusing relocating its working tree and adjusting the paths in
> the .gitmodules file.
There are only two issues I'm aware of:
*) When the .gitmodules file is already modified but unchanged
running rm or mv on a submodule will stage those changes too.
*) There is a harmless but unnecessary double invocation of strlen()
in the function (fixed by the diff below).
I plan to fix the first issue in another patch which would also get
rid of the second issue, as exactly that code would have to be touched
anyways.
Does that make sense?
--8<-
diff --git a/submodule.c b/submodule.c
index edfc23c..4670af7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -102,7 +102,7 @@ void stage_updated_gitmodules(void)
struct cache_entry *ce;
int namelen = strlen(".gitmodules");
- pos = cache_name_pos(".gitmodules", strlen(".gitmodules"));
+ pos = cache_name_pos(".gitmodules", namelen);
if (pos < 0) {
warning(_("could not find .gitmodules in index"));
return;
--
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: Poor performance of git describe in big repos
> The culprit, according to some callgrind investigation, is > lookup_commit_reference_gently() [for the unannotated case] or > deref_tag() [annotated case] calling parse_object(). Using the scenario you described earlier, I think it ends-up spending most of its time in check_sha1_signature (both deref_tag and lookup_commit_reference_gently() go there) with 20% inflating, 80% in SHA1_Update(). Not much we can do about that, can we ? -- 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: t0008-ignores failure (was: [msysGit] Git for Windows 1.8.3)
Hi Pat,
On Thu, 30 May 2013, Pat Thoyts wrote:
> On 30 May 2013 16:15, Johannes Schindelin wrote:
>
> > On Thu, 30 May 2013, Karsten Blees wrote:
> >
> >> Am 25.05.2013 21:16, schrieb Pat Thoyts:
> >> > On that note -- with this merge as it now stands I get the following
> >> > test failures:
> >> >
> >> > t0008-ignores.sh 155, 158, 162, 164
> >>
> >> These tests fail because they use absolute paths, e.g.
> >> "C:/.../global-excludes", which is then translated to
> >> "C/.../global-excludes". Can be fixed like so:
> >>
> >> --- 8< ---
> >> --- a/t/t0008-ignores.sh
> >> +++ b/t/t0008-ignores.sh
> >> @@ -5,7 +5,7 @@ test_description=check-ignore
> >> . ./test-lib.sh
> >>
> >> init_vars () {
> >> - global_excludes="$(pwd)/global-excludes"
> >> + global_excludes="global-excludes"
> >> }
> >>
> >> enable_global_excludes () {
> >> ---
> >
> > Since I do not have time for the lengthy, undirected discussion upstream
> > seems to want to start, let's make your change, but only conditional on
> > MINGW?
>
> I was just testing this -- I've already wrapped the suggested fix
> within a "test_have_prereq MINGW" for our fork and committed it. This
> was an issue partly because was alias pwd to "pwd -W" and so always
> get Windows paths. It means the test here doesn't check absolute paths
> but I think we can live with that. I tried using $(builtin pwd) to
> avoid the "-W" but it didn't help and I still got C: style paths.
>
> I also grabbed Karsten's patch "dir.c: fix ignore processing within
> not-ignored directories" as this appears to deal with a .gitignore
> regression in 1.8.3. We can carry this until the next merge with
> upstream.
Thanks!
Dscho
--
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: Should "git help" respect the 'pager' setting?
On Thu, May 30, 2013 at 10:38:59PM +0530, Ramkumar Ramachandra wrote: > Matthieu Moy wrote: > > I find it a bit weird that Git sets the configuration for external > > commands, but it may make sense. No strong opinion here. > > I don't mean a setenv() kind of thing: how would we unset it after > that? Perhaps something like execvpe(), passing in the environment as > an argument? Overriding PAGER might make sense, but I'd be quite annoyed if Git decided to override MANPAGER without providing some way to override it. If a user sets MANPAGER then it's because they want a specific pager when reading man pages - invoking man through "git help" shouldn't cause it to behave differently in this case. -- 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: Should "git help" respect the 'pager' setting?
Matthieu Moy wrote: > I find it a bit weird that Git sets the configuration for external > commands, but it may make sense. No strong opinion here. I don't mean a setenv() kind of thing: how would we unset it after that? Perhaps something like execvpe(), passing in the environment as an argument? -- 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: Should "git help" respect the 'pager' setting?
Ramkumar Ramachandra writes: > It just needs to set $PAGER or $MANPAGER before the exec(), no? Yes, that should do the same as "man -P". > I would argue that it should do this. $GIT_PAGER works everywhere > else, but obviously man has no knowledge about it. I find it a bit weird that Git sets the configuration for external commands, but it may make sense. No strong opinion here. >> If you're an Emacs user, you can read about man.viewer and set it to >> woman, or set PAGER=cat when inside Emacs. > > I just learnt about man.viewer. There's a small problem with it > though: why is there no option for Emacs man corresponding to Emacs > woman? I guess because no one implemented it ;-). >> I personally run M-x git-foo RET, and never run "git help". > > M-x man git-foo RET, you mean? Yes, sorry. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: Should "git help" respect the 'pager' setting?
Matthieu Moy wrote:
> Michael Campbell writes:
>> I have my global git config pager set to 'cat', but when I do a "git
>> help ", it still uses a pager. This is especially irksome in
>> emacs shell buffers, where I am most of the time. I know I can do a
>> M-x man -> git-, but wondered if this was a bug or user
>> error. ("git --no-pager help " does the same.)
>
> "git help foo" just calls "man git-foo" by default, so what happens is
> the same as if you called "man git-foo" by hand. Git does not have
> much control over what man will do, it could probably call "man -P
> $pager" when the Git pager is set, but I'd find it a bit weird.
It just needs to set $PAGER or $MANPAGER before the exec(), no? I
would argue that it should do this. $GIT_PAGER works everywhere else,
but obviously man has no knowledge about it.
> If you're an Emacs user, you can read about man.viewer and set it to
> woman, or set PAGER=cat when inside Emacs.
I just learnt about man.viewer. There's a small problem with it
though: why is there no option for Emacs man corresponding to Emacs
woman?
> I personally run M-x git-foo RET, and never run "git help".
M-x man git-foo RET, you mean? My style is slightly different: I love
typing out 'man git log' on the terminal (dashless); I get it to open
in an Emacs buffer using this hack:
function man_ () {
emacsclient -e "(man \"$*\")" 2>&1 >/dev/null || man "$*"
}
alias man=man_
--
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: Poor performance of git describe in big repos
Thomas Rast writes:
> I had a brief look around sha1_file.c, in particular sha1_object_info,
> and it turns out we lack the "deflate only early part" logic as I
> suspected. So that'll have to be fixed first. After that I *think* it
> should automatically carry over into the tag readers.
Strike that, I'm wrong. sha1_object_info is fast even for these big
loose objects.
The culprit, according to some callgrind investigation, is
lookup_commit_reference_gently() [for the unannotated case] or
deref_tag() [annotated case] calling parse_object().
--
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
Re: Should "git help" respect the 'pager' setting?
Michael Campbell writes:
> I have my global git config pager set to 'cat', but when I do a "git
> help ", it still uses a pager. This is especially irksome in
> emacs shell buffers, where I am most of the time. I know I can do a
> M-x man -> git-, but wondered if this was a bug or user
> error. ("git --no-pager help " does the same.)
"git help foo" just calls "man git-foo" by default, so what happens is
the same as if you called "man git-foo" by hand. Git does not have
much control over what man will do, it could probably call "man -P
$pager" when the Git pager is set, but I'd find it a bit weird.
If you're an Emacs user, you can read about man.viewer and set it to
woman, or set PAGER=cat when inside Emacs.
I personally run M-x git-foo RET, and never run "git help".
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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
preventing evil merges
Hey all, I've be burnt by what someone on IRC referred to as "evil merges", that is loss of history after amending a merge commit: git merge anotherbranch git add something git commit --amend After the steps above the addition of "something" can't be found in the history anymore, but the file is there. Moreover (I think but didn't try to reproruce) a further rebase to a common ancestore of my working branch and "anotherbranch" resulted in further loss of the changes. The only way for me to get them back has been by checking out from reflog. I've no idea about what was going on but this experience reminded me of another one I had in the past in which we could not figure out when some changes were added into a repository (!). If amending a merge is so dangerous, would it make sense to require and hard-to-type switch in order for it to really do anything ? --strk; -- 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
Should "git help" respect the 'pager' setting?
I have my global git config pager set to 'cat', but when I do a "git
help ", it still uses a pager. This is especially irksome in
emacs shell buffers, where I am most of the time. I know I can do a
M-x man -> git-, but wondered if this was a bug or user
error. ("git --no-pager help " does the same.)
12:31 [mcampbell] /tmp % git --no-pager help log
WARNING: terminal is not fully functional
- (press RETURN)
--
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: Poor performance of git describe in big repos
Alex Bennée writes:
> On 30 May 2013 16:33, Thomas Rast wrote:
>> Alex Bennée writes:
>>
>>> 41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3
>>> 33.62% git libz.so.1.2.3.4 [.] inflate_fast
>>> 10.39% git libz.so.1.2.3.4 [.] adler32
>>> 2.03% git [kernel.kallsyms] [k] clear_page_c
>>
>> Do you have any large blobs in the repo that are referenced directly by
>> a tag?
>
> Most probably. I've certainly done a bunch of releases (which are tagged) were
> the last thing that was updated was an FPGA image.
[...]
>> git-describe should probably be fixed to avoid loading blobs, though I'm
>> not sure off hand if we have any infrastructure to infer the type of a
>> loose object without inflating it. (This could probably be added by
>> inflating only the first block.) We do have this for packed objects, so
>> at least for packed repos there's a speedup to be had.
>
> Will it be loading the blob for every commit it traverses or just ones that
> hit
> a tag? Why does it need to load the blob at all? Surely the commit
> tree state doesn't
> need to be walked down?
No, my theory is that you tagged *the blobs*. Git supports this.
git-describe needs to look at the commit (if any) obtained by peeling
each tag (i.e. dereferencing tags until it reaches a non-tag). So to do
that, it resolves the tag's referent and loads it. Usually this will be
a commit, in which case it is marked as reached by the tag.
As my example shows, it also resolves tags' referents if they refer to
non-commits, in particular, it will decompress large blobs that are
(directly) referenced by a tag.
Note that while annotated tags provide the type information themselves,
e.g.
$ git cat-file tag junio-gpg-pub
object fe113d3f96636710600c6b02d5fd421fa7e87dd6
type blob
tag junio-gpg-pub
[...]
unannotated tags are simply refs, so it is not enough to just look at
the tag objects' referent type.
I had a brief look around sha1_file.c, in particular sha1_object_info,
and it turns out we lack the "deflate only early part" logic as I
suspected. So that'll have to be fixed first. After that I *think* it
should automatically carry over into the tag readers.
--
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
Re: Poor performance of git describe in big repos
On 30 May 2013 16:33, Thomas Rast wrote:
> Alex Bennée writes:
>
>> 41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3
>> 33.62% git libz.so.1.2.3.4 [.] inflate_fast
>> 10.39% git libz.so.1.2.3.4 [.] adler32
>> 2.03% git [kernel.kallsyms] [k] clear_page_c
>
> Do you have any large blobs in the repo that are referenced directly by
> a tag?
Most probably. I've certainly done a bunch of releases (which are tagged) were
the last thing that was updated was an FPGA image.
> Because this just so happens to exactly reproduce your symptoms:
>
> # in a random git.git
> $ time git describe --debug
> [...]
> real0m0.390s
> user0m0.037s
> sys 0m0.011s
> $ git tag big1 $(dd if=/dev/urandom bs=1M count=512 | git hash-object -w
> --stdin)
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 45.5088 s, 11.8 MB/s
> $ time git describe --debug
> [...]
> real0m1.875s
> user0m1.738s
> sys 0m0.129s
> $ git tag big2 $(dd if=/dev/urandom bs=1M count=512 | git hash-object -w
> --stdin)
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 44.972 s, 11.9 MB/s
> $ time git describe --debugsuche zur Beschreibung von HEAD
> [...]
> real0m3.620s
> user0m3.357s
> sys 0m0.248s
>
> (I actually ran the git-describe invocations more than once to ensure
> that they are again cache-hot.)
That looks pretty promising as a replication.
> git-describe should probably be fixed to avoid loading blobs, though I'm
> not sure off hand if we have any infrastructure to infer the type of a
> loose object without inflating it. (This could probably be added by
> inflating only the first block.) We do have this for packed objects, so
> at least for packed repos there's a speedup to be had.
Will it be loading the blob for every commit it traverses or just ones that hit
a tag? Why does it need to load the blob at all? Surely the commit
tree state doesn't
need to be walked down?
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
--
Alex, homepage: http://www.bennee.com/~alex/
--
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-gui: fix file name handling with non-empty prefix
In the hope that the Pat Thoyts who just posted in another thread from a
GMail address is the same one that maintains git-gui, let's see if that
address works...
On Sat, May 11, 2013 at 10:03:25PM -0400, Andrew Wong wrote:
> Sorry for the late reply. I was able to reproduce the problem that you
> were describing a while ago. And your patch indeed fixes it. It's a much
> more elegant way of dealing with the "absolute vs relative" path problem
> that I was trying to fix.
>
> Thanks!
>
> As for Pat, I'm not sure wha'ts going on with his email address. It was
> working back in October, and his username still seems to be active over
> at SourceForge... let's see if this email reaches him.
>
> Here's a link for his reference just in case he missed your original email:
> http://thread.gmane.org/gmane.comp.version-control.git/222646
>
>
> On 04/27/13 10:18, John Keeping wrote:
> > I got a bounce with "550 no such user" for Pat's email address when
> > sending this. Does anyone have more up-to-date contact details? Or is
> > it just SourceForge being broken?
> >
> > On Sat, Apr 27, 2013 at 02:24:16PM +0100, John Keeping wrote:
> >> Commit e3d06ca (git-gui: Detect full path when parsing arguments -
> >> 2012-10-02) fixed the handling of absolute paths passed to the browser
> >> and blame subcommands by checking whether the file exists without the
> >> prefix before prepending the prefix and checking again. Since we have
> >> chdir'd to the top level of the working tree before doing this, this
> >> does not work if a file with the same name exists in a subdirectory and
> >> at the top level (for example Makefile in git.git's t/ directory).
> >>
> >> Instead of doing this, revert that patch and fix absolute path issue by
> >> using "file join" to prepend the prefix to the supplied path. This will
> >> correctly handle absolute paths by skipping the prefix in that case.
> >>
> >> Signed-off-by: John Keeping
> >> ---
> >> git-gui.sh | 14 +++---
> >> 1 file changed, 3 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/git-gui.sh b/git-gui.sh
> >> index e11..a94ad7f 100755
> >> --- a/git-gui.sh
> >> +++ b/git-gui.sh
> >> @@ -3003,19 +3003,11 @@ blame {
> >>set jump_spec {}
> >>set is_path 0
> >>foreach a $argv {
> >> - if {[file exists $a]} {
> >> - if {$path ne {}} usage
> >> - set path [normalize_relpath $a]
> >> - break
> >> - } elseif {[file exists $_prefix$a]} {
> >> - if {$path ne {}} usage
> >> - set path [normalize_relpath $_prefix$a]
> >> - break
> >> - }
> >> + set p [file join $_prefix $a]
> >>
> >> - if {$is_path} {
> >> + if {$is_path || [file exists $p]} {
> >>if {$path ne {}} usage
> >> - set path [normalize_relpath $_prefix$a]
> >> + set path [normalize_relpath $p]
> >>break
> >>} elseif {$a eq {--}} {
> >>if {$path ne {}} {
> >> --
> >> 1.8.3.rc0.149.g98a72f2.dirty
>
> --
> 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: t0008-ignores failure (was: [msysGit] Git for Windows 1.8.3)
On 30 May 2013 16:15, Johannes Schindelin wrote:
> Hi Karsten,
>
> On Thu, 30 May 2013, Karsten Blees wrote:
>
>> Am 25.05.2013 21:16, schrieb Pat Thoyts:
>> > On that note -- with this merge as it now stands I get the following
>> > test failures:
>> >
>> > t0008-ignores.sh 155, 158, 162, 164
>>
>> These tests fail because they use absolute paths, e.g.
>> "C:/.../global-excludes", which is then translated to
>> "C/.../global-excludes". Can be fixed like so:
>>
>> --- 8< ---
>> --- a/t/t0008-ignores.sh
>> +++ b/t/t0008-ignores.sh
>> @@ -5,7 +5,7 @@ test_description=check-ignore
>> . ./test-lib.sh
>>
>> init_vars () {
>> - global_excludes="$(pwd)/global-excludes"
>> + global_excludes="global-excludes"
>> }
>>
>> enable_global_excludes () {
>> ---
>
> Since I do not have time for the lengthy, undirected discussion upstream
> seems to want to start, let's make your change, but only conditional on
> MINGW?
>
> Ciao,
> Dscho
I was just testing this -- I've already wrapped the suggested fix
within a "test_have_prereq MINGW" for our fork and committed it. This
was an issue partly because was alias pwd to "pwd -W" and so always
get Windows paths. It means the test here doesn't check absolute paths
but I think we can live with that. I tried using $(builtin pwd) to
avoid the "-W" but it didn't help and I still got C: style paths.
I also grabbed Karsten's patch "dir.c: fix ignore processing within
not-ignored directories" as this appears to deal with a .gitignore
regression in 1.8.3. We can carry this until the next merge with
upstream.
--
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: Poor performance of git describe in big repos
Alex Bennée writes:
> 41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3
> 33.62% git libz.so.1.2.3.4 [.] inflate_fast
> 10.39% git libz.so.1.2.3.4 [.] adler32
> 2.03% git [kernel.kallsyms] [k] clear_page_c
Do you have any large blobs in the repo that are referenced directly by
a tag?
Because this just so happens to exactly reproduce your symptoms:
# in a random git.git
$ time git describe --debug
[...]
real0m0.390s
user0m0.037s
sys 0m0.011s
$ git tag big1 $(dd if=/dev/urandom bs=1M count=512 | git hash-object -w
--stdin)
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 45.5088 s, 11.8 MB/s
$ time git describe --debug
[...]
real0m1.875s
user0m1.738s
sys 0m0.129s
$ git tag big2 $(dd if=/dev/urandom bs=1M count=512 | git hash-object -w
--stdin)
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 44.972 s, 11.9 MB/s
$ time git describe --debugsuche zur Beschreibung von HEAD
[...]
real0m3.620s
user0m3.357s
sys 0m0.248s
(I actually ran the git-describe invocations more than once to ensure
that they are again cache-hot.)
git-describe should probably be fixed to avoid loading blobs, though I'm
not sure off hand if we have any infrastructure to infer the type of a
loose object without inflating it. (This could probably be added by
inflating only the first block.) We do have this for packed objects, so
at least for packed repos there's a speedup to be had.
--
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
Re: [git-users] Highlevel (but simple to implement) commands provided by default for git
On Thu, May 30, 2013 at 9:54 AM, Jonathan Nieder wrote: > Felipe Contreras wrote: >> On Thu, May 30, 2013 at 12:23 AM, Jonathan Nieder wrote: >>> Felipe Contreras wrote: On Wed, May 29, 2013 at 6:43 PM, Jonathan Nieder wrote: > > A bigger problem (in my opinion) with allowing arbitrary changes to > the meaning of existing commands is that scripts, whether placed in > .sh files or given as commands to run over IRC, stop working > altogether. It's nice to have commands like "git log" and "git am" > mean the same thing no matter what machine I am on. Except that's not true: >>> >>> It's not true that my opinion is that a bigger problem than the >>> non-problem Ram mentioned with allowing arbitrary changes to the >>> meaning of existing commands is that scripts stop working reliably? >> >> It's not true what you said: >> >> commands like "git log" and "git am" mean the same thing no matter >> what machine I am on. > > It's not true that it's nice when they do? Yeah, it's nice that the sun is purple. Never-mind the fact that it's not true. The consistency you experience across machines has absolutely nothing to do with Git, since Git can be configured in a way you don't consider nice. So this argument is invalid. Any proposed change to make Git more configurable is not affected by this argument, because Git can *already* be configured in a way that would break your experience, yet it doesn't happen. In other words; it's the policy or your machine users you have to thank for, not Git's code, and changing Git's code is not going to change that policy. Either way this is a straw man, again, nobody is pushing to allow builtins to be overridable. The topic is default *aliases*. -- 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 7/7] unpack-trees: free cache_entry array members for merges
On Thu, May 30, 2013 at 9:40 AM, René Scharfe
wrote:
> Am 30.05.2013 14:04, schrieb Felipe Contreras:
>
>> On Thu, May 30, 2013 at 6:34 AM, René Scharfe
>> wrote:
>>>
>>> The merge functions duplicate entries as needed and they don't free
>>> them. Release them in unpack_nondirectories, the same function
>>> where they were allocated, after we're done.
>>
>>
>> Ah, you beat me to this change, but..
>>
>>> @@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned
>>> long mask,
>>> src[i + o->merge] = create_ce_entry(info, names + i,
>>> stage);
>>> }
>>>
>>> - if (o->merge)
>>> - return call_unpack_fn((const struct cache_entry * const
>>> *)src,
>>> - o);
>>> + if (o->merge) {
>>> + int rc = call_unpack_fn((const struct cache_entry * const
>>> *)src,
>>> + o);
>>> + for (i = 1; i <= n; i++)
>>> + if (src[i] && src[i] != o->df_conflict_entry)
>>> + free(src[i]);
>>
>>
>> Doesn't it make more sense to follow the code above and do src[i +
>> o->merge]?
>
>
> Not sure I understand. Is the goal to avoid confusion for code readers by
> using the same indexing method for allocation and release? Or are you
> worried about o->merge having a different value than 1 in that loop?
Both. In particular I'm eyeballing the code you can even see in this patch:
src[i + o->merge] = create_ce_entry(info, names + i, stage);
If you think it's better to use src[i], then I think the code above
should do the same.
--
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: Poor performance of git describe in big repos
Alex Bennée wrote: > 15:50 ajb@sloy/x86_64 [work.git] >time git log --pretty=oneline | wc -l > 24648 > > real0m0.434s > user0m0.388s > sys 0m0.112s > > Although it doesn't take too long to walk the whole mainline history > (obviously ignoring all the other branches). Damn, non-starter. linux.git has 361k+ commits in mainline history. Nit: use git rev-list --count HEAD next time. > 15:52 ajb@sloy/x86_64 [work.git] >git count-objects -v -H > count: 581 > size: 5.09 MiB > in-pack: 399307 > packs: 1 > size-pack: 1.49 GiB > prune-packable: 0 > garbage: 0 > size-garbage: 0 bytes linux.git has 2.9m+ in-pack. The pack-size is much lower at about 800+ MiB, but I don't think 1.49 GiB is a problem in itself. Looking forward to your big-files report to see why it's so big. > It is a pick repo. The gc --aggressive nearly took out my machine keeping > around 4gb resident for most of the half hour and using nearly 8gb of VM. > > Of course most of the history is not needed for day to day stuff. Maybe > if I split the pack files up it wouldn't be quite such a strain to work > through them? Really out of my depth here, sorry. Let's see what Duy (or the others) have to say. >> 2. You have have huge (binary) files checked into your repository. Do >> you? If so, why isn't the code in streaming.c kicking in? > > We do have some binary blobs in the repository (mainly DSP and FPGA images) > although not a huge number: > > 15:58 ajb@sloy/x86_64 [work.git] >time git log --pretty=oneline -- xxx > xxx/xx/*.out ./xxx/xxx/*.out ./xxx/xxx/*.out | wc -l > 234 > > real0m0.590s > user0m0.552s > sys 0m0.040s log is streaming, and is not a good measure: it doesn't even walk the entire commit graph. How big are these files? > How can I tell if streaming is kicking in or now? I use callgrind (and kcachegrind to visualize). Can you post callgrind output? It will be helpful in figuring out where exactly git is spending time. -- 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: t0008-ignores failure (was: [msysGit] Git for Windows 1.8.3)
Hi Karsten,
On Thu, 30 May 2013, Karsten Blees wrote:
> Am 25.05.2013 21:16, schrieb Pat Thoyts:
> > On that note -- with this merge as it now stands I get the following
> > test failures:
> >
> > t0008-ignores.sh 155, 158, 162, 164
>
> These tests fail because they use absolute paths, e.g.
> "C:/.../global-excludes", which is then translated to
> "C/.../global-excludes". Can be fixed like so:
>
> --- 8< ---
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -5,7 +5,7 @@ test_description=check-ignore
> . ./test-lib.sh
>
> init_vars () {
> - global_excludes="$(pwd)/global-excludes"
> + global_excludes="global-excludes"
> }
>
> enable_global_excludes () {
> ---
Since I do not have time for the lengthy, undirected discussion upstream
seems to want to start, let's make your change, but only conditional on
MINGW?
Ciao,
Dscho
--
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 1/3] read-cache: plug a few leaks
Am 30.05.2013 15:34, schrieb Felipe Contreras: > We don't free 'istate->cache' properly. > > Apparently 'initialized' doesn't really mean initialized, but loaded, or > rather 'not-empty', and the cache can be used even if it's not > 'initialized', so we can't rely on this variable to keep track of the > 'istate->cache'. > > So assume it always has data, and free it before overwriting it. > > Signed-off-by: Felipe Contreras > --- > read-cache.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/read-cache.c b/read-cache.c > index 04ed561..e5dc96f 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -1449,6 +1449,7 @@ int read_index_from(struct index_state *istate, const > char *path) > istate->version = ntohl(hdr->hdr_version); > istate->cache_nr = ntohl(hdr->hdr_entries); > istate->cache_alloc = alloc_nr(istate->cache_nr); > + free(istate->cache); With that change, callers of read_index_from need to set ->cache to NULL for uninitialized (on-stack) index_state variables. They only had to set ->initialized to 0 before in that situation. It this chunk safe for all existing callers? Shouldn't the same free in discard_index (added below) be enough? > istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry > *)); > istate->initialized = 1; > > @@ -1510,6 +1511,9 @@ int discard_index(struct index_state *istate) > > for (i = 0; i < istate->cache_nr; i++) > free(istate->cache[i]); > + free(istate->cache); > + istate->cache = NULL; > + istate->cache_alloc = 0; > resolve_undo_clear_index(istate); > istate->cache_nr = 0; > istate->cache_changed = 0; I was preparing a similar change, looks good. There is a comment at the end of discard_index() that becomes wrong due to that patch, though -- better remove it as well. It was already outdated as it mentioned active_cache, while the function can be used with any index_state. diff --git a/read-cache.c b/read-cache.c index e5dc96f..0f868af 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1522,8 +1522,6 @@ int discard_index(struct index_state *istate) free_name_hash(istate); cache_tree_free(&(istate->cache_tree)); istate->initialized = 0; - - /* no need to throw away allocated active_cache */ return 0; } -- 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: Poor performance of git describe in big repos
On 30 May 2013 15:32, Ramkumar Ramachandra wrote: > Alex Bennée wrote: >> And through my "special" repo: >> >> 41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3 >> 33.62% git libz.so.1.2.3.4 [.] inflate_fast >> 10.39% git libz.so.1.2.3.4 [.] adler32 >> 2.03% git [kernel.kallsyms] [k] clear_page_c >> >> I'm not sure why libcrypto features so highly in the results > > While Duy churns on the delta chain, let me try to make a (rather > crude) observation: > > What does it mean for libcrypto to be so high in your perf report? > sha1_block_data_order is ultimately by object.c:parse_object. While > it indicates that deltas are taking a long time to apply (or are > somehow not optimally organized for IO), I think it indicates either: > > 1. Your history is very deep and there are an unusually high number of > deltas for each blob. What are the total number of commits? Well the history does en-compose about 10 years of product development and has a high number of files in the repo (including about 3 copies of the kernel - sans upstream history). 15:50 ajb@sloy/x86_64 [work.git] >time git log --pretty=oneline | wc -l 24648 real0m0.434s user0m0.388s sys 0m0.112s Although it doesn't take too long to walk the whole mainline history (obviously ignoring all the other branches). 15:52 ajb@sloy/x86_64 [work.git] >git count-objects -v -H count: 581 size: 5.09 MiB in-pack: 399307 packs: 1 size-pack: 1.49 GiB prune-packable: 0 garbage: 0 size-garbage: 0 bytes It is a pick repo. The gc --aggressive nearly took out my machine keeping around 4gb resident for most of the half hour and using nearly 8gb of VM. Of course most of the history is not needed for day to day stuff. Maybe if I split the pack files up it wouldn't be quite such a strain to work through them? > 2. You have have huge (binary) files checked into your repository. Do > you? If so, why isn't the code in streaming.c kicking in? We do have some binary blobs in the repository (mainly DSP and FPGA images) although not a huge number: 15:58 ajb@sloy/x86_64 [work.git] >time git log --pretty=oneline -- xxx xxx/xx/*.out ./xxx/xxx/*.out ./xxx/xxx/*.out | wc -l 234 real0m0.590s user0m0.552s sys 0m0.040s How can I tell if streaming is kicking in or now? -- Alex, homepage: http://www.bennee.com/~alex/ -- 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 2/7] add tests for rebasing with patch-equivalence present
On Thu, May 30, 2013 at 5:54 AM, Johannes Sixt wrote:
> Am 30.05.2013 07:30, schrieb Martin von Zweigbergk:
>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt wrote:
>>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
+# f
+# /
+# a---b---c---g---h
+# \
+# d---G---i
>>> ...
+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..
>>>
>>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>>> it does not contain G. Hence, G should be replayed. Since h is the
>>> reversal of g, the state at h is the same as at c, and applying G should
>>> succeed (it is the same change as g). Therefore, I think the correct
>>> expectation is:
>>>
>>> test_linear_range 'd G i' h..
>>
>> Good question! It is really not obvious what the right answer is. Some
>> arguments in favor of dropping 'G':
>>
>> 1. Let's say origin/master points to 'b' when you start the 'd G i'
>> branch. You then send the 'G' patch to Junio who applies it as 'g'
>> (cherry-picking direction is reversed compared to figure, but same
>> effect). You then "git pull --rebase" when master on origin points to
>> 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
>> rebase --onto h b i'.
>
> The reason for this git pull cleverness is to be prepared for rewritten
> history:
>
>b'--c'--g'--h'
> /
> a---b
> \
>d---G---i
>
> to avoid that b is rebased.
Right. It doesn't currently drop 'G' and maybe it shouldn't, so let's
leave it as is for now, I would say.
>> 2. In the test a little before the above one, we instead do 'git
>> rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
>> case it doesn't matter what's in $branch..$upstream. Do we agree that
>> $branch..$upstream should never matter (instead, $upstream is only
>> used to find merge base with $branch)?
>
> No, we do not agree. $branch..$upstream should be the set of patches
> that should be omitted. $branch..$onto should not matter. $onto is only
> used to specify the destination of the rebased commits.
Ok. As I said to Felipe, I'm not sure why I was so convinced that it's
bad to lose the patch in 'git rebase --onto f h i'. It can result in
lost work, but it seems rare enough that no one has reported it,
AFAIK.
I'll change those tests in a re-roll, and perhaps I'll drop a few of
them. Let me know if you (anyone) disagree.
Martin
PS. Thanks for a meticulous review!
--
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-users] Highlevel (but simple to implement) commands provided by default for git
Felipe Contreras wrote: > On Thu, May 30, 2013 at 12:23 AM, Jonathan Nieder wrote: >> Felipe Contreras wrote: >>> On Wed, May 29, 2013 at 6:43 PM, Jonathan Nieder wrote: A bigger problem (in my opinion) with allowing arbitrary changes to the meaning of existing commands is that scripts, whether placed in .sh files or given as commands to run over IRC, stop working altogether. It's nice to have commands like "git log" and "git am" mean the same thing no matter what machine I am on. >>> >>> Except that's not true: >> >> It's not true that my opinion is that a bigger problem than the >> non-problem Ram mentioned with allowing arbitrary changes to the >> meaning of existing commands is that scripts stop working reliably? > > It's not true what you said: > > commands like "git log" and "git am" mean the same thing no matter > what machine I am on. It's not true that it's nice when they do? >> This combative style of communication is toxic. It kills the chance >> of a calm, pleasant discussion, even with patient people who don't >> even fundamentally disagree. Please stop it. > > Stop assuming bad faith[1]. Perhaps you mean "I'm trying, but I'm human --- please bear with me while I work on improving. Know that my intentions are good." Unfortunately, good intentions are not enough. Communicating in a way that clearly conveys what you mean instead of something else that derails the conversation is a real skill and, as I said, it's an important one that is basic to being able to have a productive conversation. If you are working on it and are not there yet, my best advice would be to lurk more and speak less, to learn from the example of others, and to start by working on how to receive criticism and to clear up misunderstandings, which can at least mitigate the damage when things go wrong. You have accused others of assuming bad faith in the past, which only escalates things. It's not a good way to move forward. It's possible that the best way to move forward involves some way (e.g., mail client configuration that holds messages in "drafts" for a little while before sending them out) of being able to cool down when discussions get hot. Sincerely, 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 v2 3/3] unpack-trees: free created cache entries
Am 30.05.2013 15:34, schrieb Felipe Contreras:
We created them, and nobody else is going to destroy them.
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index eff2944..9f19d01 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -590,8 +590,16 @@ static int unpack_nondirectories(int n, unsigned long mask,
src[i + o->merge] = create_ce_entry(info, names + i, stage);
}
- if (o->merge)
- return call_unpack_fn(src, o);
+ if (o->merge) {
+ int ret = call_unpack_fn(src, o);
+ for (i = 0; i < n; i++) {
+ struct cache_entry *ce = src[i + o->merge];
+ if (!ce || ce == o->df_conflict_entry)
+ continue;
+ free(ce);
+ }
+ return ret;
+ }
Ah, now I understand what you meant in that other email. That works as
well, of course. It's slightly nicer on the eye, admittedly.
René
--
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 7/7] unpack-trees: free cache_entry array members for merges
Am 30.05.2013 14:04, schrieb Felipe Contreras:
On Thu, May 30, 2013 at 6:34 AM, René Scharfe
wrote:
The merge functions duplicate entries as needed and they don't free
them. Release them in unpack_nondirectories, the same function
where they were allocated, after we're done.
Ah, you beat me to this change, but..
@@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned long mask,
src[i + o->merge] = create_ce_entry(info, names + i, stage);
}
- if (o->merge)
- return call_unpack_fn((const struct cache_entry * const *)src,
- o);
+ if (o->merge) {
+ int rc = call_unpack_fn((const struct cache_entry * const *)src,
+ o);
+ for (i = 1; i <= n; i++)
+ if (src[i] && src[i] != o->df_conflict_entry)
+ free(src[i]);
Doesn't it make more sense to follow the code above and do src[i + o->merge]?
Not sure I understand. Is the goal to avoid confusion for code readers
by using the same indexing method for allocation and release? Or are
you worried about o->merge having a different value than 1 in that loop?
We'd have to add 1 (== o->merge) to each index variable usage with a
zero-based loop. A one-based loop avoids that, and while it's not
pretty it's also not too complicated, I think.
René
--
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] test: fix post rewrite hook report
Felipe Contreras writes:
> First expected, then actual.
Ack. That is the prevalent (almost universal, but not quite) style.
> Signed-off-by: Felipe Contreras
> ---
> t/t5407-post-rewrite-hook.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
> index baa670c..ea2e0d4 100755
> --- a/t/t5407-post-rewrite-hook.sh
> +++ b/t/t5407-post-rewrite-hook.sh
> @@ -31,8 +31,8 @@ clear_hook_input () {
> }
>
> verify_hook_input () {
> - test_cmp "$TRASH_DIRECTORY"/post-rewrite.args expected.args &&
> - test_cmp "$TRASH_DIRECTORY"/post-rewrite.data expected.data
> + test_cmp expected.args "$TRASH_DIRECTORY"/post-rewrite.args &&
> + test_cmp expected.data "$TRASH_DIRECTORY"/post-rewrite.data
> }
>
> test_expect_success 'git commit --amend' '
--
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
Re: Poor performance of git describe in big repos
Alex Bennée wrote: > And through my "special" repo: > > 41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3 > 33.62% git libz.so.1.2.3.4 [.] inflate_fast > 10.39% git libz.so.1.2.3.4 [.] adler32 > 2.03% git [kernel.kallsyms] [k] clear_page_c > > I'm not sure why libcrypto features so highly in the results While Duy churns on the delta chain, let me try to make a (rather crude) observation: What does it mean for libcrypto to be so high in your perf report? sha1_block_data_order is ultimately by object.c:parse_object. While it indicates that deltas are taking a long time to apply (or are somehow not optimally organized for IO), I think it indicates either: 1. Your history is very deep and there are an unusually high number of deltas for each blob. What are the total number of commits? 2. You have have huge (binary) files checked into your repository. Do you? If so, why isn't the code in streaming.c kicking in? -- 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: Poor performance of git describe in big repos
On 30 May 2013 14:45, Duy Nguyen wrote: > On Thu, May 30, 2013 at 8:34 PM, Alex Bennée wrote: > > Thanks. Can you share "verify-pack -v" output of > pack-a9ba133a6f25ffa74c3c407e09ab030f8745b201.pack? I think you need > to put it somewhere on Internet temporarily as it's likely to exceed > git@vger limits. > -- > Duy http://www.bennee.com/~alex/stuff/git-pack-access.tar.bz2 -- Alex, homepage: http://www.bennee.com/~alex/ -- 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 4/4] sha1_file: trivial style cleanup
Signed-off-by: Felipe Contreras
---
sha1_file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index 67e815b..b114cc9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2138,7 +2138,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
if (!data)
die("failed to apply delta");
- free (delta_data);
+ free(delta_data);
}
*final_type = type;
--
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 3/4] unpack-trees: trivial cleanup
dfc has not been initialized at this point.
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index ede4299..36f4ff7 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1040,8 +1040,7 @@ int unpack_trees(unsigned len, struct tree_desc *t,
struct unpack_trees_options
if (!o->skip_sparse_checkout)
mark_new_skip_worktree(o->el, o->src_index, 0,
CE_NEW_SKIP_WORKTREE);
- if (!dfc)
- dfc = xcalloc(1, cache_entry_size(0));
+ dfc = xcalloc(1, cache_entry_size(0));
o->df_conflict_entry = dfc;
if (len) {
--
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 2/4] read-cache: trivial style cleanups
Signed-off-by: Felipe Contreras
---
read-cache.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 5253ec5..7040e79 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -979,7 +979,7 @@ int add_index_entry(struct index_state *istate, struct
cache_entry *ce, int opti
if (istate->cache_nr == istate->cache_alloc) {
istate->cache_alloc = alloc_nr(istate->cache_alloc);
istate->cache = xrealloc(istate->cache,
- istate->cache_alloc * sizeof(struct
cache_entry *));
+ istate->cache_alloc *
sizeof(*istate->cache));
}
/* Add it in.. */
@@ -1449,7 +1449,7 @@ int read_index_from(struct index_state *istate, const
char *path)
istate->version = ntohl(hdr->hdr_version);
istate->cache_nr = ntohl(hdr->hdr_entries);
istate->cache_alloc = alloc_nr(istate->cache_nr);
- istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry
*));
+ istate->cache = xcalloc(istate->cache_alloc, sizeof(*istate->cache));
istate->initialized = 1;
if (istate->version == 4)
--
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 1/4] read-cache: fix wrong 'the_index' usage
We are dealing with the 'istate' index, not 'the_index'.
Signed-off-by: Felipe Contreras
---
read-cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index 04ed561..5253ec5 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -626,7 +626,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
if (*ptr == '/') {
struct cache_entry *foundce;
++ptr;
- foundce = index_name_exists(&the_index,
ce->name, ptr - ce->name, ignore_case);
+ foundce = index_name_exists(istate, ce->name,
ptr - ce->name, ignore_case);
if (foundce) {
memcpy((void *)startPtr, foundce->name
+ (startPtr - ce->name), ptr - startPtr);
startPtr = ptr;
--
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 0/4] Trivial patches
Hi, Here's a bunch of trivial patches, mostly syle, but the first one might be important. Felipe Contreras (4): read-cache: fix wrong 'the_index' usage read-cache: trivial style cleanups unpack-trees: trivial cleanup sha1_file: trivial style cleanup read-cache.c | 6 +++--- sha1_file.c| 2 +- unpack-trees.c | 3 +-- 3 files changed, 5 insertions(+), 6 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] test: fix post rewrite hook report
First expected, then actual.
Signed-off-by: Felipe Contreras
---
t/t5407-post-rewrite-hook.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index baa670c..ea2e0d4 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -31,8 +31,8 @@ clear_hook_input () {
}
verify_hook_input () {
- test_cmp "$TRASH_DIRECTORY"/post-rewrite.args expected.args &&
- test_cmp "$TRASH_DIRECTORY"/post-rewrite.data expected.data
+ test_cmp expected.args "$TRASH_DIRECTORY"/post-rewrite.args &&
+ test_cmp expected.data "$TRASH_DIRECTORY"/post-rewrite.data
}
test_expect_success 'git commit --amend' '
--
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] cherry-pick: don't barf when there's nothing to do
If the user set --ff, it's expected that if theres's nothing to do we
fast-forward our current HEAD, which is a no-op.
Signed-off-by: Felipe Contreras
---
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index d8f9d30..b9d4b48 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -749,7 +749,7 @@ static void prepare_revs(struct replay_opts *opts)
if (prepare_revision_walk(opts->revs))
die(_("revision walk setup failed"));
- if (!opts->revs->commits)
+ if (!opts->revs->commits && !opts->allow_ff)
die(_("empty commit set passed"));
}
--
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 2/2] git.txt: document GIT_TRACE_PACKET
"This can help with debugging object negotiation or other protocol issues." Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 3e74440..12ef7a2 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -839,6 +839,11 @@ for further details. recorded. This may be helpful for troubleshooting some pack-related performance problems. +'GIT_TRACE_PACKET':: + If this variable is set, it shows a trace of all packets + coming in or out of a given program. This can help with + debugging object negotiation or other protocol issues. + GIT_LITERAL_PATHSPECS:: Setting this variable to `1` will cause Git to treat all pathspecs literally, rather than as glob patterns. For example, -- 1.8.2.83.gc99314b -- 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 1/2] core: use env variable instead of config var to turn on logging pack access
5f44324 (core: log offset pack data accesses happened - 2011-07-06)
provides a way to observe pack access patterns via a config
switch. Setting an environment variable looks more obvious than a
config var, especially when you just need to _observe_, and more
inline with other tracing knobs we have.
Document it as it may be useful for remote troubleshooting.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git.txt | 7 +++
cache.h | 3 ---
config.c | 3 ---
sha1_file.c | 10 ++
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 9e302b0..3e74440 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -832,6 +832,13 @@ for further details.
as a file path and will try to write the trace messages
into it.
+'GIT_TRACE_PACK_ACCESS'::
+ If this variable is set to a path, a file will be created at
+ the given path logging all accesses to any packs. For each
+ access, the pack file name and an offset in the pack is
+ recorded. This may be helpful for troubleshooting some
+ pack-related performance problems.
+
GIT_LITERAL_PATHSPECS::
Setting this variable to `1` will cause Git to treat all
pathspecs literally, rather than as glob patterns. For example,
diff --git a/cache.h b/cache.h
index 94ca1ac..9bfd76b 100644
--- a/cache.h
+++ b/cache.h
@@ -772,9 +772,6 @@ extern int parse_sha1_header(const char *hdr, unsigned long
*sizep);
/* global flag to enable extra checks when accessing packed objects */
extern int do_check_packed_object_crc;
-/* for development: log offset of pack access */
-extern const char *log_pack_access;
-
extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned
long size, const char *type);
extern int move_temp_to_file(const char *tmpfile, const char *filename);
diff --git a/config.c b/config.c
index aefd80b..ce074d7 100644
--- a/config.c
+++ b/config.c
@@ -675,9 +675,6 @@ static int git_default_core_config(const char *var, const
char *value)
return 0;
}
- if (!strcmp(var, "core.logpackaccess"))
- return git_config_string(&log_pack_access, var, value);
-
if (!strcmp(var, "core.autocrlf")) {
if (value && !strcasecmp(value, "input")) {
if (core_eol == EOL_CRLF)
diff --git a/sha1_file.c b/sha1_file.c
index 67e815b..7b47bdc 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -36,6 +36,8 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
const unsigned char null_sha1[20];
+static const char *log_pack_access = "";
+
/*
* This is meant to hold a *small* number of objects that you would
* want read_sha1_file() to be able to return, but yet you do not want
@@ -1956,6 +1958,14 @@ static void write_pack_access_log(struct packed_git *p,
off_t obj_offset)
{
static FILE *log_file;
+ if (!*log_pack_access) {
+ log_pack_access = getenv("GIT_TRACE_PACK_ACCESS");
+ if (!*log_pack_access)
+ log_pack_access = NULL;
+ if (!log_pack_access)
+ return;
+ }
+
if (!log_file) {
log_file = fopen(log_pack_access, "w");
if (!log_file) {
--
1.8.2.83.gc99314b
--
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: Poor performance of git describe in big repos
On Thu, May 30, 2013 at 8:34 PM, Alex Bennée wrote: > From the following run: > > > 14:31 ajb@sloy/x86_64 [work.git] >time /usr/bin/git --no-pager > describe --long --tags > ajb-build-test-5224-11-g9660048 > > real0m14.720s > user0m12.985s > sys 0m1.700s > 14:31 ajb@sloy/x86_64 [work.git] >wc -l /tmp/log-pack.txt > 1610 /tmp/log-pack.txt > > The pack has been "tuned" with a gc --aggressive. Assuming the numbers > are offsets into the pack it looks fairly random access until the last > 100 or so. > > [snipped] Thanks. Can you share "verify-pack -v" output of pack-a9ba133a6f25ffa74c3c407e09ab030f8745b201.pack? I think you need to put it somewhere on Internet temporarily as it's likely to exceed git@vger limits. -- 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 v2 2/3] unpack-trees: plug a memory leak
On 05/30/2013 03:34 PM, Felipe Contreras wrote: > Before overwriting the destination index, first let's discard it's > s/it's/its/ > contents. > > [SNIP] Regards, Stefano -- 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/3] cherry-pick: fix memory leaks
Hi, A small change since v1; one patch is dropped and another is updated to make up for it. Felipe Contreras (3): read-cache: plug a few leaks unpack-trees: plug a memory leak unpack-trees: free created cache entries read-cache.c | 4 unpack-trees.c | 16 +--- 2 files changed, 17 insertions(+), 3 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 2/3] unpack-trees: plug a memory leak
Before overwriting the destination index, first let's discard it's
contents.
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index ede4299..eff2944 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1146,8 +1146,10 @@ int unpack_trees(unsigned len, struct tree_desc *t,
struct unpack_trees_options
o->src_index = NULL;
ret = check_updates(o) ? (-2) : 0;
- if (o->dst_index)
+ if (o->dst_index) {
+ discard_index(o->dst_index);
*o->dst_index = o->result;
+ }
done:
clear_exclude_list(&el);
--
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 3/3] unpack-trees: free created cache entries
We created them, and nobody else is going to destroy them.
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index eff2944..9f19d01 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -590,8 +590,16 @@ static int unpack_nondirectories(int n, unsigned long mask,
src[i + o->merge] = create_ce_entry(info, names + i, stage);
}
- if (o->merge)
- return call_unpack_fn(src, o);
+ if (o->merge) {
+ int ret = call_unpack_fn(src, o);
+ for (i = 0; i < n; i++) {
+ struct cache_entry *ce = src[i + o->merge];
+ if (!ce || ce == o->df_conflict_entry)
+ continue;
+ free(ce);
+ }
+ return ret;
+ }
for (i = 0; i < n; i++)
if (src[i] && src[i] != o->df_conflict_entry)
--
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/3] read-cache: plug a few leaks
We don't free 'istate->cache' properly. Apparently 'initialized' doesn't really mean initialized, but loaded, or rather 'not-empty', and the cache can be used even if it's not 'initialized', so we can't rely on this variable to keep track of the 'istate->cache'. So assume it always has data, and free it before overwriting it. Signed-off-by: Felipe Contreras --- read-cache.c | 4 1 file changed, 4 insertions(+) diff --git a/read-cache.c b/read-cache.c index 04ed561..e5dc96f 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1449,6 +1449,7 @@ int read_index_from(struct index_state *istate, const char *path) istate->version = ntohl(hdr->hdr_version); istate->cache_nr = ntohl(hdr->hdr_entries); istate->cache_alloc = alloc_nr(istate->cache_nr); + free(istate->cache); istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry *)); istate->initialized = 1; @@ -1510,6 +1511,9 @@ int discard_index(struct index_state *istate) for (i = 0; i < istate->cache_nr; i++) free(istate->cache[i]); + free(istate->cache); + istate->cache = NULL; + istate->cache_alloc = 0; resolve_undo_clear_index(istate); istate->cache_nr = 0; istate->cache_changed = 0; -- 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: Poor performance of git describe in big repos
On Thu, May 30, 2013 at 7:29 PM, Alex Bennée wrote: > I ran perf on it and the top items in the report where: > > 41.58% git libcrypto.so.1.0.0 [.] 0x6ae73 > 33.96% git libz.so.1.2.3.4 [.] 0xe0ec > 10.39% git libz.so.1.2.3.4 [.] adler32 > 2.03% git [kernel.kallsyms] [k] clear_page_c > > So I'm guessing it's spending a lot of non-cache efficient time > un-packing and processing the deltas? If I'm not mistaken, commits are never deltified. They are usually small and packed close together for better I/O patterns. If you really just read hundreds of commits, it can't take that long. Maybe some code paths accidentally open a tree, a blob or something.. Can you try setting core.logpackaccess to a path on and rerun describe? Jugding from the code (I never actually tried it), it'll create a file at the given path with the accessed pack offsets. You can check what offset corresponds to what object with verify-pack -v. -- 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: Poor performance of git describe in big repos
> You may find that performance improves if you repack with "git gc --aggressive". It seems that increases the time to get to where it wants to: 14:12 ajb@sloy/x86_64 [work.git] >time /usr/bin/git --no-pager describe --long --tags --debug searching to describe HEAD lightweight 10 ajb-build-test-5224 lightweight 41 ajb-build-test-5222 annotated146 vnms-2-1-36-32 annotated155 vnms-2-1-36-31 annotated174 vnms-2-1-36-30 annotated183 vnms-2-1-36-29 lightweight 188 vnms-2-1-36-28 annotated193 vnms-2-1-36-27 annotated206 vnms-2-1-36-26 annotated215 vectastar-4-2-83-5 traversed 223 commits more than 10 tags found; listed 10 most recent gave up search at 2b69df72d47be8440e3ce4cee91b9b7ceaf8b77c ajb-build-test-5224-10-gfa296e6 real0m14.658s user0m12.845s sys 0m1.776s On 30 May 2013 12:48, John Keeping wrote: > On Thu, May 30, 2013 at 11:38:32AM +0100, Alex Bennée wrote: >> One factor might be the size of my repo (.git is around 2.4G). Could >> this just be due to computational cost of searching through large >> packs to walk the commit chain? Is there any way to make this easier >> for git to do? > > What does "git count-objects -v" say for your repository? > > You may find that performance improves if you repack with "git gc > --aggressive". -- Alex, homepage: http://www.bennee.com/~alex/ -- 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: Poor performance of git describe in big repos
It looks like it's a file caching effect combined with my repo being more pathalogical in size and contents. Note run 1 (cold) vs run 2 on the linux file tree: 13:52 ajb@sloy/x86_64 [linux.git] >time git describe --debug --long --tags HEAD~1 searching to describe HEAD~1 annotated 57 v2.6.34-rc2 annotated 1688 v2.6.34-rc1 annotated 7932 v2.6.33 annotated 8157 v2.6.33-rc8 annotated 8381 v2.6.33-rc7 annotated 8637 v2.6.33-rc6 annotated 8964 v2.6.33-rc5 annotated 9493 v2.6.33-rc4 annotated 9912 v2.6.33-rc3 annotated 10202 v2.6.33-rc2 traversed 10547 commits more than 10 tags found; listed 10 most recent gave up search at 55639353a0035052d9ea6cfe4dde0ac7fcbb2c9f v2.6.34-rc2-57-gef5da59 real0m7.332s user0m0.308s sys 0m0.244s 14:03 ajb@sloy/x86_64 [linux.git] >time git describe --debug --long --tags HEAD~1 searching to describe HEAD~1 annotated 57 v2.6.34-rc2 annotated 1688 v2.6.34-rc1 annotated 7932 v2.6.33 annotated 8157 v2.6.33-rc8 annotated 8381 v2.6.33-rc7 annotated 8637 v2.6.33-rc6 annotated 8964 v2.6.33-rc5 annotated 9493 v2.6.33-rc4 annotated 9912 v2.6.33-rc3 annotated 10202 v2.6.33-rc2 traversed 10547 commits more than 10 tags found; listed 10 most recent gave up search at 55639353a0035052d9ea6cfe4dde0ac7fcbb2c9f v2.6.34-rc2-57-gef5da59 real0m0.298s user0m0.244s sys 0m0.036s Although the perf profile looks subtly different. First through the linux tree: 22.35% git libz.so.1.2.3.4[.] inflate 18.56% git libz.so.1.2.3.4[.] inflate_fast 17.48% git libz.so.1.2.3.4[.] inflate_table 7.84% git git[.] hashcmp 3.93% git git[.] get_sha1_hex 3.46% git libz.so.1.2.3.4[.] adler32 And through my "special" repo: 41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3 33.62% git libz.so.1.2.3.4 [.] inflate_fast 10.39% git libz.so.1.2.3.4 [.] adler32 2.03% git [kernel.kallsyms] [k] clear_page_c I'm not sure why libcrypto features so highly in the results -- Alex. On 30 May 2013 12:33, Ramkumar Ramachandra wrote: > Alex Bennée wrote: >>>time /usr/bin/git --no-pager >> traversed 223 commits >> >> real0m4.817s >> user0m4.320s >> sys 0m0.464s > > I'm quite clueless about why it is taking this long: I think it's IO > because there's nothing to compute? I really can't trace anything > unless you can reproduce it on a public repository. On linux.git with > my rotating hard disk: > > $ time git describe --debug --long --tags HEAD~1 > searching to describe HEAD~1 > annotated 5445 v2.6.33 > annotated 5660 v2.6.33-rc8 > annotated 5884 v2.6.33-rc7 > annotated 6140 v2.6.33-rc6 > annotated 6467 v2.6.33-rc5 > annotated 6999 v2.6.33-rc4 > annotated 7430 v2.6.33-rc3 > annotated 7746 v2.6.33-rc2 > annotated 8212 v2.6.33-rc1 > annotated 13854 v2.6.32 > traversed 18895 commits > more than 10 tags found; listed 10 most recent > gave up search at 648f4e3e50c4793d9dbf9a09afa193631f76fa26 > v2.6.33-5445-ge7c84ee > > real0m0.509s > user0m0.470s > sys 0m0.037s > > 18k+ commits traversed in half a second here, so I really don't know > what is going on. -- Alex, homepage: http://www.bennee.com/~alex/ -- 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/4] commit: reload cache properly
On Thu, May 30, 2013 at 7:17 PM, Thomas Rast wrote:
> Felipe Contreras writes:
>
>> We are supposedly adding files, to to which cache if 'the_index' is
>> discarded?
> [...]
>> if (!current_head) {
>> discard_cache();
>> + if (read_cache() < 0)
>> + die(_("cannot read the index"));
>> return;
>> }
>
> It is not obvious to me that this is a correct change. discard_cache()
> without subsequent reloading could also legitimately be used to empty
> the index. So if you are fixing a bug, please justify the change and
> provide a testcase to guard against it in the future.
That discard_cache line I think came from fa9dcf8 (Fix performance
regression for partial commits - 2008-01-13). The code flow back then
was
if (initial_commit)
discard_cache();
add_remove_files();
/* do something more */
A quick look from current code seems to indicate this pattern is still
valid for creating partial commits, where temporary index will be
thrown away afterwards. But I may be wrong.
--
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 1/4] commit: reload cache properly
On Thu, May 30, 2013 at 7:58 AM, Thomas Rast wrote:
> Felipe Contreras writes:
>
>> On Thu, May 30, 2013 at 7:17 AM, Thomas Rast wrote:
>>> Felipe Contreras writes:
>>>
We are supposedly adding files, to to which cache if 'the_index' is
discarded?
>>> [...]
if (!current_head) {
discard_cache();
+ if (read_cache() < 0)
+ die(_("cannot read the index"));
return;
}
>>>
>>> It is not obvious to me that this is a correct change. discard_cache()
>>> without subsequent reloading could also legitimately be used to empty
>>> the index. So if you are fixing a bug, please justify the change and
>>> provide a testcase to guard against it in the future.
>>
>> So istate->initialized is false, yet somebody can still add entries to
>> the cache? What happens when somebody else tries to initialize this
>> cache? All the entries there will be lost, even though nobody
>> discarded it afterwards.
>
> And yet it works, and your patch breaks it.
It might work, but the API doesn't make any sense.
--
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/4] commit: reload cache properly
Felipe Contreras writes:
> On Thu, May 30, 2013 at 7:17 AM, Thomas Rast wrote:
>> Felipe Contreras writes:
>>
>>> We are supposedly adding files, to to which cache if 'the_index' is
>>> discarded?
>> [...]
>>> if (!current_head) {
>>> discard_cache();
>>> + if (read_cache() < 0)
>>> + die(_("cannot read the index"));
>>> return;
>>> }
>>
>> It is not obvious to me that this is a correct change. discard_cache()
>> without subsequent reloading could also legitimately be used to empty
>> the index. So if you are fixing a bug, please justify the change and
>> provide a testcase to guard against it in the future.
>
> So istate->initialized is false, yet somebody can still add entries to
> the cache? What happens when somebody else tries to initialize this
> cache? All the entries there will be lost, even though nobody
> discarded it afterwards.
And yet it works, and your patch breaks it.
diff --git i/t/t7501-commit.sh w/t/t7501-commit.sh
index 195e747..1608254 100755
--- i/t/t7501-commit.sh
+++ w/t/t7501-commit.sh
@@ -524,4 +524,16 @@ test_expect_success 'commit a file whose name is a dash' '
test_i18ngrep " changed, 5 insertions" output
'
+test_expect_success '--only works on to-be-born branch' '
+ git checkout --orphan orphan &&
+ echo foo >newfile &&
+ git add newfile &&
+ git commit --only newfile -m"--only on unborn branch" &&
+ cat >expected
Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present
Am 30.05.2013 07:30, schrieb Martin von Zweigbergk:
> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt wrote:
>> Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
>>> +# f
>>> +# /
>>> +# a---b---c---g---h
>>> +# \
>>> +# d---G---i
>> ...
>>> +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..
>>
>> Isn't this expectation wrong? The upstream of the rebased branch is f, and
>> it does not contain G. Hence, G should be replayed. Since h is the
>> reversal of g, the state at h is the same as at c, and applying G should
>> succeed (it is the same change as g). Therefore, I think the correct
>> expectation is:
>>
>> test_linear_range 'd G i' h..
>
> Good question! It is really not obvious what the right answer is. Some
> arguments in favor of dropping 'G':
>
> 1. Let's say origin/master points to 'b' when you start the 'd G i'
> branch. You then send the 'G' patch to Junio who applies it as 'g'
> (cherry-picking direction is reversed compared to figure, but same
> effect). You then "git pull --rebase" when master on origin points to
> 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git
> rebase --onto h b i'.
The reason for this git pull cleverness is to be prepared for rewritten
history:
b'--c'--g'--h'
/
a---b
\
d---G---i
to avoid that b is rebased.
> In this case it's clearly useful to have the
> patch dropped.
>
> 2. In the test a little before the above one, we instead do 'git
> rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that
> case it doesn't matter what's in $branch..$upstream. Do we agree that
> $branch..$upstream should never matter (instead, $upstream is only
> used to find merge base with $branch)?
No, we do not agree. $branch..$upstream should be the set of patches
that should be omitted. $branch..$onto should not matter. $onto is only
used to specify the destination of the rebased commits.
> Do we also agree that 'git
> rebase a b' should be identical to 'git rebase --onto a a b'?
Absolutely!
> Because
> 'git rebase h i' should clearly drop 'G', then so should 'git rebase
> --onto h h i'.
Yes!
> Then, if we agreed that $branch..$upstream doesn't
> matter, 'git rebase --onto h f i' should behave the same, no?
Correct in the mathematically logical sense. ;) But we do not agree that
$branch..$upstream doesn't matter.
> The set of commits to rebase that I was thinking of using was
> "$upstream..$branch, unless equivalent with patch in $branch..$onto".
> But I'm not very confident about my conclusions above :-)
At least the man page says that ..$upstream counts and $onto tells just
the new base.
The way how git pull calls rebase should be revisited, I think.
-- 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: [PATCH 1/4] commit: reload cache properly
On Thu, May 30, 2013 at 7:17 AM, Thomas Rast wrote:
> Felipe Contreras writes:
>
>> We are supposedly adding files, to to which cache if 'the_index' is
>> discarded?
> [...]
>> if (!current_head) {
>> discard_cache();
>> + if (read_cache() < 0)
>> + die(_("cannot read the index"));
>> return;
>> }
>
> It is not obvious to me that this is a correct change. discard_cache()
> without subsequent reloading could also legitimately be used to empty
> the index. So if you are fixing a bug, please justify the change and
> provide a testcase to guard against it in the future.
So istate->initialized is false, yet somebody can still add entries to
the cache? What happens when somebody else tries to initialize this
cache? All the entries there will be lost, even though nobody
discarded it afterwards.
--
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: Poor performance of git describe in big repos
The repo is a fairly hairy one as it includes two historically un-related but content related repos which I'm the process of cherry-picking stuff across. 11:58 ajb@sloy/x86_64 [work.git] >git count-objects -v count: 493 size: 4572 in-pack: 399307 packs: 1 size-pack: 1930755 prune-packable: 0 garbage: 0 size-garbage: 0 This was after a repack which did have slight negative effect on performance. The pack file is: 13:27 ajb@sloy/x86_64 [work.git] >ls -lh ./.git/objects/pack/* -r--r--r-- 1 ajb cvs 11M May 30 11:56 ./.git/objects/pack/pack-a9ba133a6f25ffa74c3c407e09ab030f8745b201.idx -r--r--r-- 1 ajb cvs 1.9G May 30 11:56 ./.git/objects/pack/pack-a9ba133a6f25ffa74c3c407e09ab030f8745b201.pack I ran perf on it and the top items in the report where: 41.58% git libcrypto.so.1.0.0 [.] 0x6ae73 33.96% git libz.so.1.2.3.4 [.] 0xe0ec 10.39% git libz.so.1.2.3.4 [.] adler32 2.03% git [kernel.kallsyms] [k] clear_page_c So I'm guessing it's spending a lot of non-cache efficient time un-packing and processing the deltas? -- Alex. On 30 May 2013 12:48, John Keeping wrote: > On Thu, May 30, 2013 at 11:38:32AM +0100, Alex Bennée wrote: >> One factor might be the size of my repo (.git is around 2.4G). Could >> this just be due to computational cost of searching through large >> packs to walk the commit chain? Is there any way to make this easier >> for git to do? > > What does "git count-objects -v" say for your repository? > > You may find that performance improves if you repack with "git gc > --aggressive". -- Alex, homepage: http://www.bennee.com/~alex/ -- 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/4] commit: reload cache properly
Felipe Contreras writes:
> We are supposedly adding files, to to which cache if 'the_index' is
> discarded?
[...]
> if (!current_head) {
> discard_cache();
> + if (read_cache() < 0)
> + die(_("cannot read the index"));
> return;
> }
It is not obvious to me that this is a correct change. discard_cache()
without subsequent reloading could also legitimately be used to empty
the index. So if you are fixing a bug, please justify the change and
provide a testcase to guard against it in the future.
--
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
Re: [PATCH v7] Add new git-related helper to contrib
On Thu, May 30, 2013 at 7:08 AM, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: >> What's your objective? Block this patch from ever going in? >> >> Not a single one of these comments makes a difference at all, all of >> them can wait until after the patch is merged, many of them are a >> matter of preferences, and some of them have already been addressed as >> precisely that: disagreements in style. > > You posted a patch, and I reviewed it. End of story. I never > explicitly or implicitly indicated that I want to block the patch, so > stop pulling stuff out of your arse. That's exactly what you are doing. Do you see any other reason for not merging this if not your comments? -- 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 v7] Add new git-related helper to contrib
Felipe Contreras wrote: > What's your objective? Block this patch from ever going in? > > Not a single one of these comments makes a difference at all, all of > them can wait until after the patch is merged, many of them are a > matter of preferences, and some of them have already been addressed as > precisely that: disagreements in style. You posted a patch, and I reviewed it. End of story. I never explicitly or implicitly indicated that I want to block the patch, so stop pulling stuff out of your arse. If you don't want a review, write "DO NOT REVIEW" (or better yet, don't hit my inbox). I'm not interested in wasting my time either. -- 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 7/7] unpack-trees: free cache_entry array members for merges
On Thu, May 30, 2013 at 6:34 AM, René Scharfe
wrote:
> The merge functions duplicate entries as needed and they don't free
> them. Release them in unpack_nondirectories, the same function
> where they were allocated, after we're done.
Ah, you beat me to this change, but..
> @@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned long
> mask,
> src[i + o->merge] = create_ce_entry(info, names + i, stage);
> }
>
> - if (o->merge)
> - return call_unpack_fn((const struct cache_entry * const *)src,
> - o);
> + if (o->merge) {
> + int rc = call_unpack_fn((const struct cache_entry * const
> *)src,
> + o);
> + for (i = 1; i <= n; i++)
> + if (src[i] && src[i] != o->df_conflict_entry)
> + free(src[i]);
Doesn't it make more sense to follow the code above and do src[i + o->merge]?
--
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 3/4] unpack-trees: plug a memory leak
Before overwriting the destination index, first let's discard it's
contents.
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index ede4299..eff2944 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1146,8 +1146,10 @@ int unpack_trees(unsigned len, struct tree_desc *t,
struct unpack_trees_options
o->src_index = NULL;
ret = check_updates(o) ? (-2) : 0;
- if (o->dst_index)
+ if (o->dst_index) {
+ discard_index(o->dst_index);
*o->dst_index = o->result;
+ }
done:
clear_exclude_list(&el);
--
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 4/4] unpack-trees: free created cache entries
We created them, and nobody else is going to destroy them.
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index eff2944..9f19d01 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -590,8 +590,16 @@ static int unpack_nondirectories(int n, unsigned long mask,
src[i + o->merge] = create_ce_entry(info, names + i, stage);
}
- if (o->merge)
- return call_unpack_fn(src, o);
+ if (o->merge) {
+ int ret = call_unpack_fn(src, o);
+ for (i = 0; i < n; i++) {
+ struct cache_entry *ce = src[i + o->merge];
+ if (!ce || ce == o->df_conflict_entry)
+ continue;
+ free(ce);
+ }
+ return ret;
+ }
for (i = 0; i < n; i++)
if (src[i] && src[i] != o->df_conflict_entry)
--
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 2/4] read-cache: plug small memory leak
We free each entry, but not the array of entries themselves. Signed-off-by: Felipe Contreras --- read-cache.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/read-cache.c b/read-cache.c index 04ed561..9d9b886 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1510,6 +1510,8 @@ int discard_index(struct index_state *istate) for (i = 0; i < istate->cache_nr; i++) free(istate->cache[i]); + free(istate->cache); + istate->cache = NULL; resolve_undo_clear_index(istate); istate->cache_nr = 0; istate->cache_changed = 0; -- 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 1/4] commit: reload cache properly
We are supposedly adding files, to to which cache if 'the_index' is
discarded?
Signed-off-by: Felipe Contreras
---
builtin/commit.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/commit.c b/builtin/commit.c
index d2f30d9..092b49e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -244,6 +244,8 @@ static void create_base_index(const struct commit
*current_head)
if (!current_head) {
discard_cache();
+ if (read_cache() < 0)
+ die(_("cannot read the index"));
return;
}
--
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 0/4] cherry-pick: fix memory leaks
Hi, I took a shot at fixing the memory leaks of cherry-pick, and at least in my tests the memory doesn't seem to increase any more. Felipe Contreras (4): commit: reload cache properly read-cache: plug small memory leak unpack-trees: plug a memory leak unpack-trees: free created cache entries builtin/commit.c | 2 ++ read-cache.c | 2 ++ unpack-trees.c | 16 +--- 3 files changed, 17 insertions(+), 3 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
Re: Poor performance of git describe in big repos
On Thu, May 30, 2013 at 11:38:32AM +0100, Alex Bennée wrote: > One factor might be the size of my repo (.git is around 2.4G). Could > this just be due to computational cost of searching through large > packs to walk the commit chain? Is there any way to make this easier > for git to do? What does "git count-objects -v" say for your repository? You may find that performance improves if you repack with "git gc --aggressive". -- 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] Makefile: promote wildmatch to be the default fnmatch implementation
On Thu, May 30, 2013 at 9:25 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> This makes git use wildmatch by default for all fnmatch() calls. Users >> who want to use system fnmatch (or compat fnmatch) need to set >> NO_WILDMATCH flag. >> >> wildmatch is a drop-in fnmatch replacement with more features. Using >> wildmatch gives us a consistent behavior across platforms. > > While I agree this is a good move in the longer term in that we get > the often-asked-for "foo/**/*.c" match and also we have one less > platform differences to worry about, I somehow have a recollection > that we discussed that there are incompatibilities in dark corners > we would want to warn users about and lay a transition plan across > some major version bump. I've skimmed through all wildmatch related mails in my gmail archive. There are differences between fnmatch versions, e.g. [1], but I don't think anyone would run into those cases on purpose. There were performance concerns [2] and they should have been addressed with nd/retire-fnmatch series. Originally I was worried that this new code might not be mature enough, but I've been running wildmatch-only git for quite some time, can't really complain. Not really a transition plan, but maybe we could provide a runtime switch to return to system fnmatch when wildmatch becomes default, for a few cycles. This way if wildmatch turns out broken, people can switch back while we work on a fix. > Hmph, could you (no need to hurry, though) check the previous > discussion and point at what we decided if we did reach any > conclusion to refresh our collective memory? We all seemed to agree that the replacement would be a way to go. But not hard decision was reached. [1] http://thread.gmane.org/gmane.comp.version-control.git/207385/focus=207540 [2] http://thread.gmane.org/gmane.comp.version-control.git/211823/focus=211836 -- 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 5/7] diff-lib, read-tree, unpack-trees: mark cache_entry pointers const
Add const to struct cache_entry pointers throughout the tree which are
only used for reading. This allows callers to pass in const pointers.
Signed-off-by: René Scharfe
---
builtin/read-tree.c | 2 +-
diff-lib.c | 23 +++---
unpack-trees.c | 91 +
3 files changed, 63 insertions(+), 53 deletions(-)
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 042ac1b..b847486 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -66,7 +66,7 @@ static int exclude_per_directory_cb(const struct option *opt,
const char *arg,
return 0;
}
-static void debug_stage(const char *label, struct cache_entry *ce,
+static void debug_stage(const char *label, const struct cache_entry *ce,
struct unpack_trees_options *o)
{
printf("%s ", label);
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..83d0cb8 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -64,8 +64,9 @@ static int check_removed(const struct cache_entry *ce, struct
stat *st)
* commits, untracked content and/or modified content).
*/
static int match_stat_with_submodule(struct diff_options *diffopt,
- struct cache_entry *ce, struct stat *st,
- unsigned ce_option, unsigned
*dirty_submodule)
+const struct cache_entry *ce,
+struct stat *st, unsigned ce_option,
+unsigned *dirty_submodule)
{
int changed = ce_match_stat(ce, st, ce_option);
if (S_ISGITLINK(ce->ce_mode)) {
@@ -237,7 +238,7 @@ int run_diff_files(struct rev_info *revs, unsigned int
option)
/* A file entry went away or appeared */
static void diff_index_show_file(struct rev_info *revs,
const char *prefix,
-struct cache_entry *ce,
+const struct cache_entry *ce,
const unsigned char *sha1, int sha1_valid,
unsigned int mode,
unsigned dirty_submodule)
@@ -246,7 +247,7 @@ static void diff_index_show_file(struct rev_info *revs,
sha1, sha1_valid, ce->name, dirty_submodule);
}
-static int get_stat_data(struct cache_entry *ce,
+static int get_stat_data(const struct cache_entry *ce,
const unsigned char **sha1p,
unsigned int *modep,
int cached, int match_missing,
@@ -283,7 +284,7 @@ static int get_stat_data(struct cache_entry *ce,
}
static void show_new_file(struct rev_info *revs,
- struct cache_entry *new,
+ const struct cache_entry *new,
int cached, int match_missing)
{
const unsigned char *sha1;
@@ -302,8 +303,8 @@ static void show_new_file(struct rev_info *revs,
}
static int show_modified(struct rev_info *revs,
-struct cache_entry *old,
-struct cache_entry *new,
+const struct cache_entry *old,
+const struct cache_entry *new,
int report_missing,
int cached, int match_missing)
{
@@ -362,8 +363,8 @@ static int show_modified(struct rev_info *revs,
* give you the position and number of entries in the index).
*/
static void do_oneway_diff(struct unpack_trees_options *o,
- struct cache_entry *idx,
- struct cache_entry *tree)
+ const struct cache_entry *idx,
+ const struct cache_entry *tree)
{
struct rev_info *revs = o->unpack_data;
int match_missing, cached;
@@ -425,8 +426,8 @@ static void do_oneway_diff(struct unpack_trees_options *o,
*/
static int oneway_diff(struct cache_entry **src, struct unpack_trees_options
*o)
{
- struct cache_entry *idx = src[0];
- struct cache_entry *tree = src[1];
+ const struct cache_entry *idx = src[0];
+ const struct cache_entry *tree = src[1];
struct rev_info *revs = o->unpack_data;
/*
diff --git a/unpack-trees.c b/unpack-trees.c
index 2fecef8..c5a40df 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -241,8 +241,11 @@ static int check_updates(struct unpack_trees_options *o)
return errs != 0;
}
-static int verify_uptodate_sparse(struct cache_entry *ce, struct
unpack_trees_options *o);
-static int verify_absent_sparse(struct cache_entry *ce, enum
unpack_trees_error_types, struct unpack_trees_options *o);
+static int verify_uptodate_sparse(const struct cache_entry *ce,
+ struct unpack_trees_options *o);
+static int verify_absent_sparse(const struct cache_entry *ce,
+ enum unpack_trees_error_types,
+ struct un
[PATCH 6/7] diff-lib, read-tree, unpack-trees: mark cache_entry array paramters const
Change the type merge_fn_t to accept the array of cache_entry pointers
as const pointers to const pointers. This documents the fact that the
merge functions don't modify the cache_entry contents or replace any of
the pointers in the array.
Only a single cast is necessary in unpack_nondirectories because adding
two const modifiers at once is not allowed in C. The cast is safe in
that it doesn't mask any modfication; call_unpack_fn only needs the
array for reading.
Signed-off-by: René Scharfe
---
builtin/read-tree.c | 3 ++-
diff-lib.c | 3 ++-
unpack-trees.c | 21 +
unpack-trees.h | 14 +-
4 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index b847486..0f5d7fe 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -80,7 +80,8 @@ static void debug_stage(const char *label, const struct
cache_entry *ce,
sha1_to_hex(ce->sha1));
}
-static int debug_merge(struct cache_entry **stages, struct
unpack_trees_options *o)
+static int debug_merge(const struct cache_entry * const *stages,
+ struct unpack_trees_options *o)
{
int i;
diff --git a/diff-lib.c b/diff-lib.c
index 83d0cb8..b6f4b21 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -424,7 +424,8 @@ static void do_oneway_diff(struct unpack_trees_options *o,
* the fairly complex unpack_trees() semantic requirements, including
* the skipping, the path matching, the type conflict cases etc.
*/
-static int oneway_diff(struct cache_entry **src, struct unpack_trees_options
*o)
+static int oneway_diff(const struct cache_entry * const *src,
+ struct unpack_trees_options *o)
{
const struct cache_entry *idx = src[0];
const struct cache_entry *tree = src[1];
diff --git a/unpack-trees.c b/unpack-trees.c
index c5a40df..2dbc05d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -300,7 +300,8 @@ static int apply_sparse_checkout(struct cache_entry *ce,
struct unpack_trees_opt
return 0;
}
-static inline int call_unpack_fn(struct cache_entry **src, struct
unpack_trees_options *o)
+static inline int call_unpack_fn(const struct cache_entry * const *src,
+struct unpack_trees_options *o)
{
int ret = o->fn(src, o);
if (ret > 0)
@@ -397,7 +398,7 @@ static void add_same_unmerged(struct cache_entry *ce,
static int unpack_index_entry(struct cache_entry *ce,
struct unpack_trees_options *o)
{
- struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };
+ const struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };
int ret;
src[0] = ce;
@@ -600,7 +601,8 @@ static int unpack_nondirectories(int n, unsigned long mask,
}
if (o->merge)
- return call_unpack_fn(src, o);
+ return call_unpack_fn((const struct cache_entry * const *)src,
+ o);
for (i = 0; i < n; i++)
if (src[i] && src[i] != o->df_conflict_entry)
@@ -1574,7 +1576,8 @@ static void show_stage_entry(FILE *o,
}
#endif
-int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)
+int threeway_merge(const struct cache_entry * const *stages,
+ struct unpack_trees_options *o)
{
const struct cache_entry *index;
const struct cache_entry *head;
@@ -1746,7 +1749,8 @@ int threeway_merge(struct cache_entry **stages, struct
unpack_trees_options *o)
* "carry forward" rule, please see .
*
*/
-int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)
+int twoway_merge(const struct cache_entry * const *src,
+struct unpack_trees_options *o)
{
const struct cache_entry *current = src[0];
const struct cache_entry *oldtree = src[1];
@@ -1812,8 +1816,8 @@ int twoway_merge(struct cache_entry **src, struct
unpack_trees_options *o)
* Keep the index entries at stage0, collapse stage1 but make sure
* stage0 does not have anything there.
*/
-int bind_merge(struct cache_entry **src,
- struct unpack_trees_options *o)
+int bind_merge(const struct cache_entry * const *src,
+ struct unpack_trees_options *o)
{
const struct cache_entry *old = src[0];
const struct cache_entry *a = src[1];
@@ -1836,7 +1840,8 @@ int bind_merge(struct cache_entry **src,
* The rule is:
* - take the stat information from stage0, take the data from stage1
*/
-int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)
+int oneway_merge(const struct cache_entry * const *src,
+struct unpack_trees_options *o)
{
const struct cache_entry *old = src[0];
const struct cache_entry *a = src[1];
diff --git a/unpack-trees.h b/unpack-trees.h
index 5e432f5..36a73a6 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -8,7 +8,7 @@
struct unpack_tree
[PATCH 4/7] unpack-trees: create working copy of merge entry in merged_entry
Duplicate the merge entry right away and work with that instead of
modifying the entry we got and duplicating it only at the end of
the function. Then mark that pointer const to document that we
don't modify the referenced cache_entry.
This change is safe because all existing merge functions call
merged_entry just before returning (or not at all), i.e. they don't
care about changes to the referenced cache_entry after the call.
unpack_nondirectories and unpack_index_entry, which call the merge
functions through call_unpack_fn, aren't interested in such changes
neither.
The change complicates merged_entry a bit because we have to free the
copy if we error out, but allows callers to pass a const pointer.
Signed-off-by: René Scharfe
---
unpack-trees.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index e8b4cc1..2fecef8 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1466,10 +1466,12 @@ static int verify_absent_sparse(struct cache_entry *ce,
return verify_absent_1(ce, orphaned_error, o);
}
-static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
- struct unpack_trees_options *o)
+static int merged_entry(const struct cache_entry *ce,
+ struct cache_entry *old,
+ struct unpack_trees_options *o)
{
int update = CE_UPDATE;
+ struct cache_entry *merge = dup_entry(ce);
if (!old) {
/*
@@ -1487,8 +1489,11 @@ static int merged_entry(struct cache_entry *merge,
struct cache_entry *old,
update |= CE_ADDED;
merge->ce_flags |= CE_NEW_SKIP_WORKTREE;
- if (verify_absent(merge,
ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
+ if (verify_absent(merge,
+ ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) {
+ free(merge);
return -1;
+ }
invalidate_ce_path(merge, o);
} else if (!(old->ce_flags & CE_CONFLICTED)) {
/*
@@ -1502,8 +1507,10 @@ static int merged_entry(struct cache_entry *merge,
struct cache_entry *old,
copy_cache_entry(merge, old);
update = 0;
} else {
- if (verify_uptodate(old, o))
+ if (verify_uptodate(old, o)) {
+ free(merge);
return -1;
+ }
/* Migrate old flags over */
update |= old->ce_flags & (CE_SKIP_WORKTREE |
CE_NEW_SKIP_WORKTREE);
invalidate_ce_path(old, o);
@@ -1516,7 +1523,7 @@ static int merged_entry(struct cache_entry *merge, struct
cache_entry *old,
invalidate_ce_path(old, o);
}
- add_entry(o, merge, update, CE_STAGEMASK);
+ do_add_entry(o, merge, update, CE_STAGEMASK);
return 1;
}
--
1.8.3
--
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 3/7] unpack-trees: factor out dup_entry
While we're add it, mark the struct cache_entry pointer of add_entry
const because we only read from it and this allows callers to pass in
const pointers.
Signed-off-by: René Scharfe
---
unpack-trees.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index ede4299..e8b4cc1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -116,14 +116,20 @@ static void do_add_entry(struct unpack_trees_options *o,
struct cache_entry *ce,
ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
}
-static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
- unsigned int set, unsigned int clear)
+static struct cache_entry *dup_entry(const struct cache_entry *ce)
{
unsigned int size = ce_size(ce);
struct cache_entry *new = xmalloc(size);
memcpy(new, ce, size);
- do_add_entry(o, new, set, clear);
+ return new;
+}
+
+static void add_entry(struct unpack_trees_options *o,
+ const struct cache_entry *ce,
+ unsigned int set, unsigned int clear)
+{
+ do_add_entry(o, dup_entry(ce), set, clear);
}
/*
--
1.8.3
--
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 7/7] unpack-trees: free cache_entry array members for merges
The merge functions duplicate entries as needed and they don't free
them. Release them in unpack_nondirectories, the same function
where they were allocated, after we're done.
Signed-off-by: René Scharfe
---
unpack-trees.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 2dbc05d..fc0dd5a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned long mask,
src[i + o->merge] = create_ce_entry(info, names + i, stage);
}
- if (o->merge)
- return call_unpack_fn((const struct cache_entry * const *)src,
- o);
+ if (o->merge) {
+ int rc = call_unpack_fn((const struct cache_entry * const *)src,
+ o);
+ for (i = 1; i <= n; i++)
+ if (src[i] && src[i] != o->df_conflict_entry)
+ free(src[i]);
+ return rc;
+ }
for (i = 0; i < n; i++)
if (src[i] && src[i] != o->df_conflict_entry)
--
1.8.3
--
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 2/7] read-cache: mark cache_entry pointers const
ie_match_stat and ie_modified only derefence their struct cache_entry
pointers for reading. Add const to the parameter declaration here and
do the same for the static helper function used by them, as it's the
same there as well. This allows callers to pass in const pointers.
Signed-off-by: René Scharfe
---
cache.h | 4 ++--
read-cache.c | 18 ++
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/cache.h b/cache.h
index 43a27e7..01e8760 100644
--- a/cache.h
+++ b/cache.h
@@ -482,8 +482,8 @@ extern void *read_blob_data_from_index(struct index_state
*, const char *, unsig
#define CE_MATCH_RACY_IS_DIRTY 02
/* do stat comparison even if CE_SKIP_WORKTREE is true */
#define CE_MATCH_IGNORE_SKIP_WORKTREE 04
-extern int ie_match_stat(const struct index_state *, struct cache_entry *,
struct stat *, unsigned int);
-extern int ie_modified(const struct index_state *, struct cache_entry *,
struct stat *, unsigned int);
+extern int ie_match_stat(const struct index_state *, const struct cache_entry
*, struct stat *, unsigned int);
+extern int ie_modified(const struct index_state *, const struct cache_entry *,
struct stat *, unsigned int);
#define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR
*/
diff --git a/read-cache.c b/read-cache.c
index 04ed561..e6e0466 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -91,7 +91,7 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat
*st)
ce_mark_uptodate(ce);
}
-static int ce_compare_data(struct cache_entry *ce, struct stat *st)
+static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
{
int match = -1;
int fd = open(ce->name, O_RDONLY);
@@ -105,7 +105,7 @@ static int ce_compare_data(struct cache_entry *ce, struct
stat *st)
return match;
}
-static int ce_compare_link(struct cache_entry *ce, size_t expected_size)
+static int ce_compare_link(const struct cache_entry *ce, size_t expected_size)
{
int match = -1;
void *buffer;
@@ -126,7 +126,7 @@ static int ce_compare_link(struct cache_entry *ce, size_t
expected_size)
return match;
}
-static int ce_compare_gitlink(struct cache_entry *ce)
+static int ce_compare_gitlink(const struct cache_entry *ce)
{
unsigned char sha1[20];
@@ -143,7 +143,7 @@ static int ce_compare_gitlink(struct cache_entry *ce)
return hashcmp(sha1, ce->sha1);
}
-static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
+static int ce_modified_check_fs(const struct cache_entry *ce, struct stat *st)
{
switch (st->st_mode & S_IFMT) {
case S_IFREG:
@@ -163,7 +163,7 @@ static int ce_modified_check_fs(struct cache_entry *ce,
struct stat *st)
return 0;
}
-static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
+static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
{
unsigned int changed = 0;
@@ -239,7 +239,8 @@ static int ce_match_stat_basic(struct cache_entry *ce,
struct stat *st)
return changed;
}
-static int is_racy_timestamp(const struct index_state *istate, struct
cache_entry *ce)
+static int is_racy_timestamp(const struct index_state *istate,
+const struct cache_entry *ce)
{
return (!S_ISGITLINK(ce->ce_mode) &&
istate->timestamp.sec &&
@@ -255,7 +256,7 @@ static int is_racy_timestamp(const struct index_state
*istate, struct cache_entr
}
int ie_match_stat(const struct index_state *istate,
- struct cache_entry *ce, struct stat *st,
+ const struct cache_entry *ce, struct stat *st,
unsigned int options)
{
unsigned int changed;
@@ -311,7 +312,8 @@ int ie_match_stat(const struct index_state *istate,
}
int ie_modified(const struct index_state *istate,
- struct cache_entry *ce, struct stat *st, unsigned int options)
+ const struct cache_entry *ce,
+ struct stat *st, unsigned int options)
{
int changed, changed_fs;
--
1.8.3
--
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 1/7] cache: mark cache_entry pointers const
Add const for pointers that are only dereferenced for reading by the
inline functions copy_cache_entry and ce_mode_from_stat. This allows
callers to pass in const pointers.
Signed-off-by: René Scharfe
---
cache.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index 94ca1ac..43a27e7 100644
--- a/cache.h
+++ b/cache.h
@@ -190,7 +190,8 @@ struct cache_entry {
* another. But we never change the name, or the hash state!
*/
#define CE_STATE_MASK (CE_HASHED | CE_UNHASHED)
-static inline void copy_cache_entry(struct cache_entry *dst, struct
cache_entry *src)
+static inline void copy_cache_entry(struct cache_entry *dst,
+ const struct cache_entry *src)
{
unsigned int state = dst->ce_flags & CE_STATE_MASK;
@@ -222,7 +223,8 @@ static inline unsigned int create_ce_mode(unsigned int mode)
return S_IFGITLINK;
return S_IFREG | ce_permissions(mode);
}
-static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned
int mode)
+static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+unsigned int mode)
{
extern int trust_executable_bit, has_symlinks;
if (!has_symlinks && S_ISREG(mode) &&
--
1.8.3
--
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 0/7] unpack-trees: plug memory leak for merges
This series adds const to cache_entry pointers in a lot of places, in order to show that we can free them in unpack_nondirectories, which the last patch finally does. First three easy patches for adding const and splitting a function in two: cache: mark cache_entry pointers const read-cache: mark cache_entry pointers const unpack-trees: factor out dup_entry Then a patch that reduces the side effects of merged_entry: unpack-trees: create working copy of merge entry in merged_entry Another easy const patch: diff-lib, read-tree, unpack-trees: mark cache_entry pointers const And patch that introduces "const struct cache_entry * const *", which may look a bit scary at first: diff-lib, read-tree, unpack-trees: mark cache_entry array paramters const Final patch that plugs a memory leak in unpack_nondirectories. unpack-trees: free cache_entry array members for merges It's basically the same one that Stephen tested a while ago (http://thread.gmane.org/gmane.comp.version-control.git/222887). It's not the only leak that affects cherry-pick; expect more (independent) patches. builtin/read-tree.c | 5 +- cache.h | 10 ++-- diff-lib.c | 26 +- read-cache.c| 18 --- unpack-trees.c | 146 unpack-trees.h | 14 +++-- 6 files changed, 131 insertions(+), 88 deletions 1.8.3 -- 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
