Re: [PATCH 5/5] rebase: fix cherry-pick invocations
Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Felipe Contreras felipe.contre...@gmail.com writes: So that all the tests pass. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-rebase--cherry.sh | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/git-rebase--cherry.sh b/git-rebase--cherry.sh index ca78b1b..c3a2ac9 100644 --- a/git-rebase--cherry.sh +++ b/git-rebase--cherry.sh @@ -23,11 +23,26 @@ test -n $rebase_root root_flag=--root mkdir $state_dir || die Could not create temporary $state_dir : $state_dir/cherry || die Could not mark as cherry +if test -n $rebase_root +then + revisions=$onto...$orig_head +else + revisions=$upstream...$orig_head +fi So that all the tests pass needs a bit more explanation to say for cherry-pick codepath why and how two-dot range fails and why and how three-dot variant with --right-only fixes it. What are the problematic cases? Yikes, sorry, this was me being slow. Walking A...B range with right-only and --cherry applied will filter the duplicates, which is wat you want, I think, and walking A..B range will not do the filtering for you. That's right. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
On Wed, May 29, 2013 at 12:51 AM, Martin von Zweigbergk martinv...@gmail.com wrote: On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: One change splits, the other change fixes, what's wrong with that? I didn't say there was anything wrong. I was asking if the bug was there before (and I didn't see an answer when Junio asked). Why wouldn't it be before? Did I mention a commit that introduced a problem? No. Did any patch in this series introduce a problem? No. All we've done in this series is 1) reorganize the code without introducing *ANY* functional changes, and 2) fix a bug. If you see 1) introducing a problem, or 2) introducing a problem, then mention that in *those* patches. If there is no problem with 1) or 2) then it follows the problem already exists. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
:-) On Tue, May 28, 2013 at 11:05 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 12:51 AM, Martin von Zweigbergk martinv...@gmail.com wrote: On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: One change splits, the other change fixes, what's wrong with that? I didn't say there was anything wrong. I was asking if the bug was there before (and I didn't see an answer when Junio asked). Why wouldn't it be before? Did I mention a commit that introduced a problem? No. Did any patch in this series introduce a problem? No. All we've done in this series is 1) reorganize the code without introducing *ANY* functional changes, and 2) fix a bug. If you see 1) introducing a problem, or 2) introducing a problem, then mention that in *those* patches. If there is no problem with 1) or 2) then it follows the problem already exists. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: zsh: improve bash script loading
Am 5/29/2013 5:24, schrieb Felipe Contreras:
+if [ -z $script ]; then
+ local -a locations
+ locations=(
+ '/etc/bash_completion.d/git' # fedora, old debian
+ '/usr/share/bash-completion/completions/git' # arch, ubuntu,
new debian
+ '/usr/share/bash-completion/git' # gentoo
+ $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
+ )
Won't you need
local e
here, or does it not matter?
+ for e in $locations; do
+ test -f $e script=$e break
+ done
+fi
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] patch-ids: check modified paths before calculating diff
On Sun, May 19, 2013 at 02:17:35PM +0100, John Keeping wrote: When using git cherry or git log --cherry-pick we often have a small number of commits on one side and a large number on the other. In revision.c::cherry_pick_list we store the patch IDs for the small side before comparing the large side to this. In this case, it is likely that only a small number of paths are touched by the commits on the smaller side of the range and by storing these we can ignore many commits on the other side before unpacking blobs and diffing them. There are two observations that make your scheme work: 1. For something like --cherry-pick, we do not even care about the actual patch-ids, only whether there are matches from the left and right sides. 2. Comparing the sets of paths touched by two commits is often sufficient to realize they do not have the same patch-ids. But I think those observations mean we can do even better than calculating the real patch-id for the small side at all. Imagine we have both loose and strict patch-ids, where the loose one is much faster to compute, and has that property that a match _might_ mean we have the same strict patch-id, but a non-match means we definitely do not have the same strict patch-id. I think such a loose patch-id could just be a hash of the filenames that were changed by the patch (e.g., the first 32-bits of the sha1 of the concatenated filenames). Computing that should be about as expensive as a tree-diff. Per observation 2 above, if two commits do not have the same loose id, we know that they cannot possibly have the same strict id. Then we can forget about the smaller-side and bigger-side entirely, and just do something like: 1. Make a sorted list (or hash table) of loose ids for one side. 2. For each commit on the other side, calculate its loose id and look that up in the sorted list. If no hits, we know that there is no match. For any hits, lazily calculate (and cache) the strict patch id for both sides and compare as usual. In the best case, we compute no patch-ids at all. And even for the average case, I'd expect our lazy calculation to only have to compute a handful of ids. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] prompt: fix for simple rebase
When we are rebasing without options ('am' mode), the head rebased lives
in '$g/rebase-apply/head-name', so lets use that information so it's
reported the same way as if we were doing other rebases (-i or -m).
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-prompt.sh | 2 ++
t/t9903-bash-prompt.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index eaf5c36..bbf7657 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -279,6 +279,7 @@ __git_ps1 ()
step=$(cat $g/rebase-apply/next)
total=$(cat $g/rebase-apply/last)
if [ -f $g/rebase-apply/rebasing ]; then
+ b=$(cat $g/rebase-apply/head-name)
r=|REBASE
elif [ -f $g/rebase-apply/applying ]; then
r=|AM
@@ -295,6 +296,7 @@ __git_ps1 ()
r=|BISECTING
fi
+ test -n $b ||
b=$(git symbolic-ref HEAD 2/dev/null) || {
detached=yes
b=$(
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 083b319..15521cc 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -276,7 +276,7 @@ test_expect_success 'prompt - rebase merge' '
'
test_expect_success 'prompt - rebase' '
- printf ((t2)|REBASE 1/3) expected
+ printf (b2|REBASE 1/3) expected
git checkout b2
test_when_finished git checkout master
test_must_fail git rebase b1 b2
--
1.8.3.rc3.312.g47657de
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: zsh: improve bash script loading
On Wed, May 29, 2013 at 1:17 AM, Johannes Sixt j.s...@viscovery.net wrote:
Am 5/29/2013 5:24, schrieb Felipe Contreras:
+if [ -z $script ]; then
+ local -a locations
+ locations=(
+ '/etc/bash_completion.d/git' # fedora, old debian
+ '/usr/share/bash-completion/completions/git' # arch, ubuntu,
new debian
+ '/usr/share/bash-completion/git' # gentoo
+ $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
+ )
Won't you need
local e
here, or does it not matter?
You are right, otherwise it would be in the user's shell.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/7] add tests for rebasing of empty commits
---
t/t3401-rebase-partial.sh | 24
t/t3420-rebase-topology-linear.sh | 58 +++
2 files changed, 58 insertions(+), 24 deletions(-)
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 58f4823..7ba1797 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was
partially merged upstr
test_path_is_missing .git/rebase-merge
'
-test_expect_success 'rebase ignores empty commit' '
- git reset --hard A
- git commit --allow-empty -m empty
- test_commit D
- git rebase C
- test $(git log --format=%s C..) = D
-'
-
-test_expect_success 'rebase --keep-empty' '
- git reset --hard D
- git rebase --keep-empty C
- test $(git log --format=%s C..) = D
-empty
-'
-
-test_expect_success 'rebase --keep-empty keeps empty even if already in
upstream' '
- git reset --hard A
- git commit --allow-empty -m also-empty
- git rebase --keep-empty D
- test $(git log --format=%s A..) = also-empty
-D
-empty
-'
-
test_done
diff --git a/t/t3420-rebase-topology-linear.sh
b/t/t3420-rebase-topology-linear.sh
index 1152491..40fe264 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -160,4 +160,62 @@ test_run_rebase failure -m
test_run_rebase failure -i
test_run_rebase failure -p
+# a---b---c---j!
+# \
+# d---k!--l
+#
+# ! = empty
+test_expect_success 'setup of linear history for empty commit tests' '
+ git checkout c
+ make_empty j
+ git checkout d
+ make_empty k
+ test_commit l
+'
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* drops empty commit
+ reset_rebase
+ git rebase $* c l
+ test_cmp_rev c HEAD~2
+ test_linear_range 'd l' c..
+
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --keep-empty
+ reset_rebase
+ git rebase $* --keep-empty c l
+ test_cmp_rev c HEAD~3
+ test_linear_range 'd k l' c..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --keep-empty keeps empty even if already
in upstream
+ reset_rebase
+ git rebase $* --keep-empty j l
+ test_cmp_rev j HEAD~3
+ test_linear_range 'd k l' j..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
test_done
--
1.8.2.674.gd17d3d2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/7] t3406: modernize style
Update the following:
- Quote 'setup'
- Remove blank lines within test case body
- Use test_commit instead of custom quick_one
- Create branch topic from tag created by test_commit
---
t/t3406-rebase-message.sh | 30 +-
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index e6a9a0d..fe8c27f 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -4,27 +4,17 @@ test_description='messages from rebase operation'
. ./test-lib.sh
-quick_one () {
- echo $1 file$1
- git add file$1
- test_tick
- git commit -m $1
-}
+test_expect_success 'setup' '
+ test_commit O fileO
+ test_commit X fileX
+ test_commit A fileA
+ test_commit B fileB
+ test_commit Y fileY
-test_expect_success setup '
- quick_one O
- git branch topic
- quick_one X
- quick_one A
- quick_one B
- quick_one Y
-
- git checkout topic
- quick_one A
- quick_one B
- quick_one Z
+ git checkout -b topic O
+ git cherry-pick A B
+ test_commit Z fileZ
git tag start
-
'
cat expect \EOF
@@ -34,12 +24,10 @@ Committed: 0003 Z
EOF
test_expect_success 'rebase -m' '
-
git rebase -m master report
sed -n -e /^Already applied: /p \
-e /^Committed: /p report actual
test_cmp expect actual
-
'
test_expect_success 'rebase --stat' '
--
1.8.2.674.gd17d3d2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 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 v2 4/7] add tests for rebasing root
---
t/t3420-rebase-topology-linear.sh | 129 ++
1 file changed, 129 insertions(+)
diff --git a/t/t3420-rebase-topology-linear.sh
b/t/t3420-rebase-topology-linear.sh
index 40fe264..2429aa8 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -218,4 +218,133 @@ test_run_rebase failure -m
test_run_rebase failure -i
test_run_rebase failure -p
+# m
+# /
+# a---b---c---g
+#
+# x---y---B
+#
+# uppercase = cherry-picked
+# m = reverted b
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for test involving root' '
+ git checkout b
+ revert m b
+ git checkout --orphan disjoint
+ git rm -rf .
+ test_commit x
+ test_commit y
+ cherry_pick B b
+'
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --onto --root
+ reset_rebase
+ git rebase $* --onto c --root y
+ test_cmp_rev c HEAD~2
+ test_linear_range 'x y' c..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* without --onto --root with disjoint
history
+ reset_rebase
+ git rebase $* c y
+ test_cmp_rev c HEAD~2
+ test_linear_range 'x y' c..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --onto --root drops patch in onto
+ reset_rebase
+ git rebase $* --onto m --root B
+ test_cmp_rev m HEAD~2
+ test_linear_range 'x y' m..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --onto --root with merge-base does not
go to root
+ reset_rebase
+ git rebase $* --onto m --root g
+ test_cmp_rev m HEAD~2
+ test_linear_range 'c g' m..
+
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* without --onto --root with disjoint
history drops patch in onto
+ reset_rebase
+ git rebase $* m B
+ test_cmp_rev m HEAD~2
+ test_linear_range 'x y' m..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --root on linear history is a no-op
+ reset_rebase
+ git rebase $* --root c
+ test_cmp_rev c HEAD
+
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* -f --root on linear history causes
re-write
+ reset_rebase
+ git rebase $* -f --root c
+ ! test_cmp_rev a HEAD~2
+ test_linear_range 'a b c' HEAD
+
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
test_done
--
1.8.2.674.gd17d3d2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/7] tests: move test for rebase messages from t3400 to t3406
t3406 is supposed to test messages from rebase operation, so let's move tests in t3400 that fit that description into 3406. Most of the functionality they tested, except for the messages, has now been subsumed by t3420. --- t/t3400-rebase.sh | 22 -- t/t3406-rebase-message.sh | 22 ++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b436ef4..45a55e9 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -59,28 +59,6 @@ test_expect_success 'rebase against master' ' git rebase master ' -test_expect_success 'rebase against master twice' ' - git rebase master out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase against master twice with --force' ' - git rebase --force-rebase master out - test_i18ngrep Current branch my-topic-branch is up to date, rebase forced out -' - -test_expect_success 'rebase against master twice from another branch' ' - git checkout my-topic-branch^ - git rebase master my-topic-branch out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase fast-forward to master' ' - git checkout my-topic-branch^ - git rebase my-topic-branch out - test_i18ngrep Fast-forwarded HEAD to my-topic-branch out -' - test_expect_success 'the rebase operation should not have destroyed author information' ' ! (git log | grep Author: | grep ) ' diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index fe8c27f..0392e36 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -30,6 +30,28 @@ test_expect_success 'rebase -m' ' test_cmp expect actual ' +test_expect_success 'rebase against master twice' ' + git rebase master out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase against master twice with --force' ' + git rebase --force-rebase master out + test_i18ngrep Current branch topic is up to date, rebase forced out +' + +test_expect_success 'rebase against master twice from another branch' ' + git checkout topic^ + git rebase master topic out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase fast-forward to master' ' + git checkout topic^ + git rebase topic out + test_i18ngrep Fast-forwarded HEAD to topic out +' + test_expect_success 'rebase --stat' ' git reset --hard start git rebase --stat master diffstat.txt -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/7] add tests for rebasing merged history
--- t/t3400-rebase.sh | 31 + t/t3401-rebase-partial.sh | 45 --- t/t3404-rebase-interactive.sh | 10 +- t/t3409-rebase-preserve-merges.sh | 53 t/t3425-rebase-topology-merges.sh | 250 ++ 5 files changed, 252 insertions(+), 137 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3425-rebase-topology-merges.sh diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b58fa1a..b436ef4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' ' echo Side C git add C git commit -m Add C - git checkout -b nonlinear my-topic-branch - echo Edit B - git add B - git commit -m Modify B - git merge side - git checkout -b upstream-merged-nonlinear - git merge master git checkout -f my-topic-branch git tag topic ' @@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' ' git rebase master ' -test_expect_success 'rebase after merge master' ' - git checkout --detach refs/tags/topic - git branch -D topic - git reset --hard topic - git merge master - git rebase master - ! (git show | grep ^Merge:) -' - -test_expect_success 'rebase of history with merges is linearized' ' - git checkout nonlinear - test 4 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - -test_expect_success 'rebase of history with merges after upstream merge is linearized' ' - git checkout upstream-merged-nonlinear - test 5 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - test_expect_success 'rebase a single mode change' ' git checkout master + git branch -D topic echo 1 X git add X test_tick diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh deleted file mode 100755 index 7ba1797..000 --- a/t/t3401-rebase-partial.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland -# - -test_description='git rebase should detect patches integrated upstream - -This test cherry-picks one local change of two into master branch, and -checks that git rebase succeeds with only the second patch in the -local branch. -' -. ./test-lib.sh - -test_expect_success 'prepare repository with topic branch' ' - test_commit A - git checkout -b my-topic-branch - test_commit B - test_commit C - git checkout -f master - test_commit A2 A.t -' - -test_expect_success 'pick top patch from topic branch into master' ' - git cherry-pick C - git checkout -f my-topic-branch -' - -test_debug ' - git cherry master - git format-patch -k --stdout --full-index master /dev/null - gitk --all sleep 1 -' - -test_expect_success 'rebase topic branch against new master and check git am did not get halted' ' - git rebase master - test_path_is_missing .git/rebase-apply -' - -test_expect_success 'rebase --merge topic branch that was partially merged upstream' ' - git reset --hard C - git rebase --merge master - test_path_is_missing .git/rebase-merge -' - -test_done diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a58406d..ffcaf02 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' ' test $one = $(git rev-parse HEAD~2) ' -test_expect_success 'ignore patch if in upstream' ' - HEAD=$(git rev-parse HEAD) - git checkout -b has-cherry-picked HEAD^ +test_expect_success '--continue tries to commit, even for edit' ' echo unrelated file7 git add file7 test_tick git commit -m unrelated change - git cherry-pick $HEAD - EXPECT_COUNT=1 git rebase -i $HEAD - test $HEAD = $(git rev-parse HEAD^) -' - -test_expect_success '--continue tries to commit, even for edit' ' parent=$(git rev-parse HEAD^) test_tick FAKE_LINES=edit 1 git rebase -i HEAD^ diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index 6de4e22..2e0c364 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -11,14 +11,6 @@ Run git rebase -p and check that merges are properly carried along GIT_AUTHOR_EMAIL=bogus_email_address export GIT_AUTHOR_EMAIL -# Clone 1 (trivial merge): -# -# A1--A2 -- origin/master -# \ \ -# B1--M -- topic -#\ -# B2 -- origin/topic -# # Clone 2 (conflicting merge): # # A1--A2--B3 -- origin/master @@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL # \--A3
[PATCH v2 2/7] add tests for rebasing with patch-equivalence present
---
t/lib-rebase.sh | 17
t/t3420-rebase-topology-linear.sh | 85 +++
2 files changed, 102 insertions(+)
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 62b3887..16eeb1c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -80,3 +80,20 @@ reset_rebase () {
git reset --hard
git clean -f
}
+
+cherry_pick () {
+ git cherry-pick -n $2
+ git commit -m $1
+ git tag $1
+}
+
+revert () {
+ git revert -n $2
+ git commit -m $1
+ git tag $1
+}
+
+make_empty () {
+ git commit --allow-empty -m $1
+ git tag $1
+}
diff --git a/t/t3420-rebase-topology-linear.sh
b/t/t3420-rebase-topology-linear.sh
index c4b32db..1152491 100755
--- a/t/t3420-rebase-topology-linear.sh
+++ b/t/t3420-rebase-topology-linear.sh
@@ -75,4 +75,89 @@ test_run_rebase success -m
test_run_rebase success -i
test_run_rebase success -p
+# f
+# /
+# a---b---c---g---h
+# \
+# d---G---i
+#
+# uppercase = cherry-picked
+# h = reverted g
+#
+# Reverted patches are there for tests to be able to check if a commit
+# that introduced the same change as another commit is
+# dropped. Without reverted commits, we could get false positives
+# because applying the patch succeeds, but simply results in no
+# changes.
+test_expect_success 'setup of linear history for range selection tests' '
+ git checkout c
+ test_commit g
+ revert h g
+ git checkout d
+ cherry_pick G g
+ test_commit i
+ git checkout b
+ test_commit f
+'
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* drops patches in upstream
+ reset_rebase
+ git rebase $* h i
+ test_cmp_rev h HEAD~2
+ test_linear_range 'd i' h..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* can drop last patch if in upstream
+ reset_rebase
+ git rebase $* h G
+ test_cmp_rev h HEAD^
+ test_linear_range 'd' h..
+
+}
+test_run_rebase success ''
+test_run_rebase failure -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --onto does not lose patches in
upstream
+ reset_rebase
+ git rebase $* --onto f h i
+ test_cmp_rev f HEAD~3
+ test_linear_range 'd G i' f..
+
+}
+test_run_rebase failure ''
+test_run_rebase success -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+ result=$1
+ shift
+ test_expect_$result rebase $* --onto drops patches in onto
+ reset_rebase
+ git rebase $* --onto h f i
+ test_cmp_rev h HEAD~2
+ test_linear_range 'd i' h..
+
+}
+test_run_rebase failure ''
+test_run_rebase failure -m
+test_run_rebase failure -i
+test_run_rebase failure -p
+
test_done
--
1.8.2.674.gd17d3d2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/7] Rebase topology test
After way too long, here is finally a new version of the tests I sent at: http://thread.gmane.org/gmane.comp.version-control.git/205796. I have split the test up into two files. They stil take quite some time to run. Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 t/lib-rebase.sh | 32 t/t3400-rebase.sh | 53 +- t/t3401-rebase-partial.sh | 69 t/t3404-rebase-interactive.sh | 10 +- t/t3406-rebase-message.sh | 50 +++--- t/t3409-rebase-preserve-merges.sh | 53 -- t/t3420-rebase-topology-linear.sh | 350 ++ t/t3425-rebase-topology-merges.sh | 250 +++ 8 files changed, 664 insertions(+), 203 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3420-rebase-topology-linear.sh create mode 100755 t/t3425-rebase-topology-merges.sh -- 1.8.2.674.gd17d3d2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Retrieving a file at a before a specified commit
Hi all, I have a commit like this: commit 4d77a3cee01db0412956d40875c79f51ac745acc tree 3443c9f633114c3bd2e015453a8c55a171e62b53 parent 340d808ade8a79857bec40770f0eb4f98224c53d author committer . which modifies file A/B/C (ie specifically does not add, but changes an existing file in the repo). I would then like to retrive the version of the file A/B/C before commit 4d77a3cee by using the parent commit 340d808a: git show 340d808ade8a79857bec40770f0eb4f98224c53d:A/B/C which works for most files/commits I try this with, but doesn't work in one particular case. Questions: - Is my understanding of the above git command incorrect? - Is this a corrupt repo? Is there some way to check? - Is there some explaination of why I can't get the previous version of that file? Appreciate any light that could be shed on this. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] Rebase topology test
On Wed, May 29, 2013 at 1:39 AM, Martin von Zweigbergk martinv...@gmail.com wrote: After way too long, here is finally a new version of the tests I sent at: http://thread.gmane.org/gmane.comp.version-control.git/205796. I have split the test up into two files. They stil take quite some time to run. This is definitely needed. I'm doing some experiments with 'git rebase' and I see so many problems in the code that these tests do exercise. I think a lot of the functionality of 'git rebase' should move to 'git cherry-pick', and then all the 'git rebase' code can be simplified greatly, and tests like these would help a lot. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] patch-ids: check modified paths before calculating diff
On Wed, May 29, 2013 at 02:20:07AM -0400, Jeff King wrote:
In the best case, we compute no patch-ids at all. And even for the
average case, I'd expect our lazy calculation to only have to compute a
handful of ids.
Here is a not-well-tested version of the idea. I tried to contain the
changes to patch-ids.c, though we may also be able to simplify the
--cherry code, too.
Here are my timings compared to stock git and to yours
(all are best-of-five, git log --cherry $revs):
revs=origin/master...origin/jk/submodule-subdirectory-ok
stock|you |me
---
real 0m0.501s | 0m0.078s | 0m0.098s
user 0m0.480s | 0m0.056s | 0m0.084s
sys 0m0.016s | 0m0.020s | 0m0.012s
revs=origin/next...origin/pu
stock|you |me
---
real 0m0.857s | 0m0.847s | 0m0.519s
user 0m0.828s | 0m0.812s | 0m0.488s
sys 0m0.024s | 0m0.028s | 0m0.024s
So it performs slightly less well on the small case, and a bit better on
the large case. I think part of the problem is that when we do have a
loose hit, we end up re-doing the tree diff to find the strict, which
involves re-inflating the trees. It's unavoidable for the lazy entries
unless we want to cache the whole diff_queued_diff, but for the non-lazy
entries we should be able to do the strict patch-id incrementally. We
just need to improve the function interfaces.
---
diff.c | 15 -
diff.h | 2 +-
patch-ids.c | 117 +++
patch-ids.h | 11 ++--
4 files changed, 82 insertions(+), 63 deletions(-)
diff --git a/diff.c b/diff.c
index f0b3e7c..3b55788 100644
--- a/diff.c
+++ b/diff.c
@@ -4233,7 +4233,8 @@ static void patch_id_consume(void *priv, char *line,
unsigned long len)
}
/* returns 0 upon success, and writes result into sha1 */
-static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
+static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1,
+int loose)
{
struct diff_queue_struct *q = diff_queued_diff;
int i;
@@ -4266,6 +4267,12 @@ static int diff_get_patch_id(struct diff_options
*options, unsigned char *sha1)
if (DIFF_PAIR_UNMERGED(p))
continue;
+ if (loose) {
+ git_SHA1_Update(ctx, p-one-path,
strlen(p-one-path));
+ git_SHA1_Update(ctx, p-two-path,
strlen(p-two-path));
+ continue;
+ }
+
diff_fill_sha1_info(p-one);
diff_fill_sha1_info(p-two);
if (fill_mmfile(mf1, p-one) 0 ||
@@ -4323,11 +4330,13 @@ int diff_flush_patch_id(struct diff_options *options,
unsigned char *sha1)
return 0;
}
-int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
+int diff_flush_patch_id(struct diff_options *options,
+ unsigned char *sha1,
+ int loose)
{
struct diff_queue_struct *q = diff_queued_diff;
int i;
- int result = diff_get_patch_id(options, sha1);
+ int result = diff_get_patch_id(options, sha1, loose);
for (i = 0; i q-nr; i++)
diff_free_filepair(q-queue[i]);
diff --git a/diff.h b/diff.h
index 78b4091..b018aaf 100644
--- a/diff.h
+++ b/diff.h
@@ -320,7 +320,7 @@ extern int do_diff_cache(const unsigned char *, struct
diff_options *);
extern int run_diff_index(struct rev_info *revs, int cached);
extern int do_diff_cache(const unsigned char *, struct diff_options *);
-extern int diff_flush_patch_id(struct diff_options *, unsigned char *);
+extern int diff_flush_patch_id(struct diff_options *, unsigned char *, int
loose);
extern int diff_result_code(struct diff_options *, int);
diff --git a/patch-ids.c b/patch-ids.c
index bc8a28f..3a83ee6 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -5,7 +5,7 @@ static int commit_patch_id(struct commit *commit, struct
diff_options *options,
#include patch-ids.h
static int commit_patch_id(struct commit *commit, struct diff_options *options,
- unsigned char *sha1)
+ unsigned char *sha1, int loose)
{
if (commit-parents)
diff_tree_sha1(commit-parents-item-object.sha1,
@@ -13,27 +13,9 @@ static int commit_patch_id(struct commit *commit, struct
diff_options *options,
else
diff_root_tree_sha1(commit-object.sha1, , options);
diffcore_std(options);
- return diff_flush_patch_id(options, sha1);
+ return diff_flush_patch_id(options, sha1, loose);
}
-static const unsigned char *patch_id_access(size_t index, void *table)
-{
- struct patch_id **id_table = table;
- return id_table[index]-patch_id;
-}
-
-static int patch_pos(struct patch_id **table, int nr, const unsigned char *id)
-{
- return sha1_pos(id, table, nr, patch_id_access);
-}
-
-#define
Re: [PATCH v2 4/7] add tests for rebasing root
Am 5/29/2013 8:39, schrieb Martin von Zweigbergk:
+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..
Here you check the outcome. There is no explicit check whether the rebase
attempted to replay a and b. But that check is implicit: If a or b were
attempted to replay, the rebase would have been interrupted with no new
changes. Right?
+
+}
+
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
Just curious: Does the last one fail because the result is not correct or
because it does go to the root?
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?
On Tue, May 28, 2013 at 9:19 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, May 29, 2013 at 10:41 AM, Duy Nguyen pclo...@gmail.com wrote: The changes in this area since 1.8.2.3 seem to be Karsten's (I'm not blaming, just wanted to narrow down the problem). The patterns of interest seem to be !/bin /bin/* !/bin/brew Without !/bin v1.8.3 seems to behave the same as v1.8.2.3. Karsten, the block /* Abort if the directory is excluded */ in prep_exclude() seems to cause this. I think it goes through the exclude patterns, hits !/bin, believes the patterns do not make sense in this context and throws all away. I think Øystein's case falls into the same path. Commenting out the block seems to gain the old behavior back (and probably breaks other stuff). Contrary to what Junio said, I'm clueless about this. I wanted to read your series through and eventually gave up. I think I now have the motivation to look at it again this weekend. The very first patch in the da/darwin series in next is one possible fix. See 29de20504e9790785fe1698300755323f74972aa Makefile: fix default regex settings on Darwin t0070-fundamental.sh fails on Mac OS X 10.8: $ uname -a Darwin lustrous 12.2.0 Darwin Kernel Version 12.2.0: Sat Aug 25 00:48:52 PDT 2012; root:xnu-2050.18.24~1/RELEASE_X86_64 x86_64 $ ./t0070-fundamental.sh -v fatal: regex bug confirmed: re-build git with NO_REGEX=1 Fix it by using Git's regex library. Does that patch also fix this issue? Karsten, you mentioned that compiling without regex support fixes it for you. Does the above commit in git.git's next branch fix it too? If so, I think it would be worth merging this early part into a maint release. -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: avoid ls-remote in certain scenarios
On Tue, May 28, 2013 at 10:20:48PM -0500, Felipe Contreras wrote:
It's _very_ slow in many cases, and there's really no point in fetching
*everything* from the remote just for completion. In many cases it might
be faster for the user to type the whole thing.
If the user manually specifies 'refs/*', then the full ls-remote
completion is triggered.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.bash | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 1c35eef..2ce4f7d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -427,14 +427,8 @@ __git_refs ()
done
;;
*)
- git ls-remote $dir HEAD ORIG_HEAD 'refs/tags/*'
'refs/heads/*' 'refs/remotes/*' 2/dev/null | \
- while read -r hash i; do
- case $i in
- *^{}) ;;
- refs/*) echo ${i#refs/*/} ;;
- *) echo $i ;;
- esac
- done
+ echo HEAD
+ git for-each-ref --format=%(refname:short) --
refs/remotes/$dir/ | sed -e s#^$dir/##
This case statement is only executed when $dir is not a git directory,
so what ensures that the cwd is in a git repo or work tree when
executing this brach of the case statement? What about 'git
--git-dir=/path/to/repo' invocations or when $GIT_DIR is specified?
Gábor
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] patch-ids: check modified paths before calculating diff
On Wed, May 29, 2013 at 03:22:25AM -0400, Jeff King wrote:
revs=origin/master...origin/jk/submodule-subdirectory-ok
stock|you |me
---
real 0m0.501s | 0m0.078s | 0m0.098s
user 0m0.480s | 0m0.056s | 0m0.084s
sys 0m0.016s | 0m0.020s | 0m0.012s
revs=origin/next...origin/pu
stock|you |me
---
real 0m0.857s | 0m0.847s | 0m0.519s
user 0m0.828s | 0m0.812s | 0m0.488s
sys 0m0.024s | 0m0.028s | 0m0.024s
So it performs slightly less well on the small case, and a bit better on
the large case. I think part of the problem is that when we do have a
loose hit, we end up re-doing the tree diff to find the strict, which
involves re-inflating the trees. It's unavoidable for the lazy entries
unless we want to cache the whole diff_queued_diff, but for the non-lazy
entries we should be able to do the strict patch-id incrementally. We
just need to improve the function interfaces.
The (somewhat hacky) patch below, on top of my previous one, does that
optimization. But the timings aren't improved by much. My best-of-five
for the second case went down to:
real0m0.495s
user0m0.488s
sys 0m0.004s
However, the actual time to just do git log --raw $revs is:
real0m0.333s
user0m0.292s
sys 0m0.032s
which provides a lower-bound for how well we can do, as it is just doing
a single tree diff for each commit. So I think we have reaped most of
the benefits of this approach already (and we will generally have to do
_some_ true patch-id calculations, so we can never meet that lower
bound).
---
diff.c | 11 +++---
diff.h | 3 ++-
patch-ids.c | 39 ++-
3 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/diff.c b/diff.c
index 3b55788..161c5bf 100644
--- a/diff.c
+++ b/diff.c
@@ -4233,8 +4233,8 @@ static void patch_id_consume(void *priv, char *line,
unsigned long len)
}
/* returns 0 upon success, and writes result into sha1 */
-static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1,
-int loose)
+int diff_get_patch_id(struct diff_options *options, unsigned char *sha1,
+ int loose)
{
struct diff_queue_struct *q = diff_queued_diff;
int i;
@@ -4330,21 +4330,16 @@ int diff_flush_patch_id(struct diff_options *options,
return 0;
}
-int diff_flush_patch_id(struct diff_options *options,
- unsigned char *sha1,
- int loose)
+void diff_queue_clear(void)
{
struct diff_queue_struct *q = diff_queued_diff;
int i;
- int result = diff_get_patch_id(options, sha1, loose);
for (i = 0; i q-nr; i++)
diff_free_filepair(q-queue[i]);
free(q-queue);
DIFF_QUEUE_CLEAR(q);
-
- return result;
}
static int is_summary_empty(const struct diff_queue_struct *q)
diff --git a/diff.h b/diff.h
index b018aaf..7207d4b 100644
--- a/diff.h
+++ b/diff.h
@@ -320,7 +320,8 @@ extern int do_diff_cache(const unsigned char *, struct
diff_options *);
extern int run_diff_index(struct rev_info *revs, int cached);
extern int do_diff_cache(const unsigned char *, struct diff_options *);
-extern int diff_flush_patch_id(struct diff_options *, unsigned char *, int
loose);
+extern int diff_get_patch_id(struct diff_options *, unsigned char *, int
loose);
+extern void diff_queue_clear(void);
extern int diff_result_code(struct diff_options *, int);
diff --git a/patch-ids.c b/patch-ids.c
index 3a83ee6..83cda92 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -4,8 +4,7 @@
#include sha1-lookup.h
#include patch-ids.h
-static int commit_patch_id(struct commit *commit, struct diff_options *options,
- unsigned char *sha1, int loose)
+static void start_patch_id(struct commit *commit, struct diff_options *options)
{
if (commit-parents)
diff_tree_sha1(commit-parents-item-object.sha1,
@@ -13,7 +12,6 @@ static int commit_patch_id(struct commit *commit, struct
diff_options *options,
else
diff_root_tree_sha1(commit-object.sha1, , options);
diffcore_std(options);
- return diff_flush_patch_id(options, sha1, loose);
}
int init_patch_ids(struct patch_ids *ids)
@@ -50,12 +48,16 @@ static struct patch_id *add_commit(struct commit *commit,
int no_add)
{
struct patch_id *p;
- unsigned char loose[20], strict[20];
+ unsigned char loose[20], strict[20] = {0};
unsigned long hash;
void **pos;
- if (commit_patch_id(commit, ids-diffopts, loose, 1))
+ start_patch_id(commit, ids-diffopts);
+ if (diff_get_patch_id(ids-diffopts, loose, 1)) {
+ diff_queue_clear();
return NULL;
+ }
+
hash = hash_sha1(loose);
p =
Re: [PATCH v2 5/7] add tests for rebasing merged history
Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# a---b---c +# \ \ +# d---e \ +#\ \ \ +# n---o---w---v +# \ +# z +#TODO: make all flavors of rebase use --topo-order +test_run_rebase success 'e n o' '' +test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' -i As test_commit offers predictable timestamps, I think you can work around this discrepancy by generating commits n and o before e. (That is not a solution--just a workaround that depends on the current implementation--because the order in which parents of a merge are listed is unspecified.) +test_expect_success rebase -p re-creates internal merge + reset_rebase + git rebase -p c w + test_revision_subjects 'c d n e o w' HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD Shouldn't this better be test_cmp_rev c HEAD~4 test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD to ensure that c is not a rewritten commit? + + +test_expect_success rebase -p can re-create two branches on onto + reset_rebase + git rebase -p --onto c d w + test_revision_subjects 'c n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD + Same here. Time is fleeting. I have to stop here. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Retrieving a file at a before a specified commit
On Wed, May 29, 2013 at 04:47:35PM +1000, Erik de Castro Lopo wrote: I have a commit like this: commit 4d77a3cee01db0412956d40875c79f51ac745acc tree 3443c9f633114c3bd2e015453a8c55a171e62b53 parent 340d808ade8a79857bec40770f0eb4f98224c53d author committer . which modifies file A/B/C (ie specifically does not add, but changes an existing file in the repo). I would then like to retrive the version of the file A/B/C before commit 4d77a3cee by using the parent commit 340d808a: git show 340d808ade8a79857bec40770f0eb4f98224c53d:A/B/C which works for most files/commits I try this with, but doesn't work in one particular case. Yes, that should work as long as the file is modified and not added. You can also say 4d77a3cee^:A/B/C if you do not want to look up the parent id yourself. Note that for a merge commit with multiple parents, the question is more complex, as there are two previous states that are merged. You say that it doesn't work in one particular case. What is that case? What happens? Questions: - Is my understanding of the above git command incorrect? No, I think it is correct. - Is this a corrupt repo? Is there some way to check? Running git fsck can tell you if there is repository corruption. - Is there some explaination of why I can't get the previous version of that file? Probably. Can you show us the commit that fails? What does git say when you try it? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Felipe Contreras felipe.contre...@gmail.com writes:
Thomas Rast wrote:
Junio C Hamano gits...@pobox.com writes:
Thomas Rast Cc'ed as he has been the primary force behind this line
of notes usability.
Thanks for pointing this out to me.
Felipe Contreras felipe.contre...@gmail.com writes:
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/revert.c | 2 +
sequencer.c | 136
--
sequencer.h | 2 +
t/t3500-cherry.sh | 32 +
4 files changed, 169 insertions(+), 3 deletions(-)
git cherry-pick should help maintaining notes just like amend and
rebase, but how should this interact with notes.rewrite.command,
where the command is capable of doing this without an explicit
option once you tell which notes need to be maintained?
Since we already have the notes.rewrite.command convention, it would
seem the obvious choice to line it up with the others. The main
bikeshedding opportunity is whether this should be an exception and
default to false (all other commands default it to true).
Also: how does this interact with notes.rewriteRef and the corresponding
env vars? Why?
How does it interact with 'cherry-pick -n' if this is done in sequence,
effectively squashing several commits (this use-case is actually
suggested by the manpage), if multiple source commits had notes? Should
it respect notes.rewriteMode (and by default concatenate)? (I don't
know if the sequencer state is expressive enough already to carry this
in a meaningful way across cherry-pick commands.)
Feel free to implement that. I'm just interested in 'git cherry-pick' being
usable for 'git rebase' purposes.
Which would have been obvious to all but the most casual readers, eh?
We've been over this already:
The body should provide a meaningful commit message, which:
. explains the problem the change tries to solve, iow, what is wrong
with the current code without the change.
. justifies the way the change solves the problem, iow, why the
result with the change is better.
. alternate solutions considered but discarded, if any.
I'll gladly review your patches again once you have done that.
--
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 1/3] cherry-pick: add support to copy notes
On Wed, May 29, 2013 at 3:09 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: Feel free to implement that. I'm just interested in 'git cherry-pick' being usable for 'git rebase' purposes. Which would have been obvious to all but the most casual readers, eh? My motivations are irrelevant, the patch is good as it is. We've been over this already: The body should provide a meaningful commit message, which: . explains the problem the change tries to solve, iow, what is wrong with the current code without the change. Obvious; there is no support to copy notes. . justifies the way the change solves the problem, iow, why the result with the change is better. Obvious: because it now actually copies the notes. . alternate solutions considered but discarded, if any. There are no alternate solutions. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 22/25] string_list_add_refs_by_glob(): add a comment about memory management
Michael Haggerty mhag...@alum.mit.edu writes:
Since string_list_add_one_ref() adds refname to the string list, but
the lifetime of refname is limited, it is important that the
string_list passed to string_list_add_one_ref() has strdup_strings
set. Document this fact.
All current callers do the right thing.
[...]
+/*
+ * The list argument must have strdup_strings set on it.
+ */
void string_list_add_refs_by_glob(struct string_list *list, const char *glob)
{
if (has_glob_specials(glob)) {
Maybe add an assert() so that this is bulletproof?
--
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 00/25] Remove assumptions about each_ref_fn arg lifetimes
Michael Haggerty mhag...@alum.mit.edu writes:
I read the entire series on Monday, and give it an Ack at maybe 90%
confidence level -- sorry, I was short on caffeine and sleep ;-)
I meant to verify this assertion:
I did a manual audit of the 50 (!) functions that are used as an
each_ref_fn callback to the for_each_ref()-style functions. (I hope I
haven't missed any.) I checked that they do not make the assumption
that the lifetimes of the refname and sha1 arguments extend past the
duration of the callback invocation. There were a number of callers
that got this wrong; I believe I have fixed them all.
But time ran out, and I wouldn't want you to hold your breath. The
series is a big improvement even if another caller slipped through the
cracks.
--
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 1/3] cherry-pick: add support to copy notes
Felipe Contreras felipe.contre...@gmail.com writes:
On Wed, May 29, 2013 at 3:09 AM, Thomas Rast tr...@inf.ethz.ch wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
Feel free to implement that. I'm just interested in 'git cherry-pick' being
usable for 'git rebase' purposes.
Which would have been obvious to all but the most casual readers, eh?
My motivations are irrelevant, the patch is good as it is.
You fooled both Junio (AFAICT anyway) and me, who both reviewed the
patch under the assumption that it implements note copying *along the
lines of existing note copying*. This proved to be a wrong, and
time-wasting, assumption.
I think all the points in this discussion have been made in the lengthy
thread about remote-{bzr,hg} already. So much so in fact that your
continuing defiance of the project standards, knowing that they will
elicit exactly this response, amounts to trolling.
So until this changes, my $0.02 is a blanket NAK and a refusal to spend
my time reviewing.
--
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
Git status reports untracked on tracked files
I would be grateful for any thoughts about the follow problem. Git status reports untracked files: $ git status # On branch master # Untracked files: # (use git add file... to include in what will be committed) # # resource.enlighten/map/enlighten_test/.enlighten/__build_object__/geometry/land_9/processed/ nothing added to commit but untracked files present (use git add to track) But we have files at this directory at index: $ git ls-tree -r HEAD |grep land_9/processed/root.pim 100644 blob 9eeca5c75dc2c945600b6e0d253a8cb8191b7e80 resource.enlighten/map/enlighten_test/.enlighten/__build_object__/geometry/land_9/processed/root.pim I have checked this error appear after the first commit, that added this file. I have tried: 1. Clone repo. 2. Clean/Checkout file (as described at this [1] article) - after checkout status is untracked. 3. Copy full directory resource.enlighten/ to another repo and add commit - no errors. 4. Run git fsck - no errors. Git version 1.8.1.2. for Windows Git config: [core] repositoryformatversion = 0 filemode = false bare = false logallrefupdates = true symlinks = false ignorecase = true hideDotFiles = dotGitOnly compression = 1 Thank you in advance. [1] http://stackoverflow.com/questions/11525358/git-untracked-files-list-is-wrong -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git hangs on pthread_join
On Tue, May 28, 2013 at 01:51:09PM -0400, Jeff King wrote: On Thu, May 23, 2013 at 03:01:43PM +0200, Ian Kumlien wrote: git 1.8.2.1 is started by xinetd [...] I have found git receive-packs that has been running for days/weeks without terminating Attaching gdb and doing a trace results in: #0 0x003261207b35 in pthread_join () from /lib64/libpthread.so.0 #1 0x004ce58b in finish_async () #2 0x0045744b in cmd_receive_pack () #3 0x00404851 in handle_internal_command () #4 0x00404c9d in main () I recently fixed a deadlock that could happen in receive-pack when clients hung up before sending a valid pack header. The fix is commit 49ecfa1, and it's in git v1.8.2.2. With dodgy connections this could easily happen =) Really nice catch! The stack trace for the deadlock fixed by 49ecfa1 would have unpack_with_sideband between #1 and #2 above, but it is entirely possible that it is simply inlined in your build of git, depending on the -O level of your build (it is a static function that is only called from one place). So it seems likely that it is the culprit. Yeah, since it's a RHEL 5 machine i don't even get a debug rpm package =P I will upgrade all machines and keep monitoring, thanks! -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: Git Merge 2013 Conference, Berlin
Scott Chacon wrote: We're starting off in Berlin, May 9-11th. GitHub has secured conference space at the Radisson Blu Berlin for those days. I have a It's a pity that you did not announce the event on the msysgit mailing list, too, which is why I totally missed it until today, the event being almost over. This is especially sad for me as I'm living in Berlin, so it would have been easy for me to attend, and as I had offered to help you organizing the event when you were still looking for a location last year. I apologize, I will try to put events on that list as well in the future. Unfortunately, read this mail too late. If you're going to organize something more regularily (w/ up to 25 people), we perhaps could to it in office-2.0. Just let me know if you're interested. cu -- 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: Merge conflicts with version numbers in release branches
Thomas Koch wrote: it's a common problem[1,2,3] in Maven (Java) projects and probably in other environments too: You have the version number of your project written in the pom.xml. When one merges changes upwards from the maint branche to master, the version numbers in maint and master are different and cause a merge conflict. My advice: dont merge directly, but rebase to latest master. Maybe even rebase incrementally (eg. git rebase master~100 git rebase master~99 ...). This heavily reduces the chance of conflicts that need to be resolved manually. I'm a big fan of topic branches. For example, we have some bug #1234 in the maintenance release. Fork off at latest maint, lets call the branch 1234_somewhat. Now do your bugfixing, testing, etc. When thats done, rebase on latest maint (in case maint moved further) and merge it into maint. Now rebase the 1234_somewhat branch onto master, do tests etc and finally merge into it. (note: all merges here will be fast-forward, IOW: pure append operations). Of course, all of this wont make the conflicts on the version number change go away magically, but at least it will be more clear while resolving it. If you always do the version number changes in some separate commit, which has some specially formatted message (eg. 'Release: 1.2.4') you could hack some some little filter-branch magic, which automatically kicks off these commits before rebasing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
Felipe Contreras wrote:
Junio C Hamano wrote:
Neil Horman nhor...@tuxdriver.com writes:
On Mon, May 27, 2013 at 11:52:18AM -0500, Felipe Contreras wrote:
We should free objects before leaving.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
sequencer.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..7eeae2f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -626,12 +626,15 @@ static int do_pick_commit(struct commit
*commit, struct replay_opts *opts)
rerere(opts-allow_rerere_auto); } else {
int allow = allow_empty(opts, commit);
- if (allow 0)
- return allow;
+ if (allow 0) {
+ res = allow;
+ goto leave;
+ }
if (!opts-no_commit)
res = run_git_commit(defmsg, opts, allow);
}
+leave:
free_message(msg);
free(defmsg);
--
1.8.3.rc3.312.g47657de
Acked-by: Neil Horman nhor...@tuxdriver.com
This is better done without goto in general.
The other patch 2/2/ adds one more we need to exit from the middle
of the flow and makes it look handier to add an exit label here,
but it would be even better to express the logic of that patch as a
normal cascade of if/else if/..., which is small enough and we do
not need the leave: label.
Linux kernel developers would disagree. In C 'goto' is quite of then
the only sane option, and you can see 'goto' used in the Linux kernel
all over the place for that reason.
In this particular case it also makes perfect sense.
It probably is better to fold this patch into the other one when it
is rerolled to correct the option name gotcha on the tin.
Why? This patch is standalone and fixes an issue that is independent
of the other patch. Why squash two patches that do *two* different
things?
Anyway, I'll happily drop this patch if you want this memory leak to
remain. But then I'll do the same in the other patch.
This mantra of avodiing 'goto' is not helping anybody.
adding 5 letters (to change the next if into an else if) versus your
addition of several lines and some 15 additional letters (ignoring the
whitsspace) is IMHO enough to see what is better?
bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/7] add tests for rebasing merged history
Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i +#\ \ +# e---u +# +# uppercase = cherry-picked +# h = reverted g +test_expect_failure rebase -p --onto in merged history does not lose 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 + My expectations are different: When a patch is in upstream, then it is not to be rebased, even --onto somewhere else than upstream. But take this with a grain of salt, as I never encounter(ed) this use-case in practice. +test_expect_success rebase -p --onto in merged history drops 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 + And this is just the opposite case, where I think the patch should be kept. +# a---b---c +# \ +# d---e +#\ \ +# n---r +# \ +# o +# +# r = tree-same with n +# uppercace = cherry-picked I do not see any upper-cased letters in this graph. ;) +test_expect_success rebase -p re-creates empty internal merge commit + reset_rebase + git rebase -p c r + test_revision_subjects 'c d e n r' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD Again, check c with test_cmp_rev. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Wed, May 29, 2013 at 4:58 AM, Joachim Schmitz
j...@schmitz-digital.de wrote:
Felipe Contreras wrote:
Junio C Hamano wrote:
It probably is better to fold this patch into the other one when it
is rerolled to correct the option name gotcha on the tin.
Why? This patch is standalone and fixes an issue that is independent
of the other patch. Why squash two patches that do *two* different
things?
Anyway, I'll happily drop this patch if you want this memory leak to
remain. But then I'll do the same in the other patch.
This mantra of avodiing 'goto' is not helping anybody.
adding 5 letters (to change the next if into an else if) versus your
addition of several lines and some 15 additional letters (ignoring the
whitsspace) is IMHO enough to see what is better?
This has nothing to do with what Junio said. Junio said it is better
to squash the two changes into one, which is not clearly better.
As for your suggestion, what happens the next time somebody needs to
add something else to this chunk of code? Another if, and then
another, and soon enough you end up with five levels of indentation,
and in some of those patches you have to change the indentation of
existing code.
If only there was much bigger and successful software project that had
hashed all these questions and came up with a code-style to last the
ages. Oh, but there is, it's called Linux, and the answer is to use
goto's.
If the code used a goto in the first place.. BAM:
--- a/sequencer.c
+++ b/sequencer.c
at at -628,8 +628,10 at at static int
do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
allow = allow_empty(opts, commit);
- if (allow 0)
- return allow;
+ if (allow 0) {
+ res = allow;
+ goto leave;
+ }
if (!opts-no_commit)
res = run_git_commit(defmsg, opts, allow);
And every time you need to add more code you just do it, and stop
worrying about increasing indentation, or re-indenting.
Problem solved.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fixup! rebase: implement --[no-]autostash and rebase.autostash
For rr/rebase-autostash, which is stalled in pu. See $gmane/225689.
This is a super-minor fix anyway: if you disagree with something, change
it; there's no need to ask me.
As for the follow-up introducing a 'stash store', I will submit it in
good time: there's no hurry. I'm working on some other interesting
things in the meantime (see hot-branch and @{push}).
Thanks.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
git-rebase.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 709ef6b..5906757 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -151,16 +151,16 @@ finish_rebase () {
stash_sha1=$(cat $state_dir/autostash)
if git stash apply $stash_sha1 21 /dev/null
then
- echo Applied autostash
+ echo $(gettext 'Applied autostash.')
else
ref_stash=refs/stash
: $GIT_DIR/logs/$ref_stash
git update-ref -m autostash $ref_stash $stash_sha1 \
|| die $(eval_gettext 'Cannot store
$stash_sha1')
- echo
-$(gettext 'Applying autostash resulted in conflicts.
+ gettext 'Applying autostash resulted in conflicts.
Your changes are safe in the stash.
-You can apply or drop it at any time.')
+You can run git stash pop or git stash drop it at any time.
+'
fi
fi
git gc --auto
--
1.8.3.rc3.11.geb5ebca
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] sequencer: trivial fix
From: Felipe Contreras [mailto:felipe.contre...@gmail.com] Sent: Wednesday, May 29, 2013 12:52 PM To: Joachim Schmitz Cc: git@vger.kernel.org Subject: Re: [PATCH 1/2] sequencer: trivial fix On Wed, May 29, 2013 at 4:58 AM, Joachim Schmitz j...@schmitz-digital.de wrote: Felipe Contreras wrote: Junio C Hamano wrote: It probably is better to fold this patch into the other one when it is rerolled to correct the option name gotcha on the tin. Why? This patch is standalone and fixes an issue that is independent of the other patch. Why squash two patches that do *two* different things? Anyway, I'll happily drop this patch if you want this memory leak to remain. But then I'll do the same in the other patch. This mantra of avodiing 'goto' is not helping anybody. adding 5 letters (to change the next if into an else if) versus your addition of several lines and some 15 additional letters (ignoring the whitsspace) is IMHO enough to see what is better? This has nothing to do with what Junio said. Well, it has, but you had snipped it. But replied to the goto issue regardless This is better done without goto in general. Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 4/8] rebase: cherry-pick: fix abort of cherry mode
On 05/29/2013 06:16 AM, Felipe Contreras wrote: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-rebase.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-rebase.sh b/git-rebase.sh index 76900a0..9b5d78b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -335,6 +335,7 @@ skip) run_specific_rebase ;; abort) + test $type == cherrypick git cherry-pick --abort Bashism; please use =, not ==. git rerere clear read_basic_state case $head_name in Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
On Wed, May 29, 2013 at 3:40 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Wed, May 29, 2013 at 3:09 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: Feel free to implement that. I'm just interested in 'git cherry-pick' being usable for 'git rebase' purposes. Which would have been obvious to all but the most casual readers, eh? My motivations are irrelevant, the patch is good as it is. You fooled both Junio (AFAICT anyway) and me, who both reviewed the patch under the assumption that it implements note copying *along the lines of existing note copying*. This proved to be a wrong, and time-wasting, assumption. Whatever arbitrary rules you are talking about, they are not codified in tests. If you care so much about *the lines of existing note copying*, why don't you implement tests that check for those? This not only would prevent that some shmuck who is not well versed in the tradition of the lines of existing note copying from breaking that tradition, which is precisely what tests are for. If this was done, we wouldn't be time-wasting here. This patch makes 'git cherry-pick' pass all the tests that 'git rebase' passes. Period. Even if it was against the lines of existing note copying, it's much better than the current situation, which is to do ABSOLUTELY NOTHING. If you were a productive person, you would take this patch and implement the code that makes it align to the lines of existing note copying that you care so much about, which should be easy, if the note framework was properly implement, but you won't. Why isn't this implemented? void copy_notes_for_rewrite(const char *rewrite_cmd, struct rewritten *list); If there was such a thing, other clients wouldn't need to implement their own methods of copying notes. But you didn't implement that, did you? You are punishing me because the notes framework is lacking, and because the testing framework is missing what you think is the proper behavior. Strike that, you are punishing the users. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Wed, May 29, 2013 at 6:13 AM, Joachim Schmitz j...@schmitz-digital.de wrote: From: Felipe Contreras [mailto:felipe.contre...@gmail.com] Sent: Wednesday, May 29, 2013 12:52 PM To: Joachim Schmitz Cc: git@vger.kernel.org Subject: Re: [PATCH 1/2] sequencer: trivial fix On Wed, May 29, 2013 at 4:58 AM, Joachim Schmitz j...@schmitz-digital.de wrote: Felipe Contreras wrote: Junio C Hamano wrote: It probably is better to fold this patch into the other one when it is rerolled to correct the option name gotcha on the tin. Why? This patch is standalone and fixes an issue that is independent of the other patch. Why squash two patches that do *two* different things? Anyway, I'll happily drop this patch if you want this memory leak to remain. But then I'll do the same in the other patch. This mantra of avodiing 'goto' is not helping anybody. adding 5 letters (to change the next if into an else if) versus your addition of several lines and some 15 additional letters (ignoring the whitsspace) is IMHO enough to see what is better? This has nothing to do with what Junio said. Well, it has, but you had snipped it. But replied to the goto issue regardless I didn't snip anything, this is a different context. This is better done without goto in general. He din't say: __ It probably is better to fold this patch into the other one when it is rerolled to correct the option name gotcha on the tin, AND you fix the goto issue. __ You added that last part in your mind. Moreover, he didn't say goto was an issue, he simply stated an opinion about some generality. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #08; Tue, 28)
Junio C Hamano wrote:
* rr/rebase-autostash (2013-05-12) 7 commits
- rebase: implement --[no-]autostash and rebase.autostash
- rebase --merge: return control to caller, for housekeeping
- rebase -i: return control to caller, for housekeeping
- am: return control to caller, for housekeeping
- rebase: prepare to do generic housekeeping
- rebase -i: don't error out if $state_dir already exists
- am: tighten a conditional that checks for $dotest
This is from v3, but after a Fixed message in $gmane/224111 we
haven't seen a reroll yet. Also there was an attempt for a
follow-up, but it was never completed.
I've sent a tiny fixup for the last part. Let me know if you're
expecting something else.
* rr/die-on-missing-upstream (2013-05-22) 2 commits
- sha1_name: fix error message for @{N}, @{date}
- sha1_name: fix error message for @{u}
When a reflog notation is used for implicit current branch, we
did not say which branch and worse said branch ''.
Waiting for series of rerolls to settle.
I'm happy with the version in pu, and I don't intend to send any
re-rolls. Is there something you're not happy with?
Quick update from my side:
* publish-rev: the @{push} thing is still in the early poc stages.
* for-each-ref-pretty: not ready; working with Duy.
* push-current-head: ready but for the commit message: in your
opinion, it doesn't fix anything other than the output. I'll
rewrite and submit soon.
* pickaxe-doc: you had some more comments in latest iteration, but the
returns from a re-roll are diminishing. Frankly, the work is too
boring: the first few iterations were interesting, because I was
learning; now, it's mostly differences in taste. Nevertheless, I'll
re-roll when I want to sleep next.
That's about it, I think. I've abandoned everything else.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] sequencer: trivial fix
From: Felipe Contreras [mailto:felipe.contre...@gmail.com] Sent: Wednesday, May 29, 2013 1:24 PM To: Joachim Schmitz Cc: git@vger.kernel.org Subject: Re: [PATCH 1/2] sequencer: trivial fix On Wed, May 29, 2013 at 6:13 AM, Joachim Schmitz j...@schmitz-digital.de wrote: From: Felipe Contreras [mailto:felipe.contre...@gmail.com] Sent: Wednesday, May 29, 2013 12:52 PM To: Joachim Schmitz Cc: git@vger.kernel.org Subject: Re: [PATCH 1/2] sequencer: trivial fix On Wed, May 29, 2013 at 4:58 AM, Joachim Schmitz j...@schmitz-digital.de wrote: Felipe Contreras wrote: Junio C Hamano wrote: It probably is better to fold this patch into the other one when it is rerolled to correct the option name gotcha on the tin. Why? This patch is standalone and fixes an issue that is independent of the other patch. Why squash two patches that do *two* different things? Anyway, I'll happily drop this patch if you want this memory leak to remain. But then I'll do the same in the other patch. This mantra of avodiing 'goto' is not helping anybody. adding 5 letters (to change the next if into an else if) versus your addition of several lines and some 15 additional letters (ignoring the whitsspace) is IMHO enough to see what is better? This has nothing to do with what Junio said. Well, it has, but you had snipped it. But replied to the goto issue regardless I didn't snip anything, this is a different context. You did in your reply to me This is better done without goto in general. He din't say: __ It probably is better to fold this patch into the other one when it is rerolled to correct the option name gotcha on the tin, AND you fix the goto issue. __ You added that last part in your mind. Moreover, he didn't say goto was an issue, he simply stated an opinion about some generality. I added nothing in my mind, I just copy/paste that statement and was commenting on that and only that. At least intended to. Whenever anybody added more else branches, that's the time to possible switch to the goto style. And for the record: I agree with you that these 2 things should rather not be in a single patch as they are completely unrelated. Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Felipe Contreras felipe.contre...@gmail.com writes:
On Wed, May 29, 2013 at 3:40 AM, Thomas Rast tr...@inf.ethz.ch wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
On Wed, May 29, 2013 at 3:09 AM, Thomas Rast tr...@inf.ethz.ch wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
Feel free to implement that. I'm just interested in 'git cherry-pick'
being
usable for 'git rebase' purposes.
Which would have been obvious to all but the most casual readers, eh?
My motivations are irrelevant, the patch is good as it is.
You fooled both Junio (AFAICT anyway) and me, who both reviewed the
patch under the assumption that it implements note copying *along the
lines of existing note copying*. This proved to be a wrong, and
time-wasting, assumption.
Whatever arbitrary rules you are talking about, they are not codified in
tests.
Tests or code don't have a thing to do with it. This is about how you
are presenting your changes to the rest of the git community. As
evidenced above, said presentation is not clear enough to communicate
your goals to at least two experienced git developers (if I may say so
myself). How are we supposed to review a change if it is not even clear
what goal it satisfies?
Again: I'll be happy to review your proposed changes if and when you
resend the series with commit messages.
--
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 1/2] sequencer: trivial fix
From: Joachim Schmitz [mailto:j...@schmitz-digital.de] Sent: Wednesday, May 29, 2013 1:30 PM To: 'Felipe Contreras' Cc: 'git@vger.kernel.org' Subject: RE: [PATCH 1/2] sequencer: trivial fix snip And for the record: I agree with you that these 2 things should rather not be in a single patch as they are completely unrelated. I take that back: your patches 'overlap' so the 2nd won't apply without the 1st Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
On Wed, May 29, 2013 at 6:34 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Wed, May 29, 2013 at 3:40 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Wed, May 29, 2013 at 3:09 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: Feel free to implement that. I'm just interested in 'git cherry-pick' being usable for 'git rebase' purposes. Which would have been obvious to all but the most casual readers, eh? My motivations are irrelevant, the patch is good as it is. You fooled both Junio (AFAICT anyway) and me, who both reviewed the patch under the assumption that it implements note copying *along the lines of existing note copying*. This proved to be a wrong, and time-wasting, assumption. Whatever arbitrary rules you are talking about, they are not codified in tests. Tests or code don't have a thing to do with it. This is about how you are presenting your changes to the rest of the git community. As evidenced above, said presentation is not clear enough to communicate your goals to at least two experienced git developers (if I may say so myself). How are we supposed to review a change if it is not even clear what goal it satisfies? My goals are irrelevant. This patch is good. It implements a missing feature, if you don't like how it is implemented it: a) Implement the code in the note framework that does it properly so other people can just call that. b) Implement the tests for other commands that check that the behavior you talk about does happen indeed. c) Implement it yourself This has nothing to do with the way I presented the patch. I presented the patch as I thought it was; implementing the note copying as it was intended. Now you came along and say I wasted your time because I didn't say I implemented it wrongly, and you assume bad faith and what not. This wouldn't have happened because you didn't do a) or b). I realized that by replacing 'am' with 'cherry-pick' in 'git rebase' the notes were not copied, according to the testing framework, so I implemented that, and the tests pass. Now you come along and say it should implemented some note.rewrite.command stuff, but the tests didn't check for that, so how was I supposed to know? And then you have the audacity to claim that *I*, the one who just wrote a bunch of code to implement a missing feature, is wasting *your* time, you, the one who is simply replying to an email and shooting from the hip. Again: I'll be happy to review your proposed changes if and when you resend the series with commit messages. I won't, I'll keep working on my actual objective. Plus, this patch does have a commit message, and the commit message says *EXACTLY* what the patch is doing: add support to copy notes. If *you* are interested in certain behavior, why don't you lift a finger and do something to make sure that such behavior is easy to implement and the test framework actually checks for that? -- 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] git-remote-mediawiki: better error message when HTTP(S) access fails
Jeff King p...@peff.net writes:
On Thu, May 23, 2013 at 10:05:03PM +0200, Matthieu Moy wrote:
My use-case is an invalid SSL certificate. Pulling from the wiki with a
recent version of libwww-perl fails, and git-remote-mediawiki gave no
clue about the reason. Give the mediawiki API detailed error message, and
since it is not so informative, hint the user about an invalid SSL
certificate on https:// urls.
This is definitely an improvement, but it seems like it just the tip of
the iceberg.
Right.
I wonder if we can do something like:
our $mw_operation;
$mediawiki-{config}-{on_error} = sub {
[...]
die $err\n;
};
Probably, but that would hardcode the fact that mediawiki errors are
fatal, while in an ideal world, some errors should be recoverable, and
some would require some cleanups before die-ing.
Also, an error during the first mediawiki operation should not
necessarily have the same diagnosis hint as the others: if I just
did a successfull querry, and the next fails, it can hardly be an SSL
certificate error.
I'll send a v2 that covers a bit more (at least, push and pull with an
invalid certificate both give the message).
More work is needed to get a real good error management, but I don't
have time for that now.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-remote-mediawiki: better error message when HTTP(S) access fails
My use-case is an invalid SSL certificate. Pulling from the wiki with a
recent version of libwww-perl fails, and git-remote-mediawiki gave no
clue about the reason. Give the mediawiki API detailed error message, and
since it is not so informative, hint the user about an invalid SSL
certificate on https:// urls.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
contrib/mw-to-git/git-remote-mediawiki.perl | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 9c14c1f..f0c348c 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -238,6 +238,22 @@ sub mw_connect_maybe {
}
}
+sub fatal_mw_error {
+ my $action = shift;
+ print STDERR fatal: could not $action.\n;
+ print STDERR fatal: '$url' does not appear to be a mediawiki\n;
+ if ($url =~ /^https/) {
+ print STDERR fatal: make sure '$url/api.php' is a valid
page\n;
+ print STDERR fatal: and the SSL certificate is correct.\n;
+ } else {
+ print STDERR fatal: make sure '$url/api.php' is a valid
page.\n;
+ }
+ print STDERR fatal: (error .
+ $mediawiki-{error}-{code} . ': ' .
+ $mediawiki-{error}-{details} . )\n;
+ exit 1;
+}
+
## Functions for listing pages on the remote wiki
sub get_mw_tracked_pages {
my $pages = shift;
@@ -290,10 +306,7 @@ sub get_mw_all_pages {
aplimit = 'max'
});
if (!defined($mw_pages)) {
- print STDERR fatal: could not get the list of wiki pages.\n;
- print STDERR fatal: '$url' does not appear to be a
mediawiki\n;
- print STDERR fatal: make sure '$url/api.php' is a valid
page.\n;
- exit 1;
+ fatal_mw_error(get the list of wiki pages);
}
foreach my $page (@{$mw_pages}) {
$pages-{$page-{title}} = $page;
@@ -316,10 +329,7 @@ sub get_mw_first_pages {
titles = $titles,
});
if (!defined($mw_pages)) {
- print STDERR fatal: could not query the list of wiki pages.\n;
- print STDERR fatal: '$url' does not appear to be a
mediawiki\n;
- print STDERR fatal: make sure '$url/api.php' is a valid
page.\n;
- exit 1;
+ fatal_mw_error(query the list of wiki pages);
}
while (my ($id, $page) = each(%{$mw_pages-{query}-{pages}})) {
if ($id 0) {
--
1.8.3.rc3.8.g5e49f30
--
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] archive: loosen restrictions on remote object lookup
So, did this patch make it anywhere? We could really use it. Here's the use case. The original ee27ca4 patch broke our build system when the git server was upgraded to Debian Wheezy last night. The builder fetches source from the repo in two pieces using git archive, and we need to make sure both pieces are from the same commit. So we get a sha1 hash with git ls-remote, and use it with git archive --remote. This, of course, breaks with the 'no such ref' error. At the very least, the documentation is wrong when it talks about passing a commit ID to git archive: maintainers must surely agree that the documentation and the actual behavior ought to match. Personally, I'd like to see this patch adopted. Thanks for listening, Ian -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Thomas Rast wrote: So until this changes, my $0.02 is a blanket NAK and a refusal to spend my time reviewing. Then don't review the damn thing. With Felipe, I have the following rule of thumb: make some concrete suggestions and forget about follow-ups. He's not going to accept any general guidelines, unless you're quoting Documentation/SubmittingPatches (and even then, it's subject to interpretation); so provide a commit message and hope that either he or Junio will use it. There is no guarantee that he will take any of your suggestions, no matter how sensible you think they might be. However, he is a productive programmer, and submits fixes to real issues. He's stubborn, and we can't do much to change that: just learn to work with him. I'm disappointed that I have to point this out: haven't you learnt anything from previous discussions with him? Felipe, I suggest you put this in your commit message: This patch implements --copy-notes for 'git cherry-pick' so it can copy notes in the same way that 'git rebase' does. That is, if it's not too much trouble. Stop this back-and-fourth nonsense, both of you. It's degrading the community, and hitting everyone's inboxes with garbage. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] get_sha1: warn about full or short object names that look like refs
When we get 40 hex digits, we immediately assume it's an SHA-1. This
is the right thing to do because we have no way else to specify an
object. If there is a ref with the same object name, it will be
ignored. Warn the user about this case because the ref with full
object name is likely a mistake, for example
git checkout -b $empty_var $(git rev-parse something)
advice.object_name_warning is not documented because frankly people
should not be aware about it until they encounter this situation.
While at there, warn about ambiguation with abbreviated SHA-1 too.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
advice.c| 2 ++
advice.h| 1 +
sha1_name.c | 25 +++--
t/t1512-rev-parse-disambiguation.sh | 18 ++
4 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/advice.c b/advice.c
index 780f58d..22abde9 100644
--- a/advice.c
+++ b/advice.c
@@ -12,6 +12,7 @@ int advice_commit_before_merge = 1;
int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
+int advice_object_name_warning = 1;
static struct {
const char *name;
@@ -29,6 +30,7 @@ static struct {
{ resolveconflict, advice_resolve_conflict },
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
+ { object_name_warning, advice_object_name_warning },
/* make this an alias for backward compatibility */
{ pushnonfastforward, advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index fad36df..24d5420 100644
--- a/advice.h
+++ b/advice.h
@@ -15,6 +15,7 @@ extern int advice_commit_before_merge;
extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
+extern int advice_object_name_warning;
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/sha1_name.c b/sha1_name.c
index 95003c7..502d107 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -435,12 +435,31 @@ static int get_sha1_1(const char *name, int len, unsigned
char *sha1, unsigned l
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
{
static const char *warn_msg = refname '%.*s' is ambiguous.;
+ static const char *object_name_msg = N_(
+ Git normally never creates a ref that ends with 40 hex characters\n
+ because it will be ignored when you just specify 40-hex. These refs\n
+ may be created by mistake. For example,\n
+ \n
+ git checkout -b $br $(git rev-parse ...)\n
+ \n
+ where \$br\ is somehow empty and a 40-hex ref is created. Please\n
+ examine these refs and maybe delete them. Turn this message off by\n
+ running \git config advice.object_name_warning false\);
+ unsigned char tmp_sha1[20];
char *real_ref = NULL;
int refs_found = 0;
int at, reflog_len;
- if (len == 40 !get_sha1_hex(str, sha1))
+ if (len == 40 !get_sha1_hex(str, sha1)) {
+ refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
+ if (refs_found 0 warn_ambiguous_refs) {
+ warning(warn_msg, len, str);
+ if (advice_object_name_warning)
+ fprintf(stderr, %s\n, _(object_name_msg));
+ }
+ free(real_ref);
return 0;
+ }
/* basic@{time or number or -number} format to query ref-log */
reflog_len = at = 0;
@@ -481,7 +500,9 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
if (!refs_found)
return -1;
- if (warn_ambiguous_refs refs_found 1)
+ if (warn_ambiguous_refs
+ (refs_found 1 ||
+!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
warning(warn_msg, len, str);
if (reflog_len) {
diff --git a/t/t1512-rev-parse-disambiguation.sh
b/t/t1512-rev-parse-disambiguation.sh
index 6b3d797..db22808 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -261,4 +261,22 @@ test_expect_success 'rev-parse --disambiguate' '
test $(sed -e s/^\(.\).*/\1/ actual | sort -u) = 0
'
+test_expect_success 'ambiguous 40-hex ref' '
+ TREE=$(git mktree /dev/null)
+ REF=`git rev-parse HEAD`
+ VAL=$(git commit-tree $TREE /dev/null)
+ git update-ref refs/heads/$REF $VAL
+ test `git rev-parse $REF 2err` = $REF
+ grep refname.*${REF}.*ambiguous err
+'
+
+test_expect_success 'ambiguous short sha1 ref' '
+ TREE=$(git mktree /dev/null)
+ REF=`git rev-parse --short HEAD`
+ VAL=$(git commit-tree $TREE /dev/null)
+ git update-ref refs/heads/$REF $VAL
+ test `git rev-parse $REF 2err` = $VAL
+ grep
Re: [PATCH v2 8/8] revert/cherry-pick: add --skip option
Felipe Contreras wrote:
Akin to 'am --skip' and 'rebase --skip'.
This ranged-cherry-pick can be useful for small ranges. As pointed
out by others on the list, it hemorrhages memory quite horribly (and
this problem is non-trivial to fix). Perhaps we should document this
in limitations or bugs if we intend to make it more useful?
diff --git a/builtin/revert.c b/builtin/revert.c
index d63b4a6..6afd990 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -99,11 +99,13 @@ static void parse_args(int argc, const char **argv,
struct replay_opts *opts)
int remove_state = 0;
int contin = 0;
int rollback = 0;
+ int skip = 0;
Ugh, one more integer. Can't we use an OPT_BIT and store the action
in one variable? No hurry ofcourse: just asking.
@@ -1201,7 +1203,7 @@ static int sequencer_continue(struct replay_opts *opts)
}
if (index_differs_from(HEAD, 0))
return error_dirty_index(opts);
- {
+ if (!skip) {
unsigned char to[20];
if (!read_ref(HEAD, to))
add_rewritten(todo_list-item-object.sha1, to);
Couldn't you just say if (skip) todo_list = todo_list - next?
+ if (setup_rerere(merge_rr, 0) = 0) {
+ rerere_clear(merge_rr);
+ string_list_clear(merge_rr, 1);
+ }
Why exactly? Why doesn't rebase --skip 'rerere clear'?
+ argv[0] = reset;
+ argv[1] = --hard;
+ argv[2] = HEAD;
+ argv[3] = NULL;
+ ret = run_command_v_opt(argv, RUN_GIT_CMD);
Unrelated to your patch, but any clue why reset doesn't have an api
yet? Does it leak memory too?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git ignore regression in 1.8.3
Hi all, First of all I would like to thank you all for the great tool that is git. I love it and it makes my days way better. This is my first post on this mailing list so please apology if there is something wrong. I have noticed a regression in the behavior of ignore rules in 1.8.3. I have read the release notes and it might be related to all the great optimization you guys have done. My use case is quite uncommon but may happens to people who are tracking their dot files like I do. Basically I have a whitelist of non-ignored files at the top of my repository and a blacklist of ignored file in some of the direct sub-directories. Example: $ cd /tmp $ mkdir repo $ cd repo $ git init $ mkdir d $ touch tracked d/tracked $ git add tracked d/tracked $ git commit -m Initial commit $ touch untracked d/untracked $ touch ignored d/ignored $ echo '/*' .gitignore $ echo '!/d/' .gitignore $ echo '!/*' d/.gitignore $ echo 'ignored' d/.gitignore $ cat .gitignore /* !/d/ $ cat d/.gitignore !/* ignored $ git --version git version 1.8.3 $ git status -sb ## master ?? d/.gitignore ?? d/ignored ?? d/untracked $ /usr/local/Cellar/git/1.8.2.3/bin/git --version git version 1.8.2.3 $ /usr/local/Cellar/git/1.8.2.3/bin/git status -sb ## master ?? d/.gitignore ?? d/untracked $ git config status.showUntrackedFiles $ git check-ignore -v d/ignored .gitignore:2:!/d/ d/ignored $ /usr/local/Cellar/git/1.8.2.3/bin/git check-ignore -v d/ignored .gitignore:2:!/d/ d/ignored Although I am confused by the fact that both version of check-ignore print the same result, my understanding of this behavior is that the local d/.gitignore file is not taken into account and that once a negate rule is set I can not include other sub-pattern any longer. I have tried to only use the root .gitignore file like this /* !/d/ /d/ignored but I have the same result. Removing the trailing slash in the negative pattern (like this !/d) does not change anything. If I use !/d/*, both version of git-status no longer report d/untracked. I was happy with the 1.8.2.3 behavior. Maybe I misunderstand the new behavior of 1.8.3. If yes please tell me how I could achieve the same tricks in 1.8.3 because I did not find out how. I have the same result with the next branch (version 1.8.3.430.gc6abf3f). I have never hacked into git code base but I am willing to help if someone can guide me a bit. Regards, -- Nicolas Desprès -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] revert/cherry-pick: add --quiet option
Felipe Contreras wrote:
if (opts-skip_empty is_index_unchanged() == 1) {
- warning(_(skipping %s... %s),
- find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
- msg.subject);
+ if (!opts-quiet)
+ warning(_(skipping %s... %s),
+ find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
+ msg.subject);
goto leave;
}
All this trouble just to suppress the skipping ... message. Do you
see anything else --quiet can be used to suppress?
--
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 init doesn't create master branch
Dear List, the manual of git init says: An initial HEAD file that references the HEAD of the master branch is also created. However, after creating the repository using git init there's no master branch. How can make sure that master is created? Thanks in advance, Ákos Tajti -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git init doesn't create master branch
Ákos, Tajti akos.ta...@intland.com writes: Dear List, the manual of git init says: An initial HEAD file that references the HEAD of the master branch is also created. However, after creating the repository using git init there's no master branch. Right, but HEAD still points to it ;-). We sometimes call this an unborn branch. How can make sure that master is created? It will be created when you do the first commit. If you insist in having master created before you actually start working, you can run: git commit -m Initial empty commit --allow-empty Right after git init. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] Rebase topology test
Felipe Contreras wrote: I think a lot of the functionality of 'git rebase' should move to 'git cherry-pick', and then all the 'git rebase' code can be simplified greatly, and tests like these would help a lot. What do we do about the leakages? Want to take on the task of fixing the merge-recursive machinery? Cf. $gmane/222887 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git init doesn't create master branch
Matthieu Moy wrote: the manual of git init says: An initial HEAD file that references the HEAD of the master branch is also created. However, after creating the repository using git init there's no master branch. Right, but HEAD still points to it ;-). We sometimes call this an unborn branch. Right. Let me also add that our prompt has support for this; on an unborn branch, my prompt looks like: artagnon|master#:/tmp/foo$ ^ the # indicates unborn branch Please consider using contrib/completion/git-prompt.sh to make your life easier. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git init doesn't create master branch
Thanks for clarifying this thing for me! I don't really insist on having a master branch it's just that I tried to pull from a repository bundle and I got this error message: Cannot merge multiple branches into empty head The command was: git pull ../dump.dmp refs/heads/*:refs/heads/* Is this a better way of doing this? Thanks, Ákos 2013.05.29. 14:54 keltezéssel, Ramkumar Ramachandra írta: Matthieu Moy wrote: the manual of git init says: An initial HEAD file that references the HEAD of the master branch is also created. However, after creating the repository using git init there's no master branch. Right, but HEAD still points to it ;-). We sometimes call this an unborn branch. Right. Let me also add that our prompt has support for this; on an unborn branch, my prompt looks like: artagnon|master#:/tmp/foo$ ^ the # indicates unborn branch Please consider using contrib/completion/git-prompt.sh to make your life easier. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git init doesn't create master branch
Ákos, Tajti akos.ta...@intland.com writes: The command was: git pull ../dump.dmp refs/heads/*:refs/heads/* git pull does internally a git fetch followed by a git merge. If you try to pull several branches at the same time, it means you want to merge all of them together (octopus merge), which probably isn't what you're trying to do. You probably want just a git fetch, or specify only one branch to pull. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Tue, May 28, 2013 at 09:32:59PM -0500, Felipe Contreras wrote:
Junio C Hamano wrote:
Neil Horman nhor...@tuxdriver.com writes:
On Mon, May 27, 2013 at 11:52:18AM -0500, Felipe Contreras wrote:
We should free objects before leaving.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
sequencer.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..7eeae2f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -626,12 +626,15 @@ static int do_pick_commit(struct commit *commit,
struct replay_opts *opts)
rerere(opts-allow_rerere_auto);
} else {
int allow = allow_empty(opts, commit);
-if (allow 0)
-return allow;
+if (allow 0) {
+res = allow;
+goto leave;
+}
if (!opts-no_commit)
res = run_git_commit(defmsg, opts, allow);
}
+leave:
free_message(msg);
free(defmsg);
--
1.8.3.rc3.312.g47657de
Acked-by: Neil Horman nhor...@tuxdriver.com
This is better done without goto in general.
The other patch 2/2/ adds one more we need to exit from the middle
of the flow and makes it look handier to add an exit label here,
but it would be even better to express the logic of that patch as a
normal cascade of if/else if/..., which is small enough and we do
not need the leave: label.
Linux kernel developers would disagree. In C 'goto' is quite of then the only
sane option, and you can see 'goto' used in the Linux kernel all over the
place
for that reason.
In this particular case it also makes perfect sense.
I agree with Felipe here. Setting asside coding practice in other projects,
while its nice to follow coding convention in a project, a jump label just makes
more sense here. To not use it either requires you to duplicate the free
statements (undesireable), or to change the sense of theif clause here and nest
your if statements (makes for ugly reading).
It probably is better to fold this patch into the other one when it
is rerolled to correct the option name gotcha on the tin.
Why? This patch is standalone and fixes an issue that is independent of the
other patch. Why squash two patches that do *two* different things?
I agree here as well. This fixes a bug that has nothing to do with the other
patch, save for it being in the same C file. Fix them separately.
Anyway, I'll happily drop this patch if you want this memory leak to remain.
But then I'll do the same in the other patch.
This mantra of avodiing 'goto' is not helping anybody.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
On Wed, May 29, 2013 at 7:09 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Thomas Rast wrote: So until this changes, my $0.02 is a blanket NAK and a refusal to spend my time reviewing. Then don't review the damn thing. With Felipe, I have the following rule of thumb: make some concrete suggestions and forget about follow-ups. He didn't make any suggestions. He's not going to accept any general guidelines, unless you're quoting Documentation/SubmittingPatches (and even then, it's subject to interpretation); so provide a commit message and hope that either he or Junio will use it. There is no guarantee that he will take any of your suggestions, no matter how sensible you think they might be. This is bullshit. Let's look at some of the suggestions you have made: == git-related == s/l/line/? I said fine, and I implemented it. Still no -CCC? I said I forgot, and I implemented it. What you are really complaining about is that I don't agree with *every* single suggestion you make. And since you made them, they must be sensible, and single I don't agree with you, I must not be sensible, is that right? And stop bringing irrelevant garbage to this discussion. The discussion about the coding-style not about guidelines, because there is no guideline for that open parenthesis, ad obviated by the fact that there's over *FIVE HUNDRED* instances where it's not aligned that way. Nobody is denying the notes.rewritten.* guideline here, I didn't because that is *actually* a guideline. So your comment about guidelines is an irrelevant straw man. However, he is a productive programmer, and submits fixes to real issues. He's stubborn, and we can't do much to change that: just learn to work with him. I'm disappointed that I have to point this out: haven't you learnt anything from previous discussions with him? Felipe, I suggest you put this in your commit message: This patch implements --copy-notes for 'git cherry-pick' so it can copy notes in the same way that 'git rebase' does. That is, if it's not too much trouble. Stop this back-and-fourth nonsense, both of you. It's degrading the community, and hitting everyone's inboxes with garbage. Thanks. But let's take a step backwards, what are we trying to achieve here? We are trying to improve Git, and the indisputable fact is that 'git cherry-pick' is missing a way to copy the notes. It's indisputable that this patch implements that, and I did it by following existing code, and by running the whole test suit for 'git rebase'. I've done my job already. Thomas Rast doesn't like the way this is implemented, and nothing in the commit message would change that. This is was a sensible community, you would stop ganging up on me, Thomas Rast would implement copy_notes_for_rewrite(), and add tests for other commits (git am, git rebase) to check that the functionality he claims to be so worried about is working properly. And this was a sensible community you wouldn't complain about me choosing how to spend my time however I see fit. I did some work, I sent a patch, Thomas Rast has some issues, I'm not interested enough in this patch to investigate them, I work on something else. What's wrong with that? Eventually I might come back to this patch, and eventually I might implement copy_notes_for_rewrite, and I might implement the tests that check for the behavior that I missed, if nobody beats me to it, which is usually the case, but I think Thomas should put his personal issues aside, put his money where his mouth is, and implement it himself. There's nothing wrong with me choosing how best to spend my time. Really. -- 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: git init doesn't create master branch
Thanks! I ill try using fetch. Ákos 2013.05.29. 15:08 keltezéssel, Matthieu Moy írta: Ákos, Tajti akos.ta...@intland.com writes: The command was: git pull ../dump.dmp refs/heads/*:refs/heads/* git pull does internally a git fetch followed by a git merge. If you try to pull several branches at the same time, it means you want to merge all of them together (octopus merge), which probably isn't what you're trying to do. You probably want just a git fetch, or specify only one branch to pull. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git init doesn't create master branch
Ákos, Tajti wrote: Cannot merge multiple branches into empty head The command was: git pull ../dump.dmp refs/heads/*:refs/heads/* Is this a better way of doing this? pull runs a fetch, which updated .git/FETCH_HEAD. Now, if .git/FETCH_HEAD has just one branch (and other not-for-merge entries), there's no problem. We just run update-ref HEAD, and get it to point to the sole branch that was fetched. If your fetch fetches multiple branches, and you have a real branch (not unborn) with branch.name.merge set, we know which one of those branches to merge in. Now, what you have is a fetch which fetches multiple branches, and you're trying to merge that into a non-existent HEAD (or unborn branch). There is ambiguity and arguably no right thing to do, which is why git complains. I'm not sure if it's possible to argue differently and patch git-pull.sh for your usecase. There are many possible solutions to the problem, depending on what you want. The simplest I can suggest is: ignore the error for that command and run git reset --hard. -- 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] wildmatch: properly fold case everywhere
On Tue, May 28, 2013 at 8:58 PM, Anthony Ramine n.ox...@gmail.com wrote:
Case folding is not done correctly when matching against the [:upper:]
character class and uppercased character ranges (e.g. A-Z).
Specifically, an uppercase letter fails to match against any of them
when case folding is requested because plain characters in the pattern
and the whole string and preemptively lowercased to handle the base case
fast.
I did a little test with glibc fnmatch and also checked the source
code. I don't think 'a' matches [:upper:]. So I'm not sure if that's a
correct behavior or a bug in glibc. The spec is not clear (I think) on
this. I guess we should just assume that 'a' should match '[:upper:]'?
@@ -196,6 +196,11 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
}
if (t_ch = p_ch t_ch = prev_ch)
matched = 1;
+ else if ((flags WM_CASEFOLD)
ISLOWER(t_ch)) {
+ uchar t_ch_upper =
toupper(t_ch);
+ if (t_ch_upper = p_ch
t_ch_upper = prev_ch)
+ matched = 1;
+ }
Or we could stick with to tolower. Something like this
if ((t_ch = p_ch t_ch = prev_ch) ||
((flags WM_CASEFOLD)
t_ch = tolower(p_ch) t_ch = tolower(prev_ch)))
match = 1;
I think it's easier to read if we either downcase all, or upcase all, not both.
p_ch = 0; /* This makes prev_ch get
set to 0. */
} else if (p_ch == '[' p[1] == ':') {
const uchar *s;
@@ -245,6 +250,8 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
} else if (CC_EQ(s,i, upper)) {
if (ISUPPER(t_ch))
matched = 1;
+ else if ((flags
WM_CASEFOLD) ISLOWER(t_ch))
+ matched = 1;
} else if (CC_EQ(s,i, xdigit)) {
if (ISXDIGIT(t_ch))
matched = 1;
If WM_CASEFOLD is set, maybe isalpha(t_ch) is enough then?
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Mon, May 27, 2013 at 11:52 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Micronit: perhaps you should move the free obejcts before leaving (in do_pick_commit) to the subject instead of trivial fix, which adds no value to the patch. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 8/8] revert/cherry-pick: add --skip option
On Wed, May 29, 2013 at 7:27 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
Felipe Contreras wrote:
Akin to 'am --skip' and 'rebase --skip'.
This ranged-cherry-pick can be useful for small ranges. As pointed
out by others on the list, it hemorrhages memory quite horribly (and
this problem is non-trivial to fix). Perhaps we should document this
in limitations or bugs if we intend to make it more useful?
Is there a test-case that triggers this?
I don't see why we shouldn't try to fix this.
diff --git a/builtin/revert.c b/builtin/revert.c
index d63b4a6..6afd990 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -99,11 +99,13 @@ static void parse_args(int argc, const char **argv,
struct replay_opts *opts)
int remove_state = 0;
int contin = 0;
int rollback = 0;
+ int skip = 0;
Ugh, one more integer. Can't we use an OPT_BIT and store the action
in one variable? No hurry ofcourse: just asking.
I thought of switching to bit fields, but then I opened
builtin/checkout.c to see how many options were implemented, and I saw
more ints, so I left it like that.
@@ -1201,7 +1203,7 @@ static int sequencer_continue(struct replay_opts *opts)
}
if (index_differs_from(HEAD, 0))
return error_dirty_index(opts);
- {
+ if (!skip) {
unsigned char to[20];
if (!read_ref(HEAD, to))
add_rewritten(todo_list-item-object.sha1, to);
Couldn't you just say if (skip) todo_list = todo_list - next?
We are already doing that. And after that we need to continue with the
rest of the commits.
+ if (setup_rerere(merge_rr, 0) = 0) {
+ rerere_clear(merge_rr);
+ string_list_clear(merge_rr, 1);
+ }
Why exactly? Why doesn't rebase --skip 'rerere clear'?
It does, and so does 'git am', so why shouldn't 'git cherry-pick'.
Also, I think the same should happen in --abort.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] revert/cherry-pick: add --quiet option
On Wed, May 29, 2013 at 7:33 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
Felipe Contreras wrote:
if (opts-skip_empty is_index_unchanged() == 1) {
- warning(_(skipping %s... %s),
- find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
- msg.subject);
+ if (!opts-quiet)
+ warning(_(skipping %s... %s),
+ find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV),
+ msg.subject);
goto leave;
}
All this trouble just to suppress the skipping ... message. Do you
see anything else --quiet can be used to suppress?
Did you miss the -q option passed to 'git commit'?
--
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: git ignore regression in 1.8.3
On Wed, May 29, 2013 at 7:31 PM, Nicolas Desprès nicolas.desp...@gmail.com wrote: I have noticed a regression in the behavior of ignore rules in 1.8.3. Yeah, it looks like everybody suddenly realizes this regression soon after the release, not before :( This has been reported three times so far. You may want to check the discussion (and hopefully progress soon) in this thread: http://thread.gmane.org/gmane.comp.version-control.git/225675/focus=225713 We really need to add some real world use cases of gitignore in the test suite. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] revert/cherry-pick: add --quiet option
Felipe Contreras wrote: Did you miss the -q option passed to 'git commit'? Ah, yes. It would help if you mentioned: Introduce --quiet to suppress warning about skipped commits (when using --skip-empty) and output from 'git commit'. in the commit message. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Wed, May 29, 2013 at 8:25 AM, Duy Nguyen pclo...@gmail.com wrote: On Mon, May 27, 2013 at 11:52 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Micronit: perhaps you should move the free obejcts before leaving (in do_pick_commit) to the subject instead of trivial fix, which adds no value to the patch. Perhaps. I prefer it this way because it's really a trivial fix not really worth much time thinking about it. So when somebody is browsing the history they can happily skip this one. The time save by not reading I think adds more value than any succinct description that would force each and every patch-reviewer/history-reader to read it. -- 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 v3] wildmatch: properly fold case everywhere
Replied inline.
Regards,
--
Anthony Ramine
Le 29 mai 2013 à 15:22, Duy Nguyen a écrit :
On Tue, May 28, 2013 at 8:58 PM, Anthony Ramine n.ox...@gmail.com wrote:
Case folding is not done correctly when matching against the [:upper:]
character class and uppercased character ranges (e.g. A-Z).
Specifically, an uppercase letter fails to match against any of them
when case folding is requested because plain characters in the pattern
and the whole string and preemptively lowercased to handle the base case
fast.
I did a little test with glibc fnmatch and also checked the source
code. I don't think 'a' matches [:upper:]. So I'm not sure if that's a
correct behavior or a bug in glibc. The spec is not clear (I think) on
this. I guess we should just assume that 'a' should match '[:upper:]'?
I don't know, in my opinion if case folding is enabled we should say [:upper:],
[:lower:] and [:alpha:] are equivalent.
This opinion is shared by GNU Flex [1]:
• If your scanner is case-insensitive (the ‘-i’ flag), then ‘[:upper:]’
and ‘[:lower:]’ are equivalent to ‘[:alpha:]’.
[1] http://flex.sourceforge.net/manual/Patterns.html
@@ -196,6 +196,11 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
}
if (t_ch = p_ch t_ch = prev_ch)
matched = 1;
+ else if ((flags WM_CASEFOLD)
ISLOWER(t_ch)) {
+ uchar t_ch_upper =
toupper(t_ch);
+ if (t_ch_upper = p_ch
t_ch_upper = prev_ch)
+ matched = 1;
+ }
Or we could stick with to tolower. Something like this
if ((t_ch = p_ch t_ch = prev_ch) ||
((flags WM_CASEFOLD)
t_ch = tolower(p_ch) t_ch = tolower(prev_ch)))
match = 1;
I think it's easier to read if we either downcase all, or upcase all, not
both.
If the range to match against is [A-_], it will become [a-_] which is an empty
range, ord('a') ord('_'). I think it is simpler to reuse toupper() after the
fact as I did.
Anyway maybe I should add a test for that corner case?
p_ch = 0; /* This makes prev_ch get
set to 0. */
} else if (p_ch == '[' p[1] == ':') {
const uchar *s;
@@ -245,6 +250,8 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
} else if (CC_EQ(s,i, upper)) {
if (ISUPPER(t_ch))
matched = 1;
+ else if ((flags
WM_CASEFOLD) ISLOWER(t_ch))
+ matched = 1;
} else if (CC_EQ(s,i, xdigit)) {
if (ISXDIGIT(t_ch))
matched = 1;
If WM_CASEFOLD is set, maybe isalpha(t_ch) is enough then?
Yes isalpha() is enought but I wanted to keep the two cases separated, I can
amend that if you want.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] revert/cherry-pick: add --quiet option
On Wed, May 29, 2013 at 8:32 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: Did you miss the -q option passed to 'git commit'? Ah, yes. It would help if you mentioned: Introduce --quiet to suppress warning about skipped commits (when using --skip-empty) and output from 'git commit'. I would switch them around, as the former is not important, in fact, it's so unimportant that I wouldn't even mention it. And in fact, it wasn't even there when I wrote this commit message I sent for v1, it was only squashed later on. The only really important thing this patch does is 'commit -q', and I cannot imagine what the purpose of a -q option to cherry-pick would be if it's not that. But yeah, it wouldn't hurt. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Wed, May 29, 2013 at 8:34 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 8:25 AM, Duy Nguyen pclo...@gmail.com wrote: On Mon, May 27, 2013 at 11:52 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Micronit: perhaps you should move the free obejcts before leaving (in do_pick_commit) to the subject instead of trivial fix, which adds no value to the patch. Perhaps. I prefer it this way because it's really a trivial fix not really worth much time thinking about it. So when somebody is browsing the history they can happily skip this one. The time save by not reading I think adds more value than any succinct description that would force each and every patch-reviewer/history-reader to read it. Some time from now, assume a ridiculus case when this function grows more complex and somebody wonders what the leave label is for, git log --oneline -Slabel: showing trivial fix would not help much. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git ignore regression in 1.8.3
On Wed, May 29, 2013 at 8:31 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, May 29, 2013 at 7:31 PM, Nicolas Desprès nicolas.desp...@gmail.com wrote: I have noticed a regression in the behavior of ignore rules in 1.8.3. Yeah, it looks like everybody suddenly realizes this regression soon after the release, not before :( That's because the vast majority of users only use the releases (maybe more than 90%?). It's not a big deal, that's what 1.8.3.1 is for :) (also, that's why we shouldn't worry to death about imaginary users, our real users will gladly point out when we have screwed up). -- 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 RESEND v2] path: Fix a sparse warning
On 2013-05-28 19.04, Junio C Hamano wrote: Can you tell me what the conclusion on the discussion on your two other patches on 'pu'? * rj/mingw-cygwin (2013-05-08) 2 commits - cygwin: Remove the CYGWIN_V15_WIN32API build variable - mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE I stopped keeping track of the discussion and my vague recollection was that it is OK for 1.5 but not verified on 1.7 or something? The tests I did under cygwin 1.7 did not show any regression. /Torsten -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Wed, May 29, 2013 at 8:42 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, May 29, 2013 at 8:34 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 8:25 AM, Duy Nguyen pclo...@gmail.com wrote: On Mon, May 27, 2013 at 11:52 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Micronit: perhaps you should move the free obejcts before leaving (in do_pick_commit) to the subject instead of trivial fix, which adds no value to the patch. Perhaps. I prefer it this way because it's really a trivial fix not really worth much time thinking about it. So when somebody is browsing the history they can happily skip this one. The time save by not reading I think adds more value than any succinct description that would force each and every patch-reviewer/history-reader to read it. Some time from now, assume a ridiculus case when this function grows more complex and somebody wonders what the leave label is for, git log --oneline -Slabel: showing trivial fix would not help much. Fortunately that's not the main use-case, and for that single instance that probably will never happen, I think it's not too much to ask to this hypothetical developer to remove the --oneline, or copy-paste the SHA-1 and take a peek. He would probably need to do that anyway. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
Felipe Contreras wrote: What you are really complaining about is that I don't agree with *every* single suggestion you make. And since you made them, they must be sensible, and single I don't agree with you, I must not be sensible, is that right? Oh, I have no problems: I reviewed git-related, and it resulted in massive simplifications and improvements; those threads didn't run wild. I was mainly criticizing Thomas' review style (when it comes to reviewing your patches in particular), and was prodding him to make concrete suggestions in a one-email review, and leave it at that. What started out as a pointer to the guidelines: Thomas Rast wrote: We've been over this already: The body should provide a meaningful commit message, which: has resulted in this thread running wild. There's nothing wrong with me choosing how best to spend my time. Really. Ofcourse you are. You have arguably spent it very productively solving a lot of user issues (especially remote-bzr). Personally, I try to do the minimum amount of boring work required to make sure that a good series gets in. Sometimes this is a little high (for a recent example, see my pickaxe-doc series). The result being that I just won't work on documentation in the future because doing iterations is so piss boring: the git community needs to recognize this problem and make amends. -- 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] wildmatch: properly fold case everywhere
On Wed, May 29, 2013 at 8:37 PM, Anthony Ramine n.ox...@gmail.com wrote:
Le 29 mai 2013 à 15:22, Duy Nguyen a écrit :
On Tue, May 28, 2013 at 8:58 PM, Anthony Ramine n.ox...@gmail.com wrote:
Case folding is not done correctly when matching against the [:upper:]
character class and uppercased character ranges (e.g. A-Z).
Specifically, an uppercase letter fails to match against any of them
when case folding is requested because plain characters in the pattern
and the whole string and preemptively lowercased to handle the base case
fast.
I did a little test with glibc fnmatch and also checked the source
code. I don't think 'a' matches [:upper:]. So I'm not sure if that's a
correct behavior or a bug in glibc. The spec is not clear (I think) on
this. I guess we should just assume that 'a' should match '[:upper:]'?
I don't know, in my opinion if case folding is enabled we should say
[:upper:], [:lower:] and [:alpha:] are equivalent.
This opinion is shared by GNU Flex [1]:
• If your scanner is case-insensitive (the ‘-i’ flag), then
‘[:upper:]’ and ‘[:lower:]’ are equivalent to ‘[:alpha:]’.
[1] http://flex.sourceforge.net/manual/Patterns.html
Then we should do it too because of this precedent, I think.
@@ -196,6 +196,11 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
}
if (t_ch = p_ch t_ch = prev_ch)
matched = 1;
+ else if ((flags WM_CASEFOLD)
ISLOWER(t_ch)) {
+ uchar t_ch_upper =
toupper(t_ch);
+ if (t_ch_upper = p_ch
t_ch_upper = prev_ch)
+ matched = 1;
+ }
Or we could stick with to tolower. Something like this
if ((t_ch = p_ch t_ch = prev_ch) ||
((flags WM_CASEFOLD)
t_ch = tolower(p_ch) t_ch = tolower(prev_ch)))
match = 1;
I think it's easier to read if we either downcase all, or upcase all, not
both.
If the range to match against is [A-_], it will become [a-_] which is an
empty range, ord('a') ord('_'). I think it is simpler to reuse toupper()
after the fact as I did.
Anyway maybe I should add a test for that corner case?
Yeah I was thinking about such a case, but I saw glibc do it... I
guess we just found another bug, at least in compat/fnmatch.c. Yes a
test for it would be great, in case I change my mind 2 years from now
and decide to turn it the other way ;)
p_ch = 0; /* This makes prev_ch
get set to 0. */
} else if (p_ch == '[' p[1] == ':') {
const uchar *s;
@@ -245,6 +250,8 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
} else if (CC_EQ(s,i, upper)) {
if (ISUPPER(t_ch))
matched = 1;
+ else if ((flags
WM_CASEFOLD) ISLOWER(t_ch))
+ matched = 1;
} else if (CC_EQ(s,i, xdigit)) {
if (ISXDIGIT(t_ch))
matched = 1;
If WM_CASEFOLD is set, maybe isalpha(t_ch) is enough then?
Yes isalpha() is enought but I wanted to keep the two cases separated, I can
amend that if you want.
Either way is fine. I don't think this code is performance critical. Your call.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] Rebase topology test
On Wed, May 29, 2013 at 7:50 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: I think a lot of the functionality of 'git rebase' should move to 'git cherry-pick', and then all the 'git rebase' code can be simplified greatly, and tests like these would help a lot. What do we do about the leakages? Want to take on the task of fixing the merge-recursive machinery? Cf. $gmane/222887 Hmm, I saw that, but I also saw the fix. Anyway, if this is such a big issue, I'm sure it should be possible to trigger it in our test frameework. -- 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: git ignore regression in 1.8.3
On Wed, May 29, 2013 at 3:31 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, May 29, 2013 at 7:31 PM, Nicolas Desprès nicolas.desp...@gmail.com wrote: I have noticed a regression in the behavior of ignore rules in 1.8.3. Yeah, it looks like everybody suddenly realizes this regression soon after the release, not before :( This has been reported three times so far. You may want to check the discussion (and hopefully progress soon) in this thread: http://thread.gmane.org/gmane.comp.version-control.git/225675/focus=225713 Oups! I'm sorry I searched the archives but did not find this one. Thanks for your reply and the link We really need to add some real world use cases of gitignore in the test suite. There is kind of one included in to my post (minus the assert statements). -- Nicolas Desprès -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sequencer: trivial fix
On Wed, May 29, 2013 at 8:46 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 8:42 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, May 29, 2013 at 8:34 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 8:25 AM, Duy Nguyen pclo...@gmail.com wrote: On Mon, May 27, 2013 at 11:52 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Micronit: perhaps you should move the free obejcts before leaving (in do_pick_commit) to the subject instead of trivial fix, which adds no value to the patch. Perhaps. I prefer it this way because it's really a trivial fix not really worth much time thinking about it. So when somebody is browsing the history they can happily skip this one. The time save by not reading I think adds more value than any succinct description that would force each and every patch-reviewer/history-reader to read it. Some time from now, assume a ridiculus case when this function grows more complex and somebody wonders what the leave label is for, git log --oneline -Slabel: showing trivial fix would not help much. Fortunately that's not the main use-case, and for that single instance that probably will never happen, I think it's not too much to ask to this hypothetical developer to remove the --oneline, or copy-paste the SHA-1 and take a peek. He would probably need to do that anyway. And the time saving by not reading is also hypothetical. But I won't continue this discussion. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] cherry-pick: add support to copy notes
On Wed, May 29, 2013 at 8:48 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: There's nothing wrong with me choosing how best to spend my time. Really. Ofcourse you are. You have arguably spent it very productively solving a lot of user issues (especially remote-bzr). Personally, I try to do the minimum amount of boring work required to make sure that a good series gets in. Sometimes this is a little high (for a recent example, see my pickaxe-doc series). The result being that I just won't work on documentation in the future because doing iterations is so piss boring: the git community needs to recognize this problem and make amends. I don't mind doing as many iterations as it takes, as long as it's about meaningful issues. I don't particularly enjoy, but I'm OK with discussing non-meaningful issues. What I'm not OK with is disagreements that end up breaking the communication, specially when a crystal-clear case has been made (IMO), and the patch goes to limbo for no reason. That I think is a real problem. For this particular patch, I don't care if it goes in at the moment. I have something big on the pipeline, and I would rather drop this than loose penguin points, although I probably don't have any left. 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 1/2] sequencer: trivial fix
On Wed, May 29, 2013 at 8:54 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, May 29, 2013 at 8:46 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 8:42 AM, Duy Nguyen pclo...@gmail.com wrote: On Wed, May 29, 2013 at 8:34 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 8:25 AM, Duy Nguyen pclo...@gmail.com wrote: On Mon, May 27, 2013 at 11:52 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Micronit: perhaps you should move the free obejcts before leaving (in do_pick_commit) to the subject instead of trivial fix, which adds no value to the patch. Perhaps. I prefer it this way because it's really a trivial fix not really worth much time thinking about it. So when somebody is browsing the history they can happily skip this one. The time save by not reading I think adds more value than any succinct description that would force each and every patch-reviewer/history-reader to read it. Some time from now, assume a ridiculus case when this function grows more complex and somebody wonders what the leave label is for, git log --oneline -Slabel: showing trivial fix would not help much. Fortunately that's not the main use-case, and for that single instance that probably will never happen, I think it's not too much to ask to this hypothetical developer to remove the --oneline, or copy-paste the SHA-1 and take a peek. He would probably need to do that anyway. And the time saving by not reading is also hypothetical. But I won't continue this discussion. Is it? How much time does it take to read trivial fix? Half a second? How much time does it take copy-paste the SHA-1 of a --oneline log? Five seconds? So to break even we need ten readers that would only browse the history per each person that goes beyond the summary. To be safe let's do +- 100% and make it twenty readers. I think it's safe to assume there will be more than 20 readers skipping this commit without much though, perhaps a 100 or even more, and how many would need to take a closer look? I'd say 0, 1 might be possible, but to err on the side of caution let's say 2, hell, let's be generous and make it 3. We are still safe well beyond profit. But we have already wasted many more seconds than any of those guys would, so does it really matter? -- 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
Check out doesn't set x-flag on CIFS
Hello, When on a CIFS filesystem a git checkout does not replicate the executable flag from the repository: $ git clone git://git/abettersqlplus Cloning into 'abettersqlplus'... remote: Counting objects: 522, done. remote: Compressing objects: 100% (342/342), done. remote: Total 522 (delta 166), reused 522 (delta 166) Receiving objects: 100% (522/522), 82.40 KiB, done. Resolving deltas: 100% (166/166), done. $ ls -l abettersqlplus/absp.py -rw-rw-r-- 1 aesser geneity 45860 May 29 14:46 abettersqlplus/absp.py Subsequently git status reports the file as changed: $ cd abettersqlplus/ $ git status # On branch master # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # #modified: absp.py # no changes added to commit (use git add and/or git commit -a) If I set the x-flag manually, all is well: $ chmod +x absp.py $ git status # On branch master nothing to commit (working directory clean) This problem doesn't occur on ext3 or NFS file systems. Client is Ubuntu 12.04 with git version 1.7.9.5. CIFS is exported from Ubuntu 12.04 with Samba version 3.6.3. Since git recognises the x-flag on this CIFS file system, shouldn't it also be able to set it on checkout? Many thanks, Andre Esser -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails
On Wed, May 29, 2013 at 02:01:59PM +0200, Matthieu Moy wrote:
I wonder if we can do something like:
our $mw_operation;
$mediawiki-{config}-{on_error} = sub {
[...]
die $err\n;
};
Probably, but that would hardcode the fact that mediawiki errors are
fatal, while in an ideal world, some errors should be recoverable, and
some would require some cleanups before die-ing.
Fortunately this is perl, not C. We can catch and re-throw die
exceptions like:
my $mw_pages = eval { $mediawiki-list(...) };
if (!$mw_pages) {
# possibly continue to something else, or even...
clean_up();
die; # propagate $@
}
but it would require checking all of the call-sites. Another alternative
would be a wrapper function that each caller could opt into. But I
suspect the norm will be to die, so the exception model should make the
code cleaner.
Also, an error during the first mediawiki operation should not
necessarily have the same diagnosis hint as the others: if I just
did a successfull querry, and the next fails, it can hardly be an SSL
certificate error.
Yeah, my error template was just a sketch; I didn't look too carefully
at all of the callers, but the concept should be extensible.
I'll send a v2 that covers a bit more (at least, push and pull with an
invalid certificate both give the message).
Thanks.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-remote-mediawiki: better error message when HTTP(S) access fails
On Wed, May 29, 2013 at 02:06:29PM +0200, Matthieu Moy wrote: My use-case is an invalid SSL certificate. Pulling from the wiki with a recent version of libwww-perl fails, and git-remote-mediawiki gave no clue about the reason. Give the mediawiki API detailed error message, and since it is not so informative, hint the user about an invalid SSL certificate on https:// urls. Signed-off-by: Matthieu Moy matthieu@imag.fr Thanks, this looks like a reasonable stopping point for now. There are still some calls that do not check the result of the mediawiki calls at all, and I think they'll probably cause a perl error when they fail (for dereferencing undef). But these ones are the initial contact, so they should be the common ones to fail. Refactoring the whole error handling is a bit larger task and can wait until somebody is interested. -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
[PATCH v3] difftool --dir-diff: allow changing any clean working tree file
The temporary directory prepared by difftool --dir-diff to
show the result of a change can be modified by the user via
the tree diff program, and we try hard not to lose changes
to them after tree diff program returns to us.
However, the set of files to be copied back is computed
differently between --symlinks and --no-symlinks modes. The
former checks all paths that start out as identical to the
working tree file, while the latter checks paths that
already had a local modification in the working tree,
allowing changes made in the tree diff program to paths that
did not have any local change to be lost.
Signed-off-by: Kenichi Saita nito...@gmail.com
---
git-difftool.perl |9 ++---
t/t7800-difftool.sh | 19 +++
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 8a75205..e57d3d1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -85,13 +85,9 @@ sub exit_cleanup
sub use_wt_file
{
- my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+ my ($repo, $workdir, $file, $sha1) = @_;
my $null_sha1 = '0' x 40;
- if ($sha1 ne $null_sha1 and not $symlinks) {
- return 0;
- }
-
if (! -e $workdir/$file) {
# If the file doesn't exist in the working tree, we cannot
# use it.
@@ -213,8 +209,7 @@ EOF
if ($rmode ne $null_mode) {
my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
- $dst_path, $rsha1,
- $symlinks);
+ $dst_path, $rsha1);
if ($use) {
push @working_tree, $dst_path;
$wtindex .= $rmode $wt_sha1\t$dst_path\0;
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index d46f041..2418528 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -385,6 +385,25 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff
--symlink without unstage
test_cmp actual expect
'
+write_script modify-right-file \EOF
+echo new content $2/file
+EOF
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged change' '
+ test_when_finished git reset --hard
+ echo orig content file
+ git difftool -d $symlinks --extcmd $(pwd)/modify-right-file branch
+ echo new content expect
+ test_cmp expect file
+'
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged change'
'
+ test_when_finished git reset --hard
+ git difftool -d $symlinks --extcmd $(pwd)/modify-right-file branch
+ echo new content expect
+ test_cmp expect file
+'
+
write_script modify-file \EOF
echo new content file
EOF
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/25] object_array_remove_duplicates(): rewrite to reduce copying
Michael Haggerty mhag...@alum.mit.edu writes:
The old version copied one entry to its destination position, then
deleted any matching entries from the tail of the array. This
required the tail of the array to be copied multiple times. It didn't
affect the complexity of the algorithm because the whole tail has to
be searched through anyway. But all the copying was unnecessary.
Instead, check for the existence of an entry with the same name in the
*head* of the list before copying an entry to its final position.
This way each entry has to be copied at most one time.
Extract a helper function contains_name() to do a bit of the work.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
object.c | 32 +---
object.h | 6 +-
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/object.c b/object.c
index fcd4a82..10b5349 100644
--- a/object.c
+++ b/object.c
@@ -294,22 +294,32 @@ void object_array_filter(struct object_array *array,
array-nr = dst;
}
+/*
+ * Return true iff array already contains an entry with name.
+ */
+static int contains_name(struct object_array *array, const char *name)
+{
+ unsigned nr = array-nr, i;
+ struct object_array_entry *object = array-objects;
+
+ for (i = 0; i nr; i++, object++)
+ if (!strcmp(object-name, name))
+ return 1;
+ return 0;
+}
Because some codepaths (e.g. patch 14/25) stuff NULL in the name
field, we may want to be more careful with this.
This is not a new problem, and I think the longer term solution is
to get rid of object_array_remove_duplicates(), so it is perfectly
fine to leave this function broken with respect to NULL input as-is.
The only caller of remove-duplicates is bundle.c, which gets many
starting points and end points from the command line and tries to be
nice by removing obvious duplicates, e.g.
git bundle create t.bundle master master
but I think its logic of deduping is wrong. It runs dwim_ref() on
the incoming refs after the remove-duplicates call, so
git bundle create t.bundle master heads/mater
will end up with two copies of refs/heads/master. To fix it, the
code must dedup the result of running dwim_ref(), and at that point,
there is no reason to call object_array_remove_duplicates().
+
void object_array_remove_duplicates(struct object_array *array)
{
- unsigned int ref, src, dst;
+ unsigned nr = array-nr, src;
struct object_array_entry *objects = array-objects;
- for (ref = 0; ref + 1 array-nr; ref++) {
- for (src = ref + 1, dst = src;
- src array-nr;
- src++) {
- if (!strcmp(objects[ref].name, objects[src].name))
- continue;
- if (src != dst)
- objects[dst] = objects[src];
- dst++;
+ array-nr = 0;
+ for (src = 0; src nr; src++) {
+ if (!contains_name(array, objects[src].name)) {
+ if (src != array-nr)
+ objects[array-nr] = objects[src];
+ array-nr++;
}
- array-nr = dst;
}
}
diff --git a/object.h b/object.h
index 0d39ff4..6c1c27f 100644
--- a/object.h
+++ b/object.h
@@ -96,7 +96,11 @@ typedef int (*object_array_each_func_t)(struct
object_array_entry *, void *);
void object_array_filter(struct object_array *array,
object_array_each_func_t want, void *cb_data);
-void object_array_remove_duplicates(struct object_array *);
+/*
+ * Remove from array all but the first entry with a given name.
+ * Warning: this function uses an O(N^2) algorithm.
+ */
+void object_array_remove_duplicates(struct object_array *array);
void clear_object_flags(unsigned flags);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?
Am 29.05.2013 06:19, schrieb Duy Nguyen:
On Wed, May 29, 2013 at 10:41 AM, Duy Nguyen pclo...@gmail.com wrote:
The changes in this area since 1.8.2.3 seem to be Karsten's (I'm not
blaming, just wanted to narrow down the problem). The patterns of
interest seem to be
!/bin
/bin/*
!/bin/brew
Without !/bin v1.8.3 seems to behave the same as v1.8.2.3.
Karsten, the block /* Abort if the directory is excluded */ in
prep_exclude() seems to cause this. I think it goes through the
exclude patterns, hits !/bin, believes the patterns do not make
sense in this context and throws all away.
Yes, I forgot to check the ! flag to determine if the directory is really
excluded. I'll prepare a patch + test case for this.
@Øystein: in the meantime, could you check if this fixes the problem for you?
--- 8 ---
diff --git a/dir.c b/dir.c
index a5926fb..13858fe 100644
--- a/dir.c
+++ b/dir.c
@@ -821,6 +821,9 @@ static void prep_exclude(struct dir_struct *dir, const char
*base, int baselen)
dir-basebuf, stk-baselen - 1,
dir-basebuf + current, dt);
dir-basebuf[stk-baselen - 1] = '/';
+ if (dir-exclude
+ dir-exclude-flags EXC_FLAG_NEGATIVE)
+ dir-exclude = NULL;
if (dir-exclude) {
dir-basebuf[stk-baselen] = 0;
dir-exclude_stack = stk;
--
--
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 15/25] object_array_entry: fix memory handling of the name field
Michael Haggerty mhag...@alum.mit.edu writes:
Change the callers that were already passing copies to
add_object_array_with_mode() to either skip the copy, or (if the
memory needed to be allocated anyway) freeing the memory itself.
A part of this commit effectively reverts
70d26c6e76 read_revisions_from_stdin: make copies for handle_revision_arg
because the copying introduced by that commit (which is still
necessary) is now done at a deeper level.
Good. Thanks for working on this.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
bundle.c | 2 +-
object.c | 26 +++---
object.h | 8 +++-
revision.c | 6 --
4 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/bundle.c b/bundle.c
index 4b0e5cd..3d64311 100644
--- a/bundle.c
+++ b/bundle.c
@@ -281,7 +281,7 @@ int create_bundle(struct bundle_header *header, const
char *path,
if (!get_sha1_hex(buf.buf + 1, sha1)) {
struct object *object =
parse_object_or_die(sha1, buf.buf);
object-flags |= UNINTERESTING;
- add_pending_object(revs, object,
xstrdup(buf.buf));
+ add_pending_object(revs, object, buf.buf);
}
} else if (!get_sha1_hex(buf.buf, sha1)) {
struct object *object = parse_object_or_die(sha1,
buf.buf);
diff --git a/object.c b/object.c
index 10b5349..e4ff714 100644
--- a/object.c
+++ b/object.c
@@ -260,11 +260,18 @@ void add_object_array(struct object *obj, const char
*name, struct object_array
add_object_array_with_mode(obj, name, array, S_IFINVALID);
}
+/*
+ * A zero-length string to which object_array_entry::name can be
+ * initialized without requiring a malloc/free.
+ */
+char object_array_slopbuf[1];
+
void add_object_array_with_mode(struct object *obj, const char *name, struct
object_array *array, unsigned mode)
{
unsigned nr = array-nr;
unsigned alloc = array-alloc;
struct object_array_entry *objects = array-objects;
+ struct object_array_entry *entry;
if (nr = alloc) {
alloc = (alloc + 32) * 2;
@@ -272,9 +279,16 @@ void add_object_array_with_mode(struct object *obj,
const char *name, struct obj
array-alloc = alloc;
array-objects = objects;
}
- objects[nr].item = obj;
- objects[nr].name = name;
- objects[nr].mode = mode;
+ entry = objects[nr];
+ entry-item = obj;
+ if (!name)
+ entry-name = NULL;
+ else if (!*name)
+ /* Use our own empty string instead of allocating one: */
+ entry-name = object_array_slopbuf;
+ else
+ entry-name = xstrdup(name);
+ entry-mode = mode;
array-nr = ++nr;
}
@@ -289,6 +303,9 @@ void object_array_filter(struct object_array *array,
if (src != dst)
objects[dst] = objects[src];
dst++;
+ } else {
+ if (objects[src].name != object_array_slopbuf)
+ free(objects[src].name);
}
}
array-nr = dst;
@@ -319,6 +336,9 @@ void object_array_remove_duplicates(struct object_array
*array)
if (src != array-nr)
objects[array-nr] = objects[src];
array-nr++;
+ } else {
+ if (objects[src].name != object_array_slopbuf)
+ free(objects[src].name);
}
}
}
diff --git a/object.h b/object.h
index 6c1c27f..2ff68c5 100644
--- a/object.h
+++ b/object.h
@@ -11,7 +11,13 @@ struct object_array {
unsigned int alloc;
struct object_array_entry {
struct object *item;
- const char *name;
+ /*
+ * name or NULL. If non-NULL, the memory pointed to
+ * is owned by this object *except* if it points at
+ * object_array_slopbuf, which is a static copy of the
+ * empty string.
+ */
+ char *name;
unsigned mode;
} *objects;
};
diff --git a/revision.c b/revision.c
index be73cb4..4aeda33 100644
--- a/revision.c
+++ b/revision.c
@@ -88,7 +88,9 @@ void add_object(struct object *obj,
struct name_path *path,
const char *name)
{
- add_object_array(obj, path_name(path, name), p);
+ char *pn = path_name(path, name);
+ add_object_array(obj, pn, p);
+ free(pn);
}
static void mark_blob_uninteresting(struct blob *blob)
@@ -1288,7 +1290,7 @@ static void read_revisions_from_stdin(struct rev_info
*revs,
}
die(options not supported in --stdin mode);
}
-
Re: [PATCH v2 7/8] remote-bzr: reorganize the way 'wanted' works
Felipe Contreras felipe.contre...@gmail.com writes:
Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
+wanted = get_config('remote-bzr.branches').rstrip().split(', ')
Two minor nits and one design suggestion:
- Why rstrip() not strip()?
The purpose of the strip is to remove the _single_ \n at the end that
subprocess communicate. Maybe get_config() should do that.
It appears that this only is helping
an end-user mistake like this:
git config remote-bzr.branches 'trunk, devel, test '
without helping people who have done this:
git config remote-bzr.branches 'trunk, devel, test'
No, that's tnot it.
Yes, rstrip() will also lose LF at the end.
But it also is true that your code also removes the trailing extra
SP in the first example above, while not losing the extra SP in the
middle in the second example, no?
So where does that's tnot it come from? Is it true or false that
the former is helped while the latter is not?
- Is
git config remote-bzr.branches trunk,devel,test
a grave sin?
In other words, wouldn't we want something like this instead?
map(lambda s: s.strip(), get_config('...').split(','))
Yeah, that might make sense.
If you go that route, you do not even have to even say stupid
python. You can write a more meaningful list comprehension, e.g.
wanted = [s.strip() for s in get_config('...').split(',')]
without an unsightly lambda in it.
- Doesn't allowing multi-valued variable, e.g.
[remote-bzr]
branches = trunk
branches = devel
branches = test
make it easier for the user to manage this configuration by
e.g. selectively removing or adding tracked branches?
How would the 'git config' command look like?
Either way, that's orthogonal to this patch.
Yeah, I notice that single variable, split at comma comes way
before this series anyway, so it is too late (and it is not worth)
to fix it using multi-valued variables. It was just an if we were
designing this from scratch kind of suggestion.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] test: improve rebase -q test
Felipe Contreras felipe.contre...@gmail.com writes:
Junio C Hamano wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
Let's show the output so it's clear why it failed.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t3400-rebase.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..fb39531 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when
upstream arg is missing' '
test_expect_success 'rebase -q is quiet' '
git checkout -b quiet topic
git rebase -q master output.out 21
+ cat output.out
test ! -s output.out
'
It is one thing to avoid squelching output that naturally comes out
of command being tested unnecessarily, so that ./t-*.sh -v
output can be used for debugging. I however am not sure if adding
cat to random places like this is a productive direction for us to
go in.
A more preferrable alternative may be adding something like this to
test-lib.sh and call it from here and elsewhere (there are about 50
places that do test ! -s filename), perhaps?
test_must_be_an_empty_file () {
if test -s $1
then
cat $1
false
fi
}
Perhaps, but I'm not interested. I'm tired of obvious fixes getting rejected
for hypothetical ideal situations that we'll never reach.
That's too bad. Addition of cat where there does not need one is
clearly not an obvious fix anyway.
--
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 25/25] refs: document the lifetime of the args passed to each_ref_fn
Michael Haggerty mhag...@alum.mit.edu writes:
The commits leading up to this have (hopefully) fixed all of the
callers of the for_each_ref()-like functions. This commit does the
last step: documents what each_ref_fn callbacks can assume about
object lifetimes.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
I looked at all the patches in the series and they all looked
sensible.
I have a few comments (sent as individual review) but all of the
suggestions in them are by the way I noticed this issue that is not
new to this series while I was reading the code, and we may want to
fix it elsewhere, not this is broken in the patch, we need to fix
it before queuing.
Thanks for a pleasant read.
---
refs.h | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/refs.h b/refs.h
index a35eafc..122ec03 100644
--- a/refs.h
+++ b/refs.h
@@ -15,13 +15,23 @@ struct ref_lock {
#define REF_ISBROKEN 0x04
/*
- * Calls the specified function for each ref file until it returns
- * nonzero, and returns the value. Please note that it is not safe to
- * modify references while an iteration is in progress, unless the
- * same callback function invocation that modifies the reference also
- * returns a nonzero value to immediately stop the iteration.
+ * The signature for the callback function for the for_each_*()
+ * functions below. The memory pointed to by the refname and sha1
+ * arguments is only guaranteed to be valid for the duration of a
+ * single callback invocation.
+ */
+typedef int each_ref_fn(const char *refname,
+ const unsigned char *sha1, int flags, void *cb_data);
+
+/*
+ * The following functions invoke the specified callback function for
+ * each reference indicated. If the function ever returns a nonzero
+ * value, stop the iteration and return that value. Please note that
+ * it is not safe to modify references while an iteration is in
+ * progress, unless the same callback function invocation that
+ * modifies the reference also returns a nonzero value to immediately
+ * stop the iteration.
*/
-typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int
flags, void *cb_data);
extern int head_ref(each_ref_fn, void *);
extern int for_each_ref(each_ref_fn, void *);
extern int for_each_ref_in(const char *, each_ref_fn, void *);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] remote-bzr: reorganize the way 'wanted' works
On Wed, May 29, 2013 at 11:42 AM, Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
+wanted = get_config('remote-bzr.branches').rstrip().split(', ')
Two minor nits and one design suggestion:
- Why rstrip() not strip()?
The purpose of the strip is to remove the _single_ \n at the end that
subprocess communicate. Maybe get_config() should do that.
It appears that this only is helping
an end-user mistake like this:
git config remote-bzr.branches 'trunk, devel, test '
without helping people who have done this:
git config remote-bzr.branches 'trunk, devel, test'
No, that's tnot it.
Yes, rstrip() will also lose LF at the end.
But it also is true that your code also removes the trailing extra
SP in the first example above, while not losing the extra SP in the
middle in the second example, no?
So where does that's tnot it come from? Is it true or false that
the former is helped while the latter is not?
You said it is *only* helping the end-user with mistakes, but that's
not true, it _also_ gets rid of the new line, which is not only
helping, but it's required for the code to work, and it's actually the
purpose behind the code. The side-effect of removing extra spaces if
the user makes mistakes is irrelevant.
- Is
git config remote-bzr.branches trunk,devel,test
a grave sin?
In other words, wouldn't we want something like this instead?
map(lambda s: s.strip(), get_config('...').split(','))
Yeah, that might make sense.
If you go that route, you do not even have to even say stupid
python. You can write a more meaningful list comprehension, e.g.
wanted = [s.strip() for s in get_config('...').split(',')]
without an unsightly lambda in it.
Python would still do the stupid thing if there's no such configuration:
['']
But we can add 'if s' at the end, so the code to fix python's
stupidness is much smaller.
I wonder what 's' means. In C I use 'i', because that's what everybody
uses, but in more functional-like code we don't use an index, we
iterate directly with the element of an enumerable, so I say 'e' for
short.
- Doesn't allowing multi-valued variable, e.g.
[remote-bzr]
branches = trunk
branches = devel
branches = test
make it easier for the user to manage this configuration by
e.g. selectively removing or adding tracked branches?
How would the 'git config' command look like?
Either way, that's orthogonal to this patch.
Yeah, I notice that single variable, split at comma comes way
before this series anyway, so it is too late (and it is not worth)
to fix it using multi-valued variables. It was just an if we were
designing this from scratch kind of suggestion.
It might be worth, I'm not sure, I'm not familiar with those, and I
don't know what the user would have to type, but my guess is that it
won't be as user friendly as 'git config foo one,two,three'.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] test: improve rebase -q test
On Wed, May 29, 2013 at 11:52 AM, Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
Junio C Hamano wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
Let's show the output so it's clear why it failed.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t3400-rebase.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..fb39531 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when
upstream arg is missing' '
test_expect_success 'rebase -q is quiet' '
git checkout -b quiet topic
git rebase -q master output.out 21
+ cat output.out
test ! -s output.out
'
It is one thing to avoid squelching output that naturally comes out
of command being tested unnecessarily, so that ./t-*.sh -v
output can be used for debugging. I however am not sure if adding
cat to random places like this is a productive direction for us to
go in.
A more preferrable alternative may be adding something like this to
test-lib.sh and call it from here and elsewhere (there are about 50
places that do test ! -s filename), perhaps?
test_must_be_an_empty_file () {
if test -s $1
then
cat $1
false
fi
}
Perhaps, but I'm not interested. I'm tired of obvious fixes getting rejected
for hypothetical ideal situations that we'll never reach.
That's too bad. Addition of cat where there does not need one is
clearly not an obvious fix anyway.
If you are an actual real user of this code; a developer that is
running the test; and the test finally achieves it's designed goal of
detecting a failure, you would be left scratching your head wondering
what's the problem if running './test -v' doesn't show anything, even
after you have added debugging code to narrow down the issue.
I had to add that cat line not once, but more than two times in
different lines of development.
So yeah, a cat is needed, and the fact you don't see that amazes me,
specially after you have reprimanded me for using 'grep -q' instead of
'grep' for this very reason.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] remote-helpers: rename tests
Felipe Contreras felipe.contre...@gmail.com writes: I do not see how it makes sense to copy how they deviate from us back to our codebase, especially if we plan to eventually move some of these tests out of contrib/ area, but even without such a plan in the future. They deviate from us, we deviate from them, whatever. We are a single project, what more than one project does is more standard. We are a single project, so it is better to consistently follow the local convention established here. If your proposal were to - Convert t/*.sh to end with .t intead, to change the project convention, and - Make contrib/ things also conform to that new convention. it may make some sense to discuss the pros and cons of such a move, but changing only contrib/ has no effect other than making it even less consistent with the rest of the project. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #08; Tue, 28)
Felipe Contreras felipe.contre...@gmail.com writes: Junio C Hamano wrote: * fc/makefile (2013-05-26) 5 commits - build: do not install git-remote-testpy - build: add NO_INSTALL variable - build: cleanup using $ - build: cleanup using $^ - build: trivial simplification (this branch is used by fc/remote-helpers-use-specified-python.) No, these are independent. By the way, I dropped the order-only one and I explained my reasoning for doing so, but I haven't heard back from you. As I haven't used the order-only dependencies nor '|' myself so far (primarily because I have not seen a case where it was needed), it would have been nice if you responded with either yes, this is not order-only and the patch should be dropped, or no, order-only is correct because In any case, I think the above remaining five were sensible changes, and am thinking about having it graduating early in the cycle. I somehow had an impression that the other series depended on this for SCRIPT_PYTHON_* stuff, but this is about the installation step and the other one is primarily about the build step, so in that sense it may be independent. * fc/remote-bzr (2013-05-28) 8 commits - remote-bzr: add fallback check for a partial clone - remote-bzr: reorganize the way 'wanted' works - remote-bzr: trivial cleanups - remote-bzr: change global repo - remote-bzr: delay cloning/pulling - remote-bzr: simplify get_remote_branch() - remote-bzr: fix for files with spaces - remote-bzr: recover from failed clones The ones near the tip conflicted with the hotfix for 1.8.3 so I discarded them for now. Will merge to 'next'? Didn't I resend these with the conflict fixed? As the date above (05-28) shows, it seems that I did not forget to drop the old one to replace with the new one, but I did forget to remove the stale comment from the previous issue. Thanks for noticing. -- 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] fixup! rebase: implement --[no-]autostash and rebase.autostash
Ramkumar Ramachandra artag...@gmail.com writes:
For rr/rebase-autostash, which is stalled in pu. See $gmane/225689.
This is a super-minor fix anyway: if you disagree with something, change
it; there's no need to ask me.
As I wasn't the one who were disagreeing, that would not work
well.
Was there anybody disagreeing in the first place? I thought
everybody was helping the series to get better.
git-rebase.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 709ef6b..5906757 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -151,16 +151,16 @@ finish_rebase () {
stash_sha1=$(cat $state_dir/autostash)
if git stash apply $stash_sha1 21 /dev/null
then
- echo Applied autostash
+ echo $(gettext 'Applied autostash.')
else
ref_stash=refs/stash
: $GIT_DIR/logs/$ref_stash
git update-ref -m autostash $ref_stash $stash_sha1 \
|| die $(eval_gettext 'Cannot store
$stash_sha1')
- echo
-$(gettext 'Applying autostash resulted in conflicts.
+ gettext 'Applying autostash resulted in conflicts.
Your changes are safe in the stash.
-You can apply or drop it at any time.')
+You can run git stash pop or git stash drop it at any time.
+'
fi
fi
git gc --auto
--
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] fixup! rebase: implement --[no-]autostash and rebase.autostash
Junio C Hamano wrote: As I wasn't the one who were disagreeing, that would not work well. I meant in the tiny details like echo + gettext versus gettext. From the review of v3, nobody had any disagreements; just minor suggestions: that's what this patch is about anyway. -- 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
