Re: What's cooking in git.git (May 2013, #09; Wed, 29)
Junio C Hamano gitster at 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 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
[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 j...@kdbg.org
---
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
[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 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 -# # Clone 2 (conflicting merge): # # A1--A2--B3 -- origin/master @@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL # \--A3
[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 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 v8 4/6] contrib: related: add option to parse from committish
For example master..feature-a.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/related/git-related | 38 --
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/contrib/related/git-related b/contrib/related/git-related
index 0de1c6c..3573237 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -7,10 +7,12 @@ require 'optparse'
$since = '5-years-ago'
$min_percent = 10
+$files = []
+$rev_args = []
OptionParser.new do |opts|
opts.program_name = 'git related'
- opts.banner = 'usage: git related [options] files'
+ opts.banner = 'usage: git related [options] files | rev-list options'
opts.on('-p', '--min-percent N', Integer,
'Minium percentage of role participation') do |v|
@@ -118,10 +120,42 @@ class Commits
end
end
+ def from_rev_args(args)
+source = nil
+File.popen(%w[git rev-list --reverse] + args) do |p|
+ p.each do |e|
+id = e.chomp
+@main_commits[id] = true
+File.popen(%w[git show -C --oneline] + [id]) do |p|
+ p.each do |e|
+case e
+when /^---\s+(\S+)/
+ source = $1 != '/dev/null' ? $1[2..-1] : nil
+when /^@@ -(\d+)(?:,(\d+))?/
+ get_blame(source, $1, $2, id) if source
+end
+ end
+end
+ end
+end
+ end
+
+end
+
+ARGV.each do |e|
+ if File.exists?(e)
+$files e
+ else
+$rev_args e
+ end
end
commits = Commits.new
-commits.from_patches(ARGV)
+if $files.empty?
+ commits.from_rev_args($rev_args)
+else
+ commits.from_patches($files)
+end
commits.import
count_per_person = Hash.new(0)
--
1.8.3.358.g5a91d05
--
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 v8 2/6] contrib: related: add option parsing
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/related/git-related | 16
1 file changed, 16 insertions(+)
diff --git a/contrib/related/git-related b/contrib/related/git-related
index 1b9b1e7..bde5b99 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -3,9 +3,25 @@
# This script finds people that might be interested in a patch
# usage: git related file
+require 'optparse'
+
$since = '5-years-ago'
$min_percent = 10
+OptionParser.new do |opts|
+ opts.program_name = 'git related'
+ opts.banner = 'usage: git related [options] file'
+
+ opts.on('-p', '--min-percent N', Integer,
+ 'Minium percentage of role participation') do |v|
+$min_percent = v
+ end
+ opts.on('-d', '--since DATE',
+ 'How far back to search for relevant commits') do |v|
+$since = v
+ end
+end.parse!
+
class Commit
attr_reader :persons
--
1.8.3.358.g5a91d05
--
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 v8 0/6] New git-related helper
Hi, Since there hasn't been any comments of importance this is basically the same as v7, plus a few other patches to make it actually usable (for me). Unfortunately it turns out Ruby's optparse is not suitable for our needs, so I implemented a very small parser that is. Felipe Contreras (6): Add new git-related helper to contrib contrib: related: add option parsing contrib: related: add support for multiple patches contrib: related: add option to parse from committish contrib: related: parse committish like format-patch contrib: related: implement custom option parser contrib/related/git-related | 247 1 file changed, 247 insertions(+) create mode 100755 contrib/related/git-related -- 1.8.3.358.g5a91d05 -- 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 v8 6/6] contrib: related: implement custom option parser
Ruby's option parser is not extensible enough to keep unknown options.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/related/git-related | 83 ++---
1 file changed, 71 insertions(+), 12 deletions(-)
diff --git a/contrib/related/git-related b/contrib/related/git-related
index 585572b..c933898 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -3,26 +3,85 @@
# This script finds people that might be interested in a patch
# usage: git related file
-require 'optparse'
-
$since = '5-years-ago'
$min_percent = 10
$files = []
$rev_args = []
-OptionParser.new do |opts|
- opts.program_name = 'git related'
- opts.banner = 'usage: git related [options] files | rev-list options'
+class SimpleParser
+ attr_writer :usage
+
+ class Option
+attr_reader :short, :long, :help
+
+def initialize(short, long, help, block)
+ @block = block
+ @short = short
+ @long = long
+ @help = help
+end
+
+def call(v)
+ @block.call(v)
+end
+ end
- opts.on('-p', '--min-percent N', Integer,
- 'Minium percentage of role participation') do |v|
-$min_percent = v
+ def initialize
+@list = {}
+ end
+
+ def on(short=nil, long=nil, help=nil, block)
+opt = Option.new(short, long, help, block)
+@list[short] = opt if short
+@list[long] = opt if long
+ end
+
+ def parse
+i = 0
+if ARGV.member?('-h') or ARGV.member?('--help')
+ usage
+ exit 1
+end
+while cur = ARGV[i] do
+ if cur =~ /^(-.+?)(?:=(.*))?$/
+opt = @list[$1]
+if opt
+ v = $2 || ARGV.delete_at(i + 1)
+ opt.call(v)
+ ARGV.delete_at(i)
+ next
+end
+ end
+ i += 1
+end
end
- opts.on('-d', '--since DATE',
- 'How far back to search for relevant commits') do |v|
-$since = v
+
+ def usage
+puts 'usage: %s' % @usage
+@list.values.uniq.each do |opt|
+ s = ''
+ s [opt.short, opt.long].compact.join(', ')
+ s '%*s%s' % [26 - s.size, '', opt.help] if opt.help
+ puts s
+end
end
-end.parse!
+
+end
+
+opts = SimpleParser.new
+opts.usage = 'usage: git related [options] files | rev-list options'
+
+opts.on('-p', '--min-percent',
+'Minium percentage of role participation') do |v|
+ $min_percent = v.to_i
+end
+
+opts.on('-d', '--since',
+'How far back to search for relevant commits') do |v|
+ $since = v
+end
+
+opts.parse
class Commit
--
1.8.3.358.g5a91d05
--
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 v8 3/6] contrib: related: add support for multiple patches
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/related/git-related | 35 +++
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/contrib/related/git-related b/contrib/related/git-related
index bde5b99..0de1c6c 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -10,7 +10,7 @@ $min_percent = 10
OptionParser.new do |opts|
opts.program_name = 'git related'
- opts.banner = 'usage: git related [options] file'
+ opts.banner = 'usage: git related [options] files'
opts.on('-p', '--min-percent N', Integer,
'Minium percentage of role participation') do |v|
@@ -56,6 +56,7 @@ class Commits
def initialize
@items = {}
+@main_commits = {}
end
def size
@@ -91,23 +92,27 @@ class Commits
p.each do |line|
if line =~ /^\h{40}/
id = $
- @items[id] = Commit.new(id)
+ @items[id] = Commit.new(id) if not @main_commits.include?(id)
end
end
end
end
- def from_patch(file)
-from = source = nil
-File.open(file) do |f|
- f.each do |line|
-case line
-when /^From (\h+) (.+)$/
- from = $1
-when /^---\s+(\S+)/
- source = $1 != '/dev/null' ? $1[2..-1] : nil
-when /^@@ -(\d+)(?:,(\d+))?/
- get_blame(source, $1, $2, from) if source and from
+ def from_patches(files)
+source = nil
+files.each do |file|
+ from = nil
+ File.open(file) do |f|
+f.each do |line|
+ case line
+ when /^From (\h+) (.+)$/
+from = $1
+@main_commits[from] = true
+ when /^---\s+(\S+)/
+source = $1 != '/dev/null' ? $1[2..-1] : nil
+ when /^@@ -(\d+)(?:,(\d+))?/
+get_blame(source, $1, $2, from) if source and from
+ end
end
end
end
@@ -115,10 +120,8 @@ class Commits
end
-exit 1 if ARGV.size != 1
-
commits = Commits.new
-commits.from_patch(ARGV[0])
+commits.from_patches(ARGV)
commits.import
count_per_person = Hash.new(0)
--
1.8.3.358.g5a91d05
--
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 v9 0/4] New git-related helper
Hi, I changed my mind, this is the same as v9 but minus some potentially controversial changes. Felipe Contreras (4): Add new git-related helper to contrib contrib: related: add support for multiple patches contrib: related: add option to parse from committish contrib: related: parse committish like format-patch contrib/related/git-related | 172 1 file changed, 172 insertions(+) create mode 100755 contrib/related/git-related -- 1.8.3.358.g5a91d05 -- 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 v9 4/4] contrib: related: parse committish like format-patch
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/related/git-related | 15 +++ 1 file changed, 15 insertions(+) diff --git a/contrib/related/git-related b/contrib/related/git-related index 20eb456..bded6f6 100755 --- a/contrib/related/git-related +++ b/contrib/related/git-related @@ -105,6 +105,21 @@ class Commits end def from_rev_args(args) +revs = [] + +File.popen(%w[git rev-parse --revs-only --default HEAD --symbolic] + args).each do |rev| + revs rev.chomp +end + +case revs.size +when 1 + r = revs[0] + r = '^' + r if r[0] != '-' + args = [ r, 'HEAD' ] +else + args = revs +end + source = nil File.popen(%w[git rev-list --reverse] + args) do |p| p.each do |e| -- 1.8.3.358.g5a91d05 -- 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 v9 1/4] Add new git-related helper to contrib
This script find people that might be interested in a patch, by going
back through the history for each single hunk modified, and finding
people that reviewed, acknowledge, signed, or authored the code the
patch is modifying.
It does this by running 'git blame' incrementally on each hunk, and then
parsing the commit message. After gathering all the relevant people, it
groups them to show what exactly was their role when the participated in
the development of the relevant commit, and on how many relevant commits
they participated. They are only displayed if they pass a minimum
threshold of participation.
For example:
% git related 0001-remote-hg-trivial-cleanups.patch
Felipe Contreras felipe.contre...@gmail.com
Jeff King p...@peff.net
Max Horn m...@quendi.de
Junio C Hamano gits...@pobox.com
Thus it can be used for 'git send-email' as a cc-cmd.
There might be some other related functions to this script, not just to
be used as a cc-cmd.
Comments-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/related/git-related | 120
1 file changed, 120 insertions(+)
create mode 100755 contrib/related/git-related
diff --git a/contrib/related/git-related b/contrib/related/git-related
new file mode 100755
index 000..1b9b1e7
--- /dev/null
+++ b/contrib/related/git-related
@@ -0,0 +1,120 @@
+#!/usr/bin/env ruby
+
+# This script finds people that might be interested in a patch
+# usage: git related file
+
+$since = '5-years-ago'
+$min_percent = 10
+
+class Commit
+
+ attr_reader :persons
+
+ def initialize(id)
+@id = id
+@persons = []
+ end
+
+ def parse(data)
+msg = nil
+data.each_line do |line|
+ if not msg
+case line
+when /^author ([^]+) (\S+) (.+)$/
+ @persons '%s %s' % [$1, $2]
+when /^$/
+ msg = true
+end
+ else
+if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^]+) (\S+?)$/
+ @persons '%s %s' % [$2, $3]
+end
+ end
+end
+@persons.uniq!
+ end
+
+end
+
+class Commits
+
+ def initialize
+@items = {}
+ end
+
+ def size
+@items.size
+ end
+
+ def each(block)
+@items.each(block)
+ end
+
+ def import
+return if @items.empty?
+File.popen(%w[git cat-file --batch], 'r+') do |p|
+ p.write(@items.keys.join(\n))
+ p.close_write
+ p.each do |line|
+if line =~ /^(\h{40}) commit (\d+)/
+ id, len = $1, $2
+ data = p.read($2.to_i)
+ @items[id].parse(data)
+end
+ end
+end
+ end
+
+ def get_blame(source, start, len, from)
+return if len == 0
+len ||= 1
+File.popen(['git', 'blame', '--incremental', '-C', '-C',
+ '-L', '%u,+%u' % [start, len],
+ '--since', $since, from + '^',
+ '--', source]) do |p|
+ p.each do |line|
+if line =~ /^\h{40}/
+ id = $
+ @items[id] = Commit.new(id)
+end
+ end
+end
+ end
+
+ def from_patch(file)
+from = source = nil
+File.open(file) do |f|
+ f.each do |line|
+case line
+when /^From (\h+) (.+)$/
+ from = $1
+when /^---\s+(\S+)/
+ source = $1 != '/dev/null' ? $1[2..-1] : nil
+when /^@@ -(\d+)(?:,(\d+))?/
+ get_blame(source, $1, $2, from) if source and from
+end
+ end
+end
+ end
+
+end
+
+exit 1 if ARGV.size != 1
+
+commits = Commits.new
+commits.from_patch(ARGV[0])
+commits.import
+
+count_per_person = Hash.new(0)
+
+commits.each do |id, commit|
+ commit.persons.each do |person|
+count_per_person[person] += 1
+ end
+end
+
+count_per_person.each do |person, count|
+ percent = count.to_f * 100 / commits.size
+ next if percent $min_percent
+ puts person
+end
--
1.8.3.358.g5a91d05
--
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 v9 3/4] contrib: related: add option to parse from committish
For example master..feature-a. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/related/git-related | 38 -- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/contrib/related/git-related b/contrib/related/git-related index 3379982..20eb456 100755 --- a/contrib/related/git-related +++ b/contrib/related/git-related @@ -1,10 +1,12 @@ #!/usr/bin/env ruby # This script finds people that might be interested in a patch -# usage: git related files +# usage: git related files | rev-list options $since = '5-years-ago' $min_percent = 10 +$files = [] +$rev_args = [] class Commit @@ -102,10 +104,42 @@ class Commits end end + def from_rev_args(args) +source = nil +File.popen(%w[git rev-list --reverse] + args) do |p| + p.each do |e| +id = e.chomp +@main_commits[id] = true +File.popen(%w[git show -C --oneline] + [id]) do |p| + p.each do |e| +case e +when /^---\s+(\S+)/ + source = $1 != '/dev/null' ? $1[2..-1] : nil +when /^@@ -(\d+)(?:,(\d+))?/ + get_blame(source, $1, $2, id) if source +end + end +end + end +end + end + +end + +ARGV.each do |e| + if File.exists?(e) +$files e + else +$rev_args e + end end commits = Commits.new -commits.from_patches(ARGV) +if $files.empty? + commits.from_rev_args($rev_args) +else + commits.from_patches($files) +end commits.import count_per_person = Hash.new(0) -- 1.8.3.358.g5a91d05 -- 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 v9 2/4] contrib: related: add support for multiple patches
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/related/git-related | 35 +++
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/contrib/related/git-related b/contrib/related/git-related
index 1b9b1e7..3379982 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -1,7 +1,7 @@
#!/usr/bin/env ruby
# This script finds people that might be interested in a patch
-# usage: git related file
+# usage: git related files
$since = '5-years-ago'
$min_percent = 10
@@ -40,6 +40,7 @@ class Commits
def initialize
@items = {}
+@main_commits = {}
end
def size
@@ -75,23 +76,27 @@ class Commits
p.each do |line|
if line =~ /^\h{40}/
id = $
- @items[id] = Commit.new(id)
+ @items[id] = Commit.new(id) if not @main_commits.include?(id)
end
end
end
end
- def from_patch(file)
-from = source = nil
-File.open(file) do |f|
- f.each do |line|
-case line
-when /^From (\h+) (.+)$/
- from = $1
-when /^---\s+(\S+)/
- source = $1 != '/dev/null' ? $1[2..-1] : nil
-when /^@@ -(\d+)(?:,(\d+))?/
- get_blame(source, $1, $2, from) if source and from
+ def from_patches(files)
+source = nil
+files.each do |file|
+ from = nil
+ File.open(file) do |f|
+f.each do |line|
+ case line
+ when /^From (\h+) (.+)$/
+from = $1
+@main_commits[from] = true
+ when /^---\s+(\S+)/
+source = $1 != '/dev/null' ? $1[2..-1] : nil
+ when /^@@ -(\d+)(?:,(\d+))?/
+get_blame(source, $1, $2, from) if source and from
+ end
end
end
end
@@ -99,10 +104,8 @@ class Commits
end
-exit 1 if ARGV.size != 1
-
commits = Commits.new
-commits.from_patch(ARGV[0])
+commits.from_patches(ARGV)
commits.import
count_per_person = Hash.new(0)
--
1.8.3.358.g5a91d05
--
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 v8 0/6] New git-related helper
On Fri, May 31, 2013 at 2:37 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Since there hasn't been any comments of importance this is basically the same as v7, plus a few other patches to make it actually usable (for me). Unfortunately it turns out Ruby's optparse is not suitable for our needs, so I implemented a very small parser that is. Nevermind, we can drop the option parsing changes for later, it's not important at the moment. -- 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
On Thu, May 30, 2013 at 6:31 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, May 30, 2013 at 4:01 AM, Ramkumar Ramachandra 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. I was going to make these stylistic changes to make you happy, but then I realized the only that does really make sense is to change msg = nil to msg = false, and it's not even worth to waste a thought on changes like that. So I'll move on to the next patches, hopefully Duy or Junio would have some comments of actual significance, or maybe you would too, but for the moment it seems pretty clear you are only stating opinions about what Ruby code-style you like best. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] Add new git-related helper to contrib
Felipe Contreras wrote: I was going to make these stylistic changes to make you happy, but then I realized the only that does really make sense is to change msg = nil to msg = false, and it's not even worth to waste a thought on changes like that. We don't have existing Ruby code in git.git to follow, so what I say can obviously not have more weightage than personal opinion. Don't do things to make me happy; I am nobody. Have a good sense of style and defend it instead of flaming me because you thought I was stalling work. Some of these style changes seem to make sense, and I'll fix them after this patch gets merged. or I have a different opinion on these issues, and here's why: would have both been appropriate responses. So I'll move on to the next patches, hopefully Duy or Junio would have some comments of actual significance, or maybe you would too, but for the moment it seems pretty clear you are only stating opinions about what Ruby code-style you like best. I've done reviews of several iterations of this patch, and stylistic comments were all that I had left: I might not be adequately intelligent to come up with comments of actual significance, but that's a limitation you're going to have to deal with. -- 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}
Jeff King p...@peff.net writes:
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.
I don't think so, but admittedly I didn't measure it.
The reason why it's unlikely is that this is specific to
lookup_commit_reference_gently, which according to some grepping is
usually done on refs or values that refs might have; e.g. on the oldnew
sides of a fetch in remote.c, or in many places in the callback of some
variant of for_each_ref.
Of course if you have a ridiculously large number of refs (and I gather
_you_ do), this will hurt somewhat in the usual case, but speed up the
case where there is a ref (usually a lightweight tag) directly pointing
at a large blob.
I'm not sure this can be fixed without the change you outline here:
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).
But in any case I see the point, I should try and gather some
performance numbers.
--
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 Fri, May 31, 2013 at 3:03 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: I was going to make these stylistic changes to make you happy, but then I realized the only that does really make sense is to change msg = nil to msg = false, and it's not even worth to waste a thought on changes like that. We don't have existing Ruby code in git.git to follow, so what I say can obviously not have more weightage than personal opinion. Don't do things to make me happy; I am nobody. Have a good sense of style and defend it instead of flaming me because you thought I was stalling work. I already defended the style, only to hear the same comment again in the next iteration. So I'll move on to the next patches, hopefully Duy or Junio would have some comments of actual significance, or maybe you would too, but for the moment it seems pretty clear you are only stating opinions about what Ruby code-style you like best. I've done reviews of several iterations of this patch, and stylistic comments were all that I had left: When a reviewer reaches that point, (s)he usually says: other than cosmetic preferences: Reviewed-by: me. -- 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
On 30 May 2013 20:30, John Keeping j...@keeping.me.uk wrote:
On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote:
Alex Bennée kernel-hac...@bennee.com writes:
On 30 May 2013 16:33, Thomas Rast tr...@inf.ethz.ch wrote:
Alex Bennée kernel-hac...@bennee.com writes:
snip
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.
Wait is this the difference between annotated and non-annotated tags?
I thought a non-annotated just acted like references to a particular
tree state?
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.
Hmm that didn't seem to work. But looking at the output by hand I
certainly have a mix of tags that are commits vs tags:
09:08 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep commit | wc -l
1345
09:12 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep -v commit | wc -l
66
Unfortunately I can't just delete those tags as they do refer to known
releases which we obviously care about. If I delete the tags on my
local repo and test for a speed increase can I re-create them as
annotated tag objects?
--
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 2/2] lookup_commit_reference_gently: do not read non-{tag,commit}
Ramkumar Ramachandra artag...@gmail.com writes:
Thomas Rast wrote:
+ 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);
[...]
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).
Yes, it does fix the bug. (It's not really buggy, just slow.)
However, you implicitly point out an important point: If we have the
object, and it was already parsed (obj-parsed is set), parse_object()
is essentially free. But sha1_object_info is not, it will in particular
unconditionally dig through long delta chains to discover the base type
of an object that has already been unpacked.
As for your original questions: lookup_object() is do we have it in our
big object hashtable? -- the one that holds many[1] objects, that Peff
recently sped up.
sha1_object_info() and read_object() are in many ways parallel functions
that do approximately the following:
check all pack indexes for this object
if we found a hit:
attempt to unpack by recursively going through deltas
(for _info, no need to unpack, but we still go through the delta
chain because the type of object is determined by the innermost
delta base)
try to load it as a loose object
it could have been repacked and pruned while we were looking, so:
reload pack index information
try the packs again (search indexes, then unpack)
complain
[1] blobs in particular are frequently not stored in that hash table,
because it is an insert-only table
--
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 v2 1/3] read-cache: plug a few leaks
On Thu, May 30, 2013 at 10:13 AM, René Scharfe rene.scha...@lsrfire.ath.cx 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 felipe.contre...@gmail.com --- 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? It would be enough if every discard_cache() is not followed by a read_cache() after adding entries. I was adding a init_index() helper, but it turns out only very few places initialize the index, and all of them zero the structure (or declare it so it's zeroed on load), so I think this change is safe like that. Also, I don't see any place manually doing initialize=0. -- 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 kernel-hac...@bennee.com writes:
On 30 May 2013 20:30, John Keeping j...@keeping.me.uk wrote:
On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote:
Alex Bennée kernel-hac...@bennee.com writes:
On 30 May 2013 16:33, Thomas Rast tr...@inf.ethz.ch wrote:
Alex Bennée kernel-hac...@bennee.com writes:
snip
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.
Wait is this the difference between annotated and non-annotated tags?
I thought a non-annotated just acted like references to a particular
tree state?
A tag is just a ref. It can point at anything, in particular also a
blob (= some file *contents*).
An annotated tag is just a tag pointing at a tag object. A tag object
contains tagger name/email/date, a reference to an object, and a tag
message.
The slowness I found relates to having tags that point at blobs directly
(unannotated).
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.
Hmm that didn't seem to work. But looking at the output by hand I
certainly have a mix of tags that are commits vs tags:
09:08 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep commit | wc -l
1345
09:12 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep -v commit | wc -l
66
Unfortunately I can't just delete those tags as they do refer to known
releases which we obviously care about. If I delete the tags on my
local repo and test for a speed increase can I re-create them as
annotated tag objects?
I would be more interested in this:
git for-each-ref | grep ' blob'
and
(git for-each-ref | grep ' blob' | cut -d\ -f1 | xargs -n1 git cat-file
blob) | wc -c
The first tells you if you have any refs pointing at blobs. The second
computes their total unpacked size. My theory is that the second yields
some large number (hundreds of megabytes at least).
It would be nice if you checked, because if there turn out to be big
blobs, we have all the pieces and just need to assemble the best
solution. Otherwise, there's something else going on and the problem
remains open.
--
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
[PATCH] submodule: remove unnecessary check
read_cache() already does that check. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- submodule.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/submodule.c b/submodule.c index e728025..1821a5b 100644 --- a/submodule.c +++ b/submodule.c @@ -603,9 +603,8 @@ int fetch_populated_submodules(const struct argv_array *options, if (!work_tree) goto out; - if (!the_index.initialized) - if (read_cache() 0) - die(index file corrupt); + if (read_cache() 0) + die(index file corrupt); argv_array_push(argv, fetch); for (i = 0; i options-argc; i++) -- 1.8.3.358.g5a91d05 -- 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 Fri, May 31, 2013 at 09:14:49AM +0100, Alex Bennée wrote:
On 30 May 2013 20:30, John Keeping j...@keeping.me.uk wrote:
On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote:
Alex Bennée kernel-hac...@bennee.com writes:
On 30 May 2013 16:33, Thomas Rast tr...@inf.ethz.ch wrote:
Alex Bennée kernel-hac...@bennee.com writes:
snip
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.
Wait is this the difference between annotated and non-annotated tags?
I thought a non-annotated just acted like references to a particular
tree state?
No, this is something slightly different. In Git there are four types
of object: tag, commit, tree and blob. When you have a heavyweight tag,
the tag reference points at a tag object (which in turn points at
another object). With a lightweight tag, the tag reference typically
points at a commit object.
However, there is no restriction that says that a tag object must point
to a commit or that a lightweight tag must point at a commit - it is
equally possible to point directly at a tree or a blob (although a lot
less common).
Thomas is suggesting that you might have a tag that does not point at a
commit but instead points to a blob object.
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.
Hmm that didn't seem to work.
You mean there was no output? In that case it's likely that all your
references do indeed point at commits.
But looking at the output by hand I
certainly have a mix of tags that are commits vs tags:
09:08 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep commit | wc -l
1345
09:12 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep -v commit | wc -l
66
This means that you have 1345 lightweight tags and 66 heavyweight tags,
assuming that all of the lines that don't say commit do say tag.
By the way, I don't remember if you said which version of Git you're
using. If it's an older version then it's possible that something has
changed.
--
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 31 May 2013 09:24, Thomas Rast tr...@inf.ethz.ch wrote: Alex Bennée kernel-hac...@bennee.com writes: On 30 May 2013 20:30, John Keeping j...@keeping.me.uk wrote: On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote: Alex Bennée kernel-hac...@bennee.com writes: On 30 May 2013 16:33, Thomas Rast tr...@inf.ethz.ch wrote: snip No, my theory is that you tagged *the blobs*. Git supports this. Wait is this the difference between annotated and non-annotated tags? I thought a non-annotated just acted like references to a particular tree state? A tag is just a ref. It can point at anything, in particular also a blob (= some file *contents*). An annotated tag is just a tag pointing at a tag object. A tag object contains tagger name/email/date, a reference to an object, and a tag message. The slowness I found relates to having tags that point at blobs directly (unannotated). I think you are right. I was brave (well I assumed the tags would come back from the upstream repo) and ran: git for-each-ref | grep refs/tags | grep commit | cut -d '/' -f 3 | xargs git tag -d And boom: 09:19 ajb@sloy/x86_64 [work.git] time /usr/bin/git --no-pager describe --long --tags ajb-build-test-5225-2-gdc0b771 real0m0.009s user0m0.008s sys 0m0.000s Which is much better performance. So it does look like unannotated tags pointing at binary blobs is the failure case. snip I would be more interested in this: git for-each-ref | grep ' blob' Hmmm that gives nothing. All the refs are either tag or commit and (git for-each-ref | grep ' blob' | cut -d\ -f1 | xargs -n1 git cat-file blob) | wc -c However I have some big commits it seems: 09:37 ajb@sloy/x86_64 [work.git] (git for-each-ref | grep ' commit' | cut -d\ -f1 | xargs -n1 git cat-file commit) | wc -c 1147231984 The first tells you if you have any refs pointing at blobs. The second computes their total unpacked size. My theory is that the second yields some large number (hundreds of megabytes at least). It would be nice if you checked, because if there turn out to be big blobs, we have all the pieces and just need to assemble the best solution. Otherwise, there's something else going on and the problem remains open. If you want any other numbers I'm only too happy to help. Sorry I can't share the repo though... -- 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
Alex Bennée kernel-hac...@bennee.com writes:
I think you are right. I was brave (well I assumed the tags would come
back from the upstream repo) and ran:
git for-each-ref | grep refs/tags | grep commit | cut -d '/' -f 3
| xargs git tag -d
So that deleted all unannotated tags pointing at commits, and then it
was fast. Curious.
However I have some big commits it seems:
09:37 ajb@sloy/x86_64 [work.git] (git for-each-ref | grep ' commit' |
cut -d\ -f1 | xargs -n1 git cat-file commit) | wc -c
1147231984
How many unique entries are there in that list, i.e., what does
git for-each-ref | grep ' commit' | cut -d\ -f1 | sort -u | wc -l
say? Perhaps you can also find the biggest commit, e.g. like so:
git for-each-ref | grep ' commit' | cut -d\ -f1 |
while read sha; do git cat-file commit $sha | wc -c; done |
sort -n
However, if that turns out to be the culprit, it's not fixable
currently[1]. Having commits with insanely long messages is just, well,
insane.
[1] unless we do a major rework of the loading infrastructure, so that
we can teach it to load only the beginning of a commit as long as we are
only interested in parents and such
--
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 31 May 2013 09:32, John Keeping j...@keeping.me.uk wrote:
On Fri, May 31, 2013 at 09:14:49AM +0100, Alex Bennée wrote:
On 30 May 2013 20:30, John Keeping j...@keeping.me.uk wrote:
On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote:
Alex Bennée kernel-hac...@bennee.com writes:
On 30 May 2013 16:33, Thomas Rast tr...@inf.ethz.ch wrote:
Alex Bennée kernel-hac...@bennee.com writes:
snip
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.
Wait is this the difference between annotated and non-annotated tags?
I thought a non-annotated just acted like references to a particular
tree state?
No, this is something slightly different. In Git there are four types
of object: tag, commit, tree and blob. When you have a heavyweight tag,
the tag reference points at a tag object (which in turn points at
another object). With a lightweight tag, the tag reference typically
points at a commit object.
I think this is the case in my repo.
However, there is no restriction that says that a tag object must point
to a commit or that a lightweight tag must point at a commit - it is
equally possible to point directly at a tree or a blob (although a lot
less common).
Thomas is suggesting that you might have a tag that does not point at a
commit but instead points to a blob object.
It's looking like I just have some very heavy commits. One data point
I probably should have mentioned at the beginning is this was a
converted CVS repo and I'm wondering if some of the artifacts that
introduced has contributed to 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.
Hmm that didn't seem to work.
You mean there was no output? In that case it's likely that all your
references do indeed point at commits.
Correct.
But looking at the output by hand I
certainly have a mix of tags that are commits vs tags:
09:08 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep commit | wc -l
1345
09:12 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep -v commit | wc -l
66
This means that you have 1345 lightweight tags and 66 heavyweight tags,
assuming that all of the lines that don't say commit do say tag.
Yep all commits and tags, nothing else
By the way, I don't remember if you said which version of Git you're
using. If it's an older version then it's possible that something has
changed.
I'm running the GIT stable PPA:
09:38 ajb@sloy/x86_64 [work.git] git --version
git version 1.8.3
Although I have also tested with the latest git.git maint. I'm happy
to try master if it's likely to have changed.
--
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
On Fri, May 31, 2013 at 09:49:57AM +0100, Alex Bennée wrote: On 31 May 2013 09:32, John Keeping j...@keeping.me.uk wrote: Thomas is suggesting that you might have a tag that does not point at a commit but instead points to a blob object. It's looking like I just have some very heavy commits. One data point I probably should have mentioned at the beginning is this was a converted CVS repo and I'm wondering if some of the artifacts that introduced has contributed to this. You can try another for-each-ref invocation to see if that's the case: eval $(git for-each-ref --format 'printf %s %s\n \ $(git cat-file -s %(objectname)) %(refname);') | sort -n That will print the size of each object followed by the ref that points to it, sorted by size. I'm running the GIT stable PPA: 09:38 ajb@sloy/x86_64 [work.git] git --version git version 1.8.3 Although I have also tested with the latest git.git maint. I'm happy to try master if it's likely to have changed. master's still very close to 1.8.3 at the moment, so I don't think that will make a difference. -- 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 31 May 2013 09:46, Thomas Rast tr...@inf.ethz.ch wrote:
Alex Bennée kernel-hac...@bennee.com writes:
I think you are right. I was brave (well I assumed the tags would come
back from the upstream repo) and ran:
git for-each-ref | grep refs/tags | grep commit | cut -d '/' -f 3
| xargs git tag -d
So that deleted all unannotated tags pointing at commits, and then it
was fast. Curious.
However I have some big commits it seems:
09:37 ajb@sloy/x86_64 [work.git] (git for-each-ref | grep ' commit' |
cut -d\ -f1 | xargs -n1 git cat-file commit) | wc -c
1147231984
How many unique entries are there in that list, i.e., what does
git for-each-ref | grep ' commit' | cut -d\ -f1 | sort -u | wc -l
09:49 ajb@sloy/x86_64 [work.git] git for-each-ref | grep ' commit' |
cut -d\ -f1 | sort -u | wc -l
1508
say? Perhaps you can also find the biggest commit, e.g. like so:
git for-each-ref | grep ' commit' | cut -d\ -f1 |
while read sha; do git cat-file commit $sha | wc -c; done |
sort -n
Yeah there is a range from a few hundred bytes to a large number of 3M
commits. I guess I need to identify which commits they are and remove
the tags or convert them to annotated reference tags.
However, if that turns out to be the culprit, it's not fixable
currently[1]. Having commits with insanely long messages is just, well,
insane.
[1] unless we do a major rework of the loading infrastructure, so that
we can teach it to load only the beginning of a commit as long as we are
only interested in parents and such
I'll do a bit of scripting to dig into the nature of these
uber-commits and try and work out how they cam about. I suspect they
are simply start of branch states in our broken and disparate history.
I'll get back to you once I've dug a little deeper.
--
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: Poor performance of git describe in big repos
Thomas Rast tr...@inf.ethz.ch writes:
However, if that turns out to be the culprit, it's not fixable
currently[1]. Having commits with insanely long messages is just, well,
insane.
[1] unless we do a major rework of the loading infrastructure, so that
we can teach it to load only the beginning of a commit as long as we are
only interested in parents and such
Actually, Peff, doesn't your commit parent/tree pointer caching give us
this for free?
--
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
[PATCH 2/2] diffcore-pickaxe doc: document -S and -G properly
The documentation of -S and -G is very sketchy. Completely rewrite the sections in Documentation/diff-options.txt and Documentation/gitdiffcore.txt. References: 52e9578 ([PATCH] Introducing software archaeologist's tool pickaxe.) f506b8e (git log/diff: add -Gregexp that greps in the patch text) Inputs-from: Phil Hord phil.h...@gmail.com Co-authored-by: Junio C Hamano gits...@pobox.com Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Documentation/diff-options.txt | 38 +++ Documentation/gitdiffcore.txt | 45 +- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index b8a9b86..a85288f 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -383,14 +383,36 @@ ifndef::git-format-patch[] that matches other criteria, nothing is selected. -Sstring:: - Look for differences that introduce or remove an instance of - string. Note that this is different than the string simply - appearing in diff output; see the 'pickaxe' entry in - linkgit:gitdiffcore[7] for more details. + Look for differences that change the number of occurrences of + the specified string (i.e. addition/deletion) in a file. + Intended for the scripter's use. ++ +It is useful when you're looking for an exact block of code (like a +struct), and want to know the history of that block since it first +came into being: use the feature iteratively to feed the interesting +block in the preimage back into `-S`, and keep going until you get the +very first version of the block. -Gregex:: - Look for differences whose added or removed line matches - the given regex. + Look for differences whose patch text contains added/removed + lines that match regex. ++ +To illustrate the difference between `-Sregex --pickaxe-regex` and +`-Gregex`, consider a commit with the following diff in the same +file: ++ + ++return !regexec(regexp, two-ptr, 1, regmatch, 0); +... +-hit = !regexec(regexp, mf2.ptr, 1, regmatch, 0); + ++ +While `git log -Gregexec\(regexp` will show this commit, `git log +-Sregexec\(regexp --pickaxe-regex` will not (because the number of +occurrences of that string did not change). ++ +See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more +information. --pickaxe-all:: When `-S` or `-G` finds a change, show all the changes in that @@ -398,8 +420,8 @@ ifndef::git-format-patch[] in string. --pickaxe-regex:: - Make the string not a plain string but an extended POSIX - regex to match. + Treat the string given to `-S` as an extended POSIX regular + expression to match. endif::git-format-patch[] -Oorderfile:: diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt index 568d757..c8b3e51 100644 --- a/Documentation/gitdiffcore.txt +++ b/Documentation/gitdiffcore.txt @@ -222,26 +222,35 @@ version prefixed with '+'. diffcore-pickaxe: For Detecting Addition/Deletion of Specified String - -This transformation is used to find filepairs that represent -changes that touch a specified string, and is controlled by the --S option and the `--pickaxe-all` option to the 'git diff-*' -commands. - -When diffcore-pickaxe is in use, it checks if there are -filepairs whose result side and whose origin side have -different number of specified string. Such a filepair represents -the string appeared in this changeset. It also checks for the -opposite case that loses the specified string. - -When `--pickaxe-all` is not in effect, diffcore-pickaxe leaves -only such filepairs that touch the specified string in its -output. When `--pickaxe-all` is used, diffcore-pickaxe leaves all -filepairs intact if there is such a filepair, or makes the -output empty otherwise. The latter behaviour is designed to -make reviewing of the changes in the context of the whole +This transformation limits the set of filepairs to those that change +specified strings between the preimage and the postimage in a certain +way. -Sblock of text and -Gregular expression options are used to +specify different ways these strings are sought. + +-Sblock of text detects filepairs whose preimage and postimage +have different number of occurrences of the specified block of text. +By definition, it will not detect in-file moves. Also, when a +changeset moves a file wholesale without affecting the interesting +string, diffcore-rename kicks in as usual, and `-S` omits the filepair +(since the number of occurrences of that string didn't change in that +rename-detected filepair). When used with `--pickaxe-regex`, treat +the block of text as an extended POSIX regular expression to match, +instead of a literal string. + +-Gregular expression (mnemonic: grep) detects filepairs whose +textual diff
[PATCH 1/2] diffcore-pickaxe: make error messages more consistent
Currently, diffcore-pickaxe reports two distinct errors for the same
user error:
$ git log --pickaxe-regex -S'\1'
fatal: invalid pickaxe regex: Invalid back reference
$ git log -G'\1' # --pickaxe-regex is implied
fatal: invalid log-grep regex: Invalid back reference
Since the error has nothing to do with log-grep, change the -G and -S
error messages to say invalid regex.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
diffcore-pickaxe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 63722f8..c97ac9b 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -122,7 +122,7 @@ static void diffcore_pickaxe_grep(struct diff_options *o)
char errbuf[1024];
regerror(err, regex, errbuf, 1024);
regfree(regex);
- die(invalid log-grep regex: %s, errbuf);
+ die(invalid regex: %s, errbuf);
}
pickaxe(diff_queued_diff, o, regex, NULL, diff_grep);
@@ -246,7 +246,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)
char errbuf[1024];
regerror(err, regex, errbuf, 1024);
regfree(regex);
- die(invalid pickaxe regex: %s, errbuf);
+ die(invalid regex: %s, errbuf);
}
regexp = regex;
} else {
--
1.8.3.114.gcd03571
--
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 v4 0/2] Improve diffcore-pickaxe documentation
Junio had some suggestions in the previous round. The inter-diff
follows.
Yeah, word-diff is a bit messy. Which brings me to: is it possible to
turn on word-diff only where heuristically appropriate? word-diff
applied on the rewrite of the first paragraph of gitdiffcore.txt is a
disaster, but okay everywhere else.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index efb5dfe..a85288f 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -387,11 +387,11 @@ ifndef::git-format-patch[]
the specified string (i.e. addition/deletion) in a file.
Intended for the scripter's use.
+
It is[-especially-] useful when you're looking for an exact block of code (like
a
struct), and want to know the history of that block since it first
came into being: use the feature iteratively to feed the interesting
block in the preimage back into `-S`, and keep going until you get the
very first version of the block.
-Gregex::
Look for differences whose patch text contains added/removed
diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index ef4c04a..c8b3e51 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -222,25 +222,27 @@ version prefixed with '+'.
diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
-
[-There are two kinds of pickaxe:-]{+This transformation limits+} the [-S kind
(corresponding-]{+set of filepairs+} to [-'git log-]
[--S')-]{+those that change+}
{+specified strings between the preimage+} and the [-G kind (mnemonic: grep;
corresponding-]{+postimage in a certain+}
{+way. -Sblock of text and -Gregular expression options are used+} to
[-'git log -G').-]{+specify different ways these strings are sought.+}
-Sblock of text detects filepairs whose preimage and postimage
have different number of occurrences of the specified block of text.
By definition, it will not detect in-file moves. Also, when a
changeset moves a file wholesale without affecting the interesting
string, [-rename detection-]{+diffcore-rename+} kicks in as usual, and `-S`
omits the filepair
(since the number of occurrences of that string didn't change in that
rename-detected filepair).[-The implementation essentially-]
[-runs a count, and is significantly cheaper than the G kind.-] When used with
`--pickaxe-regex`, treat
the block of text as an extended POSIX regular expression to match,
instead of a literal string.
-Gregular expression {+(mnemonic: grep)+} detects filepairs whose
textual diff has an added or a deleted line that matches the given
regular expression. This means that it [-can-]{+will+} detect in-file (or what
rename-detection considers the same file) [-moves.-]{+moves, which is noise.+}
The
implementation runs diff twice and greps, and this can be quite
expensive.
When `-S` or `-G` are used without `--pickaxe-all`, only filepairs
that match their respective criterion are kept in the output. When
Ramkumar Ramachandra (2):
diffcore-pickaxe: make error messages more consistent
diffcore-pickaxe doc: document -S and -G properly
Documentation/diff-options.txt | 38 +++
Documentation/gitdiffcore.txt | 45 +-
diffcore-pickaxe.c | 4 ++--
3 files changed, 59 insertions(+), 28 deletions(-)
--
1.8.3.114.gcd03571
--
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 v3 5/7] add tests for rebasing merged history
Am 31.05.2013 08:49, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i +#\ \ +# e---u +# +# uppercase = cherry-picked +# h = reverted g ... +test_expect_success rebase -p --onto in merged history drops patches in upstream + reset_rebase + git rebase -p --onto f h u + test_cmp_rev f HEAD~3 + test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD + + +test_expect_success rebase -p --onto in merged history does not drop patches in onto + reset_rebase + git rebase -p --onto h f u + test_cmp_rev h HEAD~3 + test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD + I noticed one new aspect: The interdiff between v2 and v3 looks like this: -test_expect_failure rebase -p --onto in merged history does not lose patches in upstream +test_expect_success rebase -p --onto in merged history drops patches in upstream reset_rebase git rebase -p --onto f h u test_cmp_rev f HEAD~3 - test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD + test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD -test_expect_success rebase -p --onto in merged history drops patches in onto +test_expect_success rebase -p --onto in merged history does not drop patches in onto reset_rebase git rebase -p --onto h f u test_cmp_rev h HEAD~3 - test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD + test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD The expectations that these two tests check changed from v2 to v3. Notice that former test goes from expect_failure to expect_success, as it should, but the latter does not change. Strange, isn't it? The reason is that this check is incomplete: test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD and allowed the latter test in the v2 form to pass. It should be test_revision_subjects 'd i d e u' HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD The check of the latter test should be: test_revision_subjects 'd G i d e u' HEAD^2~2 HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD i.e. check all the way back to the mergebase via both branches. This can be extrapolated to all tests that reconstruct mergy history (not just these two cases). -- 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
contributing to git-mediawiki
Hello, We are another team from Ensimag (Célestin MATTE Benoit PERSON) who will contribute to Git and more specifically to Git-Mediawiki for our one-month school project - and possibly more. We already have a couple of basic patches in local and will submit them in the upcoming days. After that, we will start working on more useful features. As a first start, we thought about these two ideas posted in the Git-Mediawiki’s issue tracker: - A mechanism to preview local modifications in the browser (with the wiki style) -- https://github.com/moy/Git-Mediawiki/issues/7 - Support for “push-by-rev” to increase push performance on some wikis -- https://github.com/moy/Git-Mediawiki/issues/6 If you have any suggestions, ideas, guidances, feel free to share them with us. Best regards, Célestin MATTE Benoit PERSON -- 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 10:08:06AM +0200, Thomas Rast wrote: 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. I don't think so, but admittedly I didn't measure it. The reason why it's unlikely is that this is specific to lookup_commit_reference_gently, which according to some grepping is usually done on refs or values that refs might have; e.g. on the oldnew sides of a fetch in remote.c, or in many places in the callback of some variant of for_each_ref. Yeah, I saw that the _gently form backs some of the other forms (non-gently, lookup_commit_or_die) and was worried that we would use it as part of the revision traversal to find parents. But we don't, of course; we use lookup_commit, because we would not accept a parent that is a tag pointing to a commit. So I think it probably won't matter in any sane case. Of course if you have a ridiculously large number of refs (and I gather _you_ do), this will hurt somewhat in the usual case, but speed up the case where there is a ref (usually a lightweight tag) directly pointing at a large blob. In my large-ref cases, there are often a lot of duplicate refs anyway (e.g., many forks of a project having the same tags). So usually the right thing there is to use lookup_object to see if we have the object already anyway. parse_object has this optimization, but we can add it into sha1_object_info, too, if it turns out to be a problem. -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: Poor performance of git describe in big repos
On Fri, May 31, 2013 at 12:27:11PM +0200, Thomas Rast wrote: Thomas Rast tr...@inf.ethz.ch writes: However, if that turns out to be the culprit, it's not fixable currently[1]. Having commits with insanely long messages is just, well, insane. [1] unless we do a major rework of the loading infrastructure, so that we can teach it to load only the beginning of a commit as long as we are only interested in parents and such Actually, Peff, doesn't your commit parent/tree pointer caching give us this for free? It does. You can test it from the jk/metapacks branch at git://github.com/peff/git. After building, you'd need to do: $ git gc $ git metapack --all --commits in the target repository. You can check that it's working because git rev-list --all --count should be an order of magnitude faster. You may need to add save_commit_buffer = 0 in any commands you are checking, though, as the optimization can only kick in if parse_commit does not want to save the buffer as a side effect. I also looked into trying to just read the beginning part of a commit[1], but it turned out not to be all that much of an improvement. -Peff [1] http://article.gmane.org/gmane.comp.version-control.git/212301 -- 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)
On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote: 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. To summarize what I think are the outstanding issues from your email: * Should '$sm_path' be relative in submodule foreach? * submodule add with a relative path * submodule init initializes all submodules * Tests The current version does make '$sm_path' relative in submodule foreach, although it's hard to spot because we have to leave doing so until right before the eval. I'm not sure what you mean about submodule add - the new version treats the path argument as relative (providing it is not an absolute path). The repository argument is not changed by running from a subdirectory but I think that's correct since it is documented as being relative to the superproject's origin repository. submodule init is behaving in the same way as deinit - if you say submodule init . then it will only initialize submodules below the current directory. The difference is that deinit dies if it is not given any arguments whereas init will initialize everything from the top level down. I'm not sure whether to change this; given the direction git add -u is heading in for 2.0 I think the current behaviour is the most consistent with the rest of Git. 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? I think the only thing outstanding is tests. I'm happy to add those as a follow-up or in a re-roll. -- 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
git daemon --access-hook problem
Hi, I'm trying to test this new feature and having problems getting any results in the following scenario: i have a repo in local folder /home/users/myuser/repos/projectA/.git i start the daemon with the following: git daemon --export-all --base-path=/home/users/myuser/repos --enable=receive-pack --access-hook=/home/users/myuser/test_hook.bash test_hook.bash has the following: #!/bin/bash echo $@ test_hook_out.txt echo $REMOTE_ADDR test_hook_out.txt the hook is set to be executable - otherwise it complains when i do anything via git protocol, which proves that it seems to or check the hook: then i did: cd ~/tmp/ git clone git://myhost/projectA projectA cd projectA and trying to perform some operations like fetch or push. It is cloned and fetches and pushes successfully. The problem is that the file test_hook_out.txt doesn't have anything in it after the execution, So the hook doesn't seem to work. What might be the issue here? Thanks, Eugene -- 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
ls-files -i directories
Hi, the gitignore rules work so that if a directory is ignored, all files in that directory are ignored. While that behavior isn't clearly documented in gitignore, this behavior is consistent across all git tools (status, ls-files, ...). An exception is that listing the ignored files using ls-files -i doesn't behave the same way. example: $ mkdir d $ touch d/f $ echo /d/ .gitignore $ git ls-files -o --exclude-standard .gitignore #d/f is correctly not listed $ git ls-files -i --exclude-standard #no output d/f isn't listed even though it is treated as an ignored file by all other git tools. That seems inconsistent to me. Is that behavior intentionally or is this a bug? A very similar question was asked before: http://git.661346.n2.nabble.com/git-ls-files-ignored-and-ignored-directory-tt7570641.html but without an answer. Roland -- ORNL/UT Center for Molecular Biophysics cmb.ornl.gov 865-241-1537, ORNL PO BOX 2008 MS6309 -- 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: ls-files -i directories
On Fri, May 31, 2013 at 04:22:37PM -0400, Roland Schulz wrote: Hi, the gitignore rules work so that if a directory is ignored, all files in that directory are ignored. While that behavior isn't clearly documented in gitignore, this behavior is consistent across all git tools (status, ls-files, ...). An exception is that listing the ignored files using ls-files -i doesn't behave the same way. example: $ mkdir d $ touch d/f $ echo /d/ .gitignore $ git ls-files -o --exclude-standard .gitignore #d/f is correctly not listed $ git ls-files -i --exclude-standard #no output d/f isn't listed even though it is treated as an ignored file by all other git tools. That seems inconsistent to me. Is that behavior intentionally or is this a bug? It is listed with git ls-files -i -o --exclude-standard. The documentation says: Show only ignored files in the output. When showing files in the index, print only those matched by an exclude pattern. When showing other files, show only those matched by an exclude pattern. If you do this then it is shown: $ git add -f d/f $ git ls-files -i --exclude-standard d/f I think this is working as documented. -- 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
can we prevent reflog deletion when branch is deleted?
Hi, Is there a way to prevent reflog deletion when the branch is deleted? The last entry could simply be a line where the second SHA is all 0's. -- Sitaram -- 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: can we prevent reflog deletion when branch is deleted?
On 06/01/2013 03:31 AM, Sitaram Chamarty wrote: Is there a way to prevent reflog deletion when the branch is deleted? The last entry could simply be a line where the second SHA is all 0's. This is a known problem. The technical reason that this is not trivial to solve is the possibility of a directory/file conflict between old reflog files and references that might be created subsequently (which in turn is a limitation of how loose references and reflogs are mapped to filenames): git branch foo git branch -d foo git branch foo/bar Under your proposal, the second line would retain the reflog file for foo, which is named .git/logs/refs/heads/foo. But the third line wants to create a file .git/logs/refs/heads/foo/bar. The existence of the foo file prevents the creation of a foo directory. A similar problem exists if foo and foo/bar are exchanged in the above example. Peff proposed a solution to this problem [1], but AFAIK it is not making progress. Michael [1] http://thread.gmane.org/gmane.comp.version-control.git/201715/focus=201752 -- 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: can we prevent reflog deletion when branch is deleted?
On Sat, Jun 01, 2013 at 05:00:07AM +0200, Michael Haggerty wrote: This is a known problem. The technical reason that this is not trivial to solve is the possibility of a directory/file conflict between old reflog files and references that might be created subsequently (which in turn is a limitation of how loose references and reflogs are mapped to filenames): [...] Peff proposed a solution to this problem [1], but AFAIK it is not making progress. I was running with the patch series you mentioned for a while, but there are some weird bugs with it that need to be tracked down. I don't recall the details, but I would occasionally get error messages that showed that some parts of the code were surprised that the reflog existed without the ref existing. While I think solving the D/F conflict in the ref namespaces overall would be a nice thing to have, doing it with compatibility with the current system is complex and error-prone. I wonder if simply sticking the reflog entries into a big GRAVEYARD reflog wouldn't be a great deal simpler and accomplish the keep deleted reflogs goal, which is what people actually want. -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
