* Re: [PATCH 3/2] t5547-push-quarantine: run the path separator test on Windows, too
From: Junio C Hamano @ 2016-12-13 19:23 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Klaus Ethgen, git
In-Reply-To: <d9d2580c-a2e5-d9f3-1f56-6814b2b2285d@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> To perform the test case on Windows in a way that corresponds to the
> POSIX version, inject the semicolon in a directory name.
>
> Typically, an absolute POSIX style path, such as the one in $PWD, is
> translated into a Windows style path by bash when it invokes git.exe.
> However, the presence of the semicolon suppresses this translation;
> but the untranslated POSIX style path is useless for git.exe.
> Therefore, instead of $PWD pass the Windows style path that $(pwd)
> produces.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> Am 12.12.2016 um 20:53 schrieb Jeff King:
>> Johannes, please let me know if I am wrong about skipping the test on
>> !MINGW. The appropriate check there would be ";" anyway, but I am not
>> sure _that_ is allowed in paths, either.
>
> Here is a version for Windows. I'd prefer this patch on top instead
> of squashing it into yours to keep the $PWD vs. $(pwd) explanation.
>
> The result is the same as yours in all practical matters; but this
> version I have already tested.
Will queue (I would wait for peff@ to say "OK", but I suspect he
would be OK in this case).
Thanks.
> t/t5547-push-quarantine.sh | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
> index 6275ec807b..af9fcd833a 100755
> --- a/t/t5547-push-quarantine.sh
> +++ b/t/t5547-push-quarantine.sh
> @@ -33,8 +33,7 @@ test_expect_success 'rejected objects are removed' '
> test_cmp expect actual
> '
>
> -# MINGW does not allow colons in pathnames in the first place
> -test_expect_success !MINGW 'push to repo path with colon' '
> +test_expect_success 'push to repo path with path separator (colon)' '
> # The interesting failure case here is when the
> # receiving end cannot access its original object directory,
> # so make it likely for us to generate a delta by having
> @@ -43,13 +42,20 @@ test_expect_success !MINGW 'push to repo path with colon' '
> test-genrandom foo 4096 >file.bin &&
> git add file.bin &&
> git commit -m bin &&
> - git clone --bare . xxx:yyy.git &&
> +
> + if test_have_prereq MINGW
> + then
> + pathsep=";"
> + else
> + pathsep=":"
> + fi &&
> + git clone --bare . "xxx${pathsep}yyy.git" &&
>
> echo change >>file.bin &&
> git commit -am change &&
> # Note that we have to use the full path here, or it gets confused
> # with the ssh host:path syntax.
> - git push "$PWD/xxx:yyy.git" HEAD
> + git push "$(pwd)/xxx${pathsep}yyy.git" HEAD
> '
>
> test_done
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #02; Mon, 12)
From: Johannes Schindelin @ 2016-12-13 19:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqoa0g96o3.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 12 Dec 2016, Junio C Hamano wrote:
> * bw/grep-recurse-submodules (2016-11-22) 6 commits
> - grep: search history of moved submodules
> - grep: enable recurse-submodules to work on <tree> objects
> - grep: optionally recurse into submodules
> - grep: add submodules as a grep source type
> - submodules: load gitmodules file from commit sha1
> - submodules: add helper functions to determine presence of submodules
>
> "git grep" learns to optionally recurse into submodules
>
> Has anybody else seen t7814 being flakey with this series?
It is not flakey for me, it fails consistently on Windows. This is the
output with -i -v -x (sorry, I won't have time this week to do anything
about it, but maybe it helps identify the root cause):
-- snipsnap --
Initialized empty Git repository in C:/git-sdk-64/usr/src/git/wip3/t/trash
directory.t7814-grep-recurse-submodules/.git/
expecting success:
echo "foobar" >a &&
mkdir b &&
echo "bar" >b/b &&
git add a b &&
git commit -m "add a and b" &&
git init submodule &&
echo "foobar" >submodule/a &&
git -C submodule add a &&
git -C submodule commit -m "add a" &&
git submodule add ./submodule &&
git commit -m "added submodule"
++ echo foobar
++ mkdir b
++ echo bar
++ git add a b
++ git commit -m 'add a and b'
[master (root-commit) 6a17548] add a and b
Author: A U Thor <author@example.com>
2 files changed, 2 insertions(+)
create mode 100644 a
create mode 100644 b/b
++ git init submodule
Initialized empty Git repository in C:/git-sdk-64/usr/src/git/wip3/t/trash
directory.t7814-grep-recurse-submodules/submodule/.git/
++ echo foobar
++ git -C submodule add a
++ git -C submodule commit -m 'add a'
[master (root-commit) 081a998] add a
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 a
++ git submodule add ./submodule
Adding existing repo at 'submodule' to the index
++ git commit -m 'added submodule'
[master 0c0fdd0] added submodule
Author: A U Thor <author@example.com>
2 files changed, 4 insertions(+)
create mode 100644 .gitmodules
create mode 160000 submodule
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 1 - setup directory structure and submodule
expecting success:
cat >expect <<-\EOF &&
a:foobar
b/b:bar
submodule/a:foobar
EOF
git grep -e "bar" --recurse-submodules >actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='b/b:bar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
b/b:bar
submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='b/b:bar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
b/b:bar
submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'a:foobar
b/b:bar
submodule/a:foobar
'
++ test -n 'a:foobar
b/b:bar
submodule/a:foobar
'
++ test 'a:foobar
b/b:bar
submodule/a:foobar
' = 'a:foobar
b/b:bar
submodule/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 2 - grep correctly finds patterns in a submodule
expecting success:
cat >expect <<-\EOF &&
submodule/a:foobar
EOF
git grep -e. --recurse-submodules -- submodule >actual &&
test_cmp expect actual
++ cat
++ git grep -e. --recurse-submodules -- submodule
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'submodule/a:foobar
'
++ test -n 'submodule/a:foobar
'
++ test 'submodule/a:foobar
' = 'submodule/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 3 - grep and basic pathspecs
expecting success:
git init submodule/sub &&
echo "foobar" >submodule/sub/a &&
git -C submodule/sub add a &&
git -C submodule/sub commit -m "add a" &&
git -C submodule submodule add ./sub &&
git -C submodule add sub &&
git -C submodule commit -m "added sub" &&
git add submodule &&
git commit -m "updated submodule" &&
cat >expect <<-\EOF &&
a:foobar
b/b:bar
submodule/a:foobar
submodule/sub/a:foobar
EOF
git grep -e "bar" --recurse-submodules >actual &&
test_cmp expect actual
++ git init submodule/sub
Initialized empty Git repository in C:/git-sdk-64/usr/src/git/wip3/t/trash
directory.t7814-grep-recurse-submodules/submodule/sub/.git/
++ echo foobar
++ git -C submodule/sub add a
++ git -C submodule/sub commit -m 'add a'
[master (root-commit) b95b263] add a
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 a
++ git -C submodule submodule add ./sub
Adding existing repo at 'sub' to the index
++ git -C submodule add sub
++ git -C submodule commit -m 'added sub'
[master 190608e] added sub
Author: A U Thor <author@example.com>
2 files changed, 4 insertions(+)
create mode 100644 .gitmodules
create mode 160000 sub
++ git add submodule
++ git commit -m 'updated submodule'
[master 5198849] updated submodule
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+), 1 deletion(-)
++ cat
++ git grep -e bar --recurse-submodules
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='b/b:bar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
b/b:bar
submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/sub/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
b/b:bar
submodule/a:foobar
submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='b/b:bar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
b/b:bar
submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/sub/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
b/b:bar
submodule/a:foobar
submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'a:foobar
b/b:bar
submodule/a:foobar
submodule/sub/a:foobar
'
++ test -n 'a:foobar
b/b:bar
submodule/a:foobar
submodule/sub/a:foobar
'
++ test 'a:foobar
b/b:bar
submodule/a:foobar
submodule/sub/a:foobar
' = 'a:foobar
b/b:bar
submodule/a:foobar
submodule/sub/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 4 - grep and nested submodules
expecting success:
cat >expect <<-\EOF &&
a:foobar
submodule/a:foobar
submodule/sub/a:foobar
EOF
git grep -e "bar" --and -e "foo" --recurse-submodules >actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --and -e foo --recurse-submodules
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/sub/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='a:foobar
submodule/a:foobar
submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='submodule/sub/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='a:foobar
submodule/a:foobar
submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'a:foobar
submodule/a:foobar
submodule/sub/a:foobar
'
++ test -n 'a:foobar
submodule/a:foobar
submodule/sub/a:foobar
'
++ test 'a:foobar
submodule/a:foobar
submodule/sub/a:foobar
' = 'a:foobar
submodule/a:foobar
submodule/sub/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 5 - grep and multiple patterns
expecting success:
cat >expect <<-\EOF &&
b/b:bar
EOF
git grep -e "bar" --and --not -e "foo" --recurse-submodules
>actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --and --not -e foo --recurse-submodules
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='b/b:bar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='b/b:bar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'b/b:bar
'
++ test -n 'b/b:bar
'
++ test 'b/b:bar
' = 'b/b:bar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 6 - grep and multiple patterns
expecting success:
cat >expect <<-\EOF &&
HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
EOF
git grep -e "bar" --recurse-submodules HEAD >actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules HEAD
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:b/b:bar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:a:foobar
HEAD:b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:b/b:bar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:a:foobar
HEAD:b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ test -n 'HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ test 'HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
' = 'HEAD:a:foobar
HEAD:b/b:bar
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 7 - basic grep tree
expecting success:
cat >expect <<-\EOF &&
HEAD^:a:foobar
HEAD^:b/b:bar
HEAD^:submodule/a:foobar
EOF
git grep -e "bar" --recurse-submodules HEAD^ >actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules 'HEAD^'
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^:a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD^:a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^:b/b:bar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD^:a:foobar
HEAD^:b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^:submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD^:a:foobar
HEAD^:b/b:bar
HEAD^:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^:a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD^:a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^:b/b:bar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD^:a:foobar
HEAD^:b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^:submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD^:a:foobar
HEAD^:b/b:bar
HEAD^:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'HEAD^:a:foobar
HEAD^:b/b:bar
HEAD^:submodule/a:foobar
'
++ test -n 'HEAD^:a:foobar
HEAD^:b/b:bar
HEAD^:submodule/a:foobar
'
++ test 'HEAD^:a:foobar
HEAD^:b/b:bar
HEAD^:submodule/a:foobar
' = 'HEAD^:a:foobar
HEAD^:b/b:bar
HEAD^:submodule/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 8 - grep tree HEAD^
expecting success:
cat >expect <<-\EOF &&
HEAD^^:a:foobar
HEAD^^:b/b:bar
EOF
git grep -e "bar" --recurse-submodules HEAD^^ >actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules 'HEAD^^'
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^^:a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD^^:a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^^:b/b:bar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD^^:a:foobar
HEAD^^:b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^^:a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD^^:a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD^^:b/b:bar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD^^:a:foobar
HEAD^^:b/b:bar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'HEAD^^:a:foobar
HEAD^^:b/b:bar
'
++ test -n 'HEAD^^:a:foobar
HEAD^^:b/b:bar
'
++ test 'HEAD^^:a:foobar
HEAD^^:b/b:bar
' = 'HEAD^^:a:foobar
HEAD^^:b/b:bar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 9 - grep tree HEAD^^
expecting success:
cat >expect <<-\EOF &&
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
EOF
git grep -e "bar" --recurse-submodules HEAD -- submodule >actual
&&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules HEAD -- submodule
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ test -n 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ test 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
' = 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 10 - grep tree and pathspecs
expecting success:
cat >expect <<-\EOF &&
HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
EOF
git grep -e "bar" --recurse-submodules HEAD -- "submodule*a"
>actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules HEAD -- 'submodule*a'
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ test -n 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
++ test 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
' = 'HEAD:submodule/a:foobar
HEAD:submodule/sub/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 11 - grep tree and pathspecs
expecting success:
cat >expect <<-\EOF &&
HEAD:submodule/a:foobar
EOF
git grep -e "bar" --recurse-submodules HEAD -- "submodul?/a"
>actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules HEAD -- 'submodul?/a'
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:submodule/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'HEAD:submodule/a:foobar
'
++ test -n 'HEAD:submodule/a:foobar
'
++ test 'HEAD:submodule/a:foobar
' = 'HEAD:submodule/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 12 - grep tree and more pathspecs
expecting success:
cat >expect <<-\EOF &&
HEAD:submodule/sub/a:foobar
EOF
git grep -e "bar" --recurse-submodules HEAD -- "submodul*/sub/a"
>actual &&
test_cmp expect actual
++ cat
++ git grep -e bar --recurse-submodules HEAD -- 'submodul*/sub/a'
++ test_cmp expect actual
++ mingw_test_cmp expect actual
++ local test_cmp_a= test_cmp_b=
++ local stdin_for_diff=
++ test -s expect
++ test -s actual
++ mingw_read_file_strip_cr_ test_cmp_a
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_a=$test_cmp_a$line'
+++ test_cmp_a='HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ mingw_read_file_strip_cr_ test_cmp_b
++ local line
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ line='HEAD:submodule/sub/a:foobar
'
++ eval 'test_cmp_b=$test_cmp_b$line'
+++ test_cmp_b='HEAD:submodule/sub/a:foobar
'
++ :
++ IFS=$'\r'
++ read -r -d '
' line
++ test -z ''
++ break
++ test -n 'HEAD:submodule/sub/a:foobar
'
++ test -n 'HEAD:submodule/sub/a:foobar
'
++ test 'HEAD:submodule/sub/a:foobar
' = 'HEAD:submodule/sub/a:foobar
'
+ test_eval_ret_=0
+ want_trace
+ test t = t
+ test t = t
+ set +x
ok 13 - grep tree and more pathspecs
expecting success:
git init parent &&
test_when_finished "rm -rf parent" &&
echo "foobar" >"parent/fi:le" &&
git -C parent add "fi:le" &&
git -C parent commit -m "add fi:le" &&
git init "su:b" &&
test_when_finished "rm -rf su:b" &&
echo "foobar" >"su:b/fi:le" &&
git -C "su:b" add "fi:le" &&
git -C "su:b" commit -m "add fi:le" &&
git -C parent submodule add "../su:b" "su:b" &&
git -C parent commit -m "add submodule" &&
cat >expect <<-\EOF &&
fi:le:foobar
su:b/fi:le:foobar
EOF
git -C parent grep -e "foobar" --recurse-submodules >actual &&
test_cmp expect actual &&
cat >expect <<-\EOF &&
HEAD:fi:le:foobar
HEAD:su:b/fi:le:foobar
EOF
git -C parent grep -e "foobar" --recurse-submodules HEAD >actual
&&
test_cmp expect actual
++ git init parent
Initialized empty Git repository in C:/git-sdk-64/usr/src/git/wip3/t/trash
directory.t7814-grep-recurse-submodules/parent/.git/
++ test_when_finished 'rm -rf parent'
++ test 0 = 0
++ test_cleanup='{ rm -rf parent
} && (exit "$eval_ret"); eval_ret=$?; :'
++ echo foobar
++ git -C parent add fi:le
fatal: pathspec 'fi:le' did not match any files
+ test_eval_ret_=128
+ want_trace
+ test t = t
+ test t = t
+ set +x
error: last command exited with $?=128
not ok 14 - grep recurse submodule colon in name
#
# git init parent &&
# test_when_finished "rm -rf parent" &&
# echo "foobar" >"parent/fi:le" &&
# git -C parent add "fi:le" &&
# git -C parent commit -m "add fi:le" &&
#
# git init "su:b" &&
# test_when_finished "rm -rf su:b" &&
# echo "foobar" >"su:b/fi:le" &&
# git -C "su:b" add "fi:le" &&
# git -C "su:b" commit -m "add fi:le" &&
#
# git -C parent submodule add "../su:b" "su:b" &&
# git -C parent commit -m "add submodule" &&
#
# cat >expect <<-\EOF &&
# fi:le:foobar
# su:b/fi:le:foobar
# EOF
# git -C parent grep -e "foobar" --recurse-submodules
# >actual &&
# test_cmp expect actual &&
#
# cat >expect <<-\EOF &&
# HEAD:fi:le:foobar
# HEAD:su:b/fi:le:foobar
# EOF
# git -C parent grep -e "foobar" --recurse-submodules HEAD
# >actual &&
# test_cmp expect actual
#
^ permalink raw reply
* Re: [PATCH 3/2] t5547-push-quarantine: run the path separator test on Windows, too
From: Jeff King @ 2016-12-13 19:12 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <d9d2580c-a2e5-d9f3-1f56-6814b2b2285d@kdbg.org>
On Tue, Dec 13, 2016 at 08:09:31PM +0100, Johannes Sixt wrote:
> Am 12.12.2016 um 20:53 schrieb Jeff King:
> > Johannes, please let me know if I am wrong about skipping the test on
> > !MINGW. The appropriate check there would be ";" anyway, but I am not
> > sure _that_ is allowed in paths, either.
>
> Here is a version for Windows. I'd prefer this patch on top instead
> of squashing it into yours to keep the $PWD vs. $(pwd) explanation.
>
> The result is the same as yours in all practical matters; but this
> version I have already tested.
Yeah, I'm happy to have this on top. The patch itself looks obviously
correct. Thanks!
-Peff
^ permalink raw reply
* Re: git add -p with new file
From: Junio C Hamano @ 2016-12-13 19:12 UTC (permalink / raw)
To: Jeff King; +Cc: Stephan Beyer, Ariel, git
In-Reply-To: <20161213185653.ys3ig377zhmblncl@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Perhaps the latter is not advertised well enough? "add -p" does not
>> even page so it is not very useful way to check what is being added
>> if you are adding a new file (unless you are doing a toy example to
>> add a 7-line file).
>
> I use "add -p" routinely for my final add-and-sanity-check,...
> ... To me they are all tools in the toolbox, and I can pick the one that
> works best in any given situation, or that I just feel like using that
> day.
Oh, there is no question about that. I was just pointing out that
"add -p" is not the "one that works best" when dealing with a path
that is not yet even in the index.
^ permalink raw reply
* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Junio C Hamano @ 2016-12-13 19:11 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <CAGZ79kY_E8xnOpCAFQo_91FeQCs9X3fkassFYunG=adx81AcBg@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> I do not think there is no dispute about what embedding means.
>
> double negative: You think we have a slight dispute here.
Sorry, I do not think there is any dispute on that.
>> A
>> submodule whose .git is inside its working tree has its repository
>> embedded.
>>
>> What we had trouble settling on was what to call the operation to
>> undo the embedding, unentangling its repository out of the working
>> tree. I'd still vote for unembed if you want a name to be nominated.
>
> So I can redo the series with two commands "git submodule [un]embed".
>
> For me "unembed" == "absorb", such that we could also go with
> absorb into superproject <-> embed into worktree
With us agreeing that "embed" is about something is _IN_ submodule
working tree, unembed would naturally be something becomes OUTSIDE
the same thing (i.e. "submodule working tree"). However, if you
introduce "absorb", we suddenly need to talk about a different
thing, i.e. "superproject's .git/modules", that is doing the
absorption. That is why I suggest "unembed" over "absorb".
^ permalink raw reply
* Re: [PATCH] builtin/commit.c: convert trivial snprintf calls to xsnprintf
From: Jeff King @ 2016-12-13 19:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elia Pinto, git
In-Reply-To: <xmqqy3zj3b3a.fsf@gitster.mtv.corp.google.com>
On Tue, Dec 13, 2016 at 11:03:53AM -0800, Junio C Hamano wrote:
> >> @@ -1525,12 +1526,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
> >> static int run_rewrite_hook(const unsigned char *oldsha1,
> >> const unsigned char *newsha1)
> >> {
> >> - /* oldsha1 SP newsha1 LF NUL */
> >> - static char buf[2*40 + 3];
> >> + char *buf;
> >> struct child_process proc = CHILD_PROCESS_INIT;
> >> const char *argv[3];
> >> int code;
> >> - size_t n;
> >>
> >> argv[0] = find_hook("post-rewrite");
> >> if (!argv[0])
> >> @@ -1546,34 +1545,33 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
> >> code = start_command(&proc);
> >> if (code)
> >> return code;
> >> - n = snprintf(buf, sizeof(buf), "%s %s\n",
> >> - sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
> >> + buf = xstrfmt("%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
> >> sigchain_push(SIGPIPE, SIG_IGN);
> >> - write_in_full(proc.in, buf, n);
> >> + write_in_full(proc.in, buf, strlen(buf));
> >> close(proc.in);
> >> + free(buf);
> >
> > Any time you care about the length of the result, I'd generally use an
> > actual strbuf instead of xstrfmt. The extra strlen isn't a big deal
> > here, but it somehow seems simpler to me. It probably doesn't matter
> > much either way, though.
>
> Your justification for this extra allocation was that it is a
> heavy-weight operation. While I agree that the runtime cost of
> allocation and deallocation does not matter, I would be a bit
> worried about extra cognitive burden to programmers. They did not
> have to worry about leaking because they are writing a fixed length
> string. Now they do, whether they use xstrfmt() or struct strbuf.
> When they need to update what they write, they do have to remember
> to adjust the size of the "fixed string", and the original is not
> free from the "programmers' cognitive cost" point of view, of
> course. Probably use of strbuf/xstrfmt is an overall win.
So I think you are agreeing, but I have a minor nit to pick. :)
The fact that the extra allocation will not hurt performance is
_necessary_, but not _sufficient_. So it's not a justification in
itself, only something we have to check before proceeding.
The only justification here is that magic numbers like "2*40 + 3" are
confusing and a potential maintenance burden. And that's why I suggested
splitting this one out from the other two (whose justification is
"PATH_MAX is sometimes too small").
I agree with you that it's a tradeoff between "magic numbers" versus
"having to free resources". In my opinion it's a net improvement, but I
think it would also be reasonable to switch to xsnprintf() here. Then
the programmer has an automatic check that the buffer size is
sufficient.
-Peff
^ permalink raw reply
* [PATCH 3/2] t5547-push-quarantine: run the path separator test on Windows, too
From: Johannes Sixt @ 2016-12-13 19:09 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <20161212195355.znqlu44lgnke3ltc@sigill.intra.peff.net>
To perform the test case on Windows in a way that corresponds to the
POSIX version, inject the semicolon in a directory name.
Typically, an absolute POSIX style path, such as the one in $PWD, is
translated into a Windows style path by bash when it invokes git.exe.
However, the presence of the semicolon suppresses this translation;
but the untranslated POSIX style path is useless for git.exe.
Therefore, instead of $PWD pass the Windows style path that $(pwd)
produces.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 12.12.2016 um 20:53 schrieb Jeff King:
> Johannes, please let me know if I am wrong about skipping the test on
> !MINGW. The appropriate check there would be ";" anyway, but I am not
> sure _that_ is allowed in paths, either.
Here is a version for Windows. I'd prefer this patch on top instead
of squashing it into yours to keep the $PWD vs. $(pwd) explanation.
The result is the same as yours in all practical matters; but this
version I have already tested.
t/t5547-push-quarantine.sh | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 6275ec807b..af9fcd833a 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -33,8 +33,7 @@ test_expect_success 'rejected objects are removed' '
test_cmp expect actual
'
-# MINGW does not allow colons in pathnames in the first place
-test_expect_success !MINGW 'push to repo path with colon' '
+test_expect_success 'push to repo path with path separator (colon)' '
# The interesting failure case here is when the
# receiving end cannot access its original object directory,
# so make it likely for us to generate a delta by having
@@ -43,13 +42,20 @@ test_expect_success !MINGW 'push to repo path with colon' '
test-genrandom foo 4096 >file.bin &&
git add file.bin &&
git commit -m bin &&
- git clone --bare . xxx:yyy.git &&
+
+ if test_have_prereq MINGW
+ then
+ pathsep=";"
+ else
+ pathsep=":"
+ fi &&
+ git clone --bare . "xxx${pathsep}yyy.git" &&
echo change >>file.bin &&
git commit -am change &&
# Note that we have to use the full path here, or it gets confused
# with the ssh host:path syntax.
- git push "$PWD/xxx:yyy.git" HEAD
+ git push "$(pwd)/xxx${pathsep}yyy.git" HEAD
'
test_done
--
2.11.0.55.g6a4dbb1
^ permalink raw reply related
* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Stefan Beller @ 2016-12-13 19:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <xmqq37hr4q5t.fsf@gitster.mtv.corp.google.com>
On Tue, Dec 13, 2016 at 10:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Mon, Dec 12, 2016 at 11:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Stefan Beller <sbeller@google.com> writes:
>>>
>>>> The "checkout --recurse-submodules" series got too large to comfortably send
>>>> it out for review, so I had to break it up into smaller series'; this is the
>>>> first subseries, but it makes sense on its own.
>>>>
>>>> This series teaches git-rm to absorb the git directory of a submodule instead
>>>> of failing and complaining about the git directory preventing deletion.
>>>>
>>>> It applies on origin/sb/submodule-embed-gitdir.
>>>
>>> Thanks. I probably should rename the topic again with s/embed/absorb/;
>>
>> I mostly renamed it in the hope to settle our dispute what embedding means. ;)
>
> I do not think there is no dispute about what embedding means.
double negative: You think we have a slight dispute here.
> A
> submodule whose .git is inside its working tree has its repository
> embedded.
>
> What we had trouble settling on was what to call the operation to
> undo the embedding, unentangling its repository out of the working
> tree. I'd still vote for unembed if you want a name to be nominated.
So I can redo the series with two commands "git submodule [un]embed".
For me "unembed" == "absorb", such that we could also go with
absorb into superproject <-> embed into worktree
>
>> Note that sb/t3600-cleanup is part of this series now,
>> (The first commit of that series is in patch 6/6 of this series, and patch 5 is
>> the modernization effort.)
>
> Well, "t3600: remove useless redirect" has been in 'next' already;
> do you mean that you want me to queue this series on top of that
> topic?
>
I need to reroll this series any way; the reroll will be on top of that.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] builtin/commit.c: convert trivial snprintf calls to xsnprintf
From: Junio C Hamano @ 2016-12-13 19:03 UTC (permalink / raw)
To: Jeff King; +Cc: Elia Pinto, git
In-Reply-To: <20161213135514.z7eituxgxsvybwgz@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> + argv_array_pushf(&env, "GIT_INDEX_FILE=%s", index_file);
>> + if (launch_editor(git_path_commit_editmsg(), NULL, env.argv)) {
>> fprintf(stderr,
>> _("Please supply the message using either -m or -F option.\n"));
>> + argv_array_clear(&env);
>> exit(1);
>> }
>> + argv_array_clear(&env);
>
> I'd generally skip the clear() right before exiting, though I saw
> somebody disagree with me recently on that. I wonder if we should
> decide as a project on whether it is worth doing or not.
I'd say it is a responsibility of the person who turns exit(1) to
return -1 to ensure the code does not leak.
>> @@ -1525,12 +1526,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
>> static int run_rewrite_hook(const unsigned char *oldsha1,
>> const unsigned char *newsha1)
>> {
>> - /* oldsha1 SP newsha1 LF NUL */
>> - static char buf[2*40 + 3];
>> + char *buf;
>> struct child_process proc = CHILD_PROCESS_INIT;
>> const char *argv[3];
>> int code;
>> - size_t n;
>>
>> argv[0] = find_hook("post-rewrite");
>> if (!argv[0])
>> @@ -1546,34 +1545,33 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
>> code = start_command(&proc);
>> if (code)
>> return code;
>> - n = snprintf(buf, sizeof(buf), "%s %s\n",
>> - sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
>> + buf = xstrfmt("%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
>> sigchain_push(SIGPIPE, SIG_IGN);
>> - write_in_full(proc.in, buf, n);
>> + write_in_full(proc.in, buf, strlen(buf));
>> close(proc.in);
>> + free(buf);
>
> Any time you care about the length of the result, I'd generally use an
> actual strbuf instead of xstrfmt. The extra strlen isn't a big deal
> here, but it somehow seems simpler to me. It probably doesn't matter
> much either way, though.
Your justification for this extra allocation was that it is a
heavy-weight operation. While I agree that the runtime cost of
allocation and deallocation does not matter, I would be a bit
worried about extra cognitive burden to programmers. They did not
have to worry about leaking because they are writing a fixed length
string. Now they do, whether they use xstrfmt() or struct strbuf.
When they need to update what they write, they do have to remember
to adjust the size of the "fixed string", and the original is not
free from the "programmers' cognitive cost" point of view, of
course. Probably use of strbuf/xstrfmt is an overall win.
And of course, I think strbuf is more appropriate if you have to do
strlen().
^ permalink raw reply
* Re: git add -p with new file
From: Jeff King @ 2016-12-13 18:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephan Beyer, Ariel, git
In-Reply-To: <xmqq7f734qe0.fsf@gitster.mtv.corp.google.com>
On Tue, Dec 13, 2016 at 10:48:07AM -0800, Junio C Hamano wrote:
> > I think the problem is just that "add -p" does not give the whole story
> > of what you might want to do before making a commit.
>
> The same is shared by "git diff [HEAD]", by the way. It is beyond
> me why people use "add -p", not "git diff [HEAD]", for the final
> sanity check before committing.
>
> Perhaps the latter is not advertised well enough? "add -p" does not
> even page so it is not very useful way to check what is being added
> if you are adding a new file (unless you are doing a toy example to
> add a 7-line file).
I use "add -p" routinely for my final add-and-sanity-check, and it is
certainly not because I don't know about "git diff". I think it's just
nice to break it into bite-size chunks and sort them into "yes, OK" or
"no, not yet" bins. The lack of paging isn't usually a problem, because
this "add -p" is useful precisely when you have a lot of little changes
spread across the code base.
I'd probably also run "git diff" if I wanted to look at something
bigger. And I routinely use "git status", too, to see the full state of
my tree.
To me they are all tools in the toolbox, and I can pick the one that
works best in any given situation, or that I just feel like using that
day.
> >> Hm, "interactive.showUntracked" is a confusing name because "git add -i"
> >> (interactive) already handles untracked files.
> >
> > Sure, that was just meant for illustration. I agree there's probably a
> > better name.
>
> "interactive.*" is not a sensible hierarchy to use, because things
> other than "add" can go interactive.
>
> addPatch.showUntracked?
Hmm, I wonder if interactive.diffFilter was mis-named then. The
description is written in such a way as to cover other possible
interactive commands showing a diff.
It might be possible to do the same here: come up with a general option
that _could_ cover new situations, but right now just applies here. Or
maybe it would be too confusing.
TBH, I wasn't all that concerned with the name yet. Whoever writes the
patch can figure it out, and _then_ we can all bikeshed it. :)
-Peff
^ permalink raw reply
* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Junio C Hamano @ 2016-12-13 18:53 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <CAGZ79kbmtYzFmEKrxHKx-_WY=0NDJM=QZYJziim-eh-w4WzDKw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Mon, Dec 12, 2016 at 11:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> The "checkout --recurse-submodules" series got too large to comfortably send
>>> it out for review, so I had to break it up into smaller series'; this is the
>>> first subseries, but it makes sense on its own.
>>>
>>> This series teaches git-rm to absorb the git directory of a submodule instead
>>> of failing and complaining about the git directory preventing deletion.
>>>
>>> It applies on origin/sb/submodule-embed-gitdir.
>>
>> Thanks. I probably should rename the topic again with s/embed/absorb/;
>
> I mostly renamed it in the hope to settle our dispute what embedding means. ;)
I do not think there is no dispute about what embedding means. A
submodule whose .git is inside its working tree has its repository
embedded.
What we had trouble settling on was what to call the operation to
undo the embedding, unentangling its repository out of the working
tree. I'd still vote for unembed if you want a name to be nominated.
> Note that sb/t3600-cleanup is part of this series now,
> (The first commit of that series is in patch 6/6 of this series, and patch 5 is
> the modernization effort.)
Well, "t3600: remove useless redirect" has been in 'next' already;
do you mean that you want me to queue this series on top of that
topic?
^ permalink raw reply
* Re: git add -p with new file
From: Junio C Hamano @ 2016-12-13 18:48 UTC (permalink / raw)
To: Jeff King; +Cc: Stephan Beyer, Ariel, git
In-Reply-To: <20161213173341.wemlunlixdp6277h@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> I am also not really sure what problem this feature is trying to solve.
>> If the "problem"(?) is that it should act more like "git add" instead of
>> "git add -u", for whatever reason, this may be fine (but the
>> configuration option is a must-have then).
>
> I think the problem is just that "add -p" does not give the whole story
> of what you might want to do before making a commit.
The same is shared by "git diff [HEAD]", by the way. It is beyond
me why people use "add -p", not "git diff [HEAD]", for the final
sanity check before committing.
Perhaps the latter is not advertised well enough? "add -p" does not
even page so it is not very useful way to check what is being added
if you are adding a new file (unless you are doing a toy example to
add a 7-line file).
>> > I'd also probably add interactive.showUntracked to make the whole thing
>> > optional (but I think it would be OK to default it to on).
>> Hm, "interactive.showUntracked" is a confusing name because "git add -i"
>> (interactive) already handles untracked files.
>
> Sure, that was just meant for illustration. I agree there's probably a
> better name.
"interactive.*" is not a sensible hierarchy to use, because things
other than "add" can go interactive.
addPatch.showUntracked?
^ permalink raw reply
* Re: [PATCH 0/2] handling alternates paths with colons
From: Jeff King @ 2016-12-13 18:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <xmqqbmwf4qn4.fsf@gitster.mtv.corp.google.com>
On Tue, Dec 13, 2016 at 10:42:39AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Right, but we also support relative paths via environment variables. I
> > don't think that changes the conclusion though. I'm not convinced
> > relative paths via the environment aren't slightly insane in the first
> > place,
>
> Sorry, a triple negation is above my head. "relative paths in env
> aren't insane" == "using relative paths is OK" and you are not
> convinced that it is the case, so you are saying that you think
> relative paths in env is not something that is sensible?
I think relative paths in env have very flaky semantics which makes them
inadvisable to use in general. That being said, when we broke even those
flaky semantics somebody complained. So I consider a feature we _do_
support, but not one I would recommend to people.
-Peff
^ permalink raw reply
* Re: [PATCH 0/2] handling alternates paths with colons
From: Junio C Hamano @ 2016-12-13 18:42 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161213181755.wrgu6drm45v7xhnl@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Right, but we also support relative paths via environment variables. I
> don't think that changes the conclusion though. I'm not convinced
> relative paths via the environment aren't slightly insane in the first
> place,
Sorry, a triple negation is above my head. "relative paths in env
aren't insane" == "using relative paths is OK" and you are not
convinced that it is the case, so you are saying that you think
relative paths in env is not something that is sensible?
> given the discussion in 37a95862c (alternates: re-allow relative
> paths from environment, 2016-11-07). And they'd probably start with
> "../" as well.
Yeah. In any case, there is a potential regression but the chance
is miniscule---the user has to have been using a relative path that
begins with a double-quote in the environment or in alternates file.
^ permalink raw reply
* Re: [PATCH 0/2] handling alternates paths with colons
From: Jeff King @ 2016-12-13 18:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <xmqqshpr4s41.fsf@gitster.mtv.corp.google.com>
On Tue, Dec 13, 2016 at 10:10:54AM -0800, Junio C Hamano wrote:
> > We do support non-absolute paths, both in alternates files and
> > environment variables. It's a nice feature if you want to have a
> > relocatable family of shared repositories. I'd imagine that most cases
> > start with "../", though.
>
> Yes. I was only talking about the environment variable, as you can
> tell from my mention of "colon" there.
Right, but we also support relative paths via environment variables. I
don't think that changes the conclusion though. I'm not convinced
relative paths via the environment aren't slightly insane in the first
place, given the discussion in 37a95862c (alternates: re-allow relative
paths from environment, 2016-11-07). And they'd probably start with
"../" as well.
-Peff
^ permalink raw reply
* Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines
From: Junio C Hamano @ 2016-12-13 18:15 UTC (permalink / raw)
To: Vasco Almeida
Cc: Johannes Schindelin, git, Jiang Xin,
Ævar Arnfjörð Bjarmason, Jean-Noël AVILA,
Jakub Narębski, David Aguilar
In-Reply-To: <1481627820.2041.21.camel@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
>> We only update comment_line_char from the default "#" when the
>> configured value is a single-byte character and we ignore incorrect
>> values in the configuration file. So I think the patch you sent is
>> correct after all.
>
> I am still not sure what version do we prefer.
>
> Check whether core.commentchar is a single character. If not, use '#'
> as the $comment_line_char.
This, plus special casing "auto".
Picking the first byte is inconsistent with the current practice
(the paragraph you quoted above), I think.
^ permalink raw reply
* Re: [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Jeff King @ 2016-12-13 18:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <xmqqwpf34s5f.fsf@gitster.mtv.corp.google.com>
On Tue, Dec 13, 2016 at 10:10:04AM -0800, Junio C Hamano wrote:
> > - git clone --bare . xxx:yyy.git &&
> > + git clone --bare . xxx${path_sep}yyy.git &&
>
> Don't you want to dq the whole thing to prevent the shell from
> splitting this into two commands at ';'? The other one below is OK.
After expansion, I don't think the shell will do any further processing
except for whitespace splitting. E.g.:
$ debug() { for i in "$@"; do echo "got $i"; done; }
$ sep=';'
$ space=' '
$ debug foo${sep}bar
got foo;bar
$ debug foo${space}bar
got foo
got bar
I don't mind quoting it to make it more obvious, though.
-Peff
^ permalink raw reply
* Re: [PATCH 1/4] doc: add articles (grammar)
From: Junio C Hamano @ 2016-12-13 18:11 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Philip Oakley, git
In-Reply-To: <8737hsj7wp.fsf@gmail.com>
Kristoffer Haugsbakk <kristoffer.haugsbakk@gmail.com> writes:
> Thank you for reviewing this series, Philip.
>
> Philip Oakley writes:
>
>> This looks good to me.
>
> I'll add this header:
>
> Acked-by: Philip Oakley <philipoakley@iee.org>
>
> To the commit message of this patch in the next review round (version 2
> of the patch series).
>
> Let me know if I should add another header, or do something different
> than this.
I was planning to merge all four from you as-is to 'next' today,
though. Should I wait?
^ permalink raw reply
* Re: [PATCH 0/2] handling alternates paths with colons
From: Junio C Hamano @ 2016-12-13 18:10 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161213115018.quulwlheycjtlsub@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Dec 12, 2016 at 02:37:08PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > So here are patches that do that. It kicks in only when the first
>> > character of a path is a double-quote, and then expects the usual
>> > C-style quoting.
>>
>> The quote being per delimited component is what makes this fifth
>> approach a huge win.
>>
>> All sane components on a list-valued environment are still honored
>> and an insane component that has either a colon in the middle or
>> begins with a double-quote gets quoted. As long as nobody used a
>> path that begins with a double-quote as an element in such a
>> list-valued environment (and they cannot be, as using a non-absolute
>> path as an element does not make much sense), this will be safe, and
>> a path with a colon didn't work with Git unaware of the new quoting
>> rule anyway. Nice.
>
> We do support non-absolute paths, both in alternates files and
> environment variables. It's a nice feature if you want to have a
> relocatable family of shared repositories. I'd imagine that most cases
> start with "../", though.
Yes. I was only talking about the environment variable, as you can
tell from my mention of "colon" there.
^ permalink raw reply
* Re: [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Junio C Hamano @ 2016-12-13 18:10 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161213114414.masgfo7lf7e3utym@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The naive conversion is just:
> ...
> -# MINGW does not allow colons in pathnames in the first place
> -test_expect_success !MINGW 'push to repo path with colon' '
> +if test_have_prereq MINGW
> +then
> + path_sep=';'
> +else
> + path_sep=':'
> +fi
> ...
> - git clone --bare . xxx:yyy.git &&
> + git clone --bare . xxx${path_sep}yyy.git &&
Don't you want to dq the whole thing to prevent the shell from
splitting this into two commands at ';'? The other one below is OK.
>
> echo change >>file.bin &&
> git commit -am change &&
> # Note that we have to use the full path here, or it gets confused
> # with the ssh host:path syntax.
> - git push "$PWD/xxx:yyy.git" HEAD
> + git push "$PWD/xxx${path_sep}yyy.git" HEAD
> '
>
> test_done
>
> Does that work?
>
> -Peff
^ permalink raw reply
* Re: [PATCH 1/2] alternates: accept double-quoted paths
From: Junio C Hamano @ 2016-12-13 18:08 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jeff King, Johannes Sixt, Klaus Ethgen, Git Mailing List
In-Reply-To: <CACsJy8B52ZDRTUjGLqub_1wELtugv99xbDnBg1PX1LUTb6nVMQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> At least attr has the same problem and is going the same direction
> [1]. Cool. (I actually thought the patch was in and evidence that this
> kind of backward compatibility breaking was ok, turns out the patch
> has stayed around for years)
>
> [1] http://public-inbox.org/git/%3C20161110203428.30512-18-sbeller@google.com%3E/
Seeing that again, I see this in the proposed log message:
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly
'pat"t"ern', not 'pattern'.
I cannot tell what it is trying to say. The code suggests something
quite different, i.e. a line like this
"pat\"t\"ern" attr
would tell us that a path that matches the pattern
pat"t"ern
is given 'attr'.
The log message may need to be cleaned up. The update by that
commit to the documentation looks alright, thoguh.
^ permalink raw reply
* RE: [PATCH 6/6] rm: add absorb a submodules git dir before deletion
From: David Turner @ 2016-12-13 18:06 UTC (permalink / raw)
To: 'Stefan Beller', gitster@pobox.com
Cc: git@vger.kernel.org, bmwill@google.com
In-Reply-To: <20161213014055.14268-7-sbeller@google.com>
> -----Original Message-----
> From: Stefan Beller [mailto:sbeller@google.com]
> Sent: Monday, December 12, 2016 8:41 PM
> To: gitster@pobox.com
> Cc: git@vger.kernel.org; David Turner; bmwill@google.com; Stefan Beller
> Subject: [PATCH 6/6] rm: add absorb a submodules git dir before deletion
>
> When deleting a submodule we need to keep the actual git directory around,
Nit: comma after submodule.
> - strbuf_reset(&buf);
> - strbuf_addstr(&buf, path);
> - if (!remove_dir_recursively(&buf, 0)) {
> - removed = 1;
> - if
> (!remove_path_from_gitmodules(path))
> - gitmodules_modified = 1;
> - strbuf_release(&buf);
> - continue;
> - } else if (!file_exists(path))
> - /* Submodule was removed by user */
> - if
> (!remove_path_from_gitmodules(path))
> - gitmodules_modified = 1;
> + if (file_exists(path))
> + depopulate_submodule(path);
> + removed = 1;
> + if (!remove_path_from_gitmodules(path))
> + gitmodules_modified = 1;
> + continue;
> /* Fallthrough and let remove_path() fail.
> */
It seems odd to have a continue right before a comment that says "Fallthrough".
^ permalink raw reply
* Re: [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit'
From: Junio C Hamano @ 2016-12-13 18:02 UTC (permalink / raw)
To: Chris Packham; +Cc: git, mah, peff, jacob.keller
In-Reply-To: <20161213084859.13426-1-judge.packham@gmail.com>
Chris Packham <judge.packham@gmail.com> writes:
> +'git merge' --continue
>
> DESCRIPTION
> -----------
> @@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
> discouraged: while possible, it may leave you in a state that is hard to
> back out of in the case of a conflict.
>
> +The fourth syntax ("`git merge --continue`") can only be run after the
> +merge has resulted in conflicts.
OK. I can see that the code refuses if there is no MERGE_HEAD, so
"can only be run" is ensured correctly.
> 'git merge --continue' will take the
> +currently staged changes and complete the merge.
For Git-savvy folks, this may be sufficient to tell that they are
expected to resolve conflicts in the working tree and register the
resolusion by doing "git add" before running "git merge --continue",
but I wonder if that is clear enough for new readers.
The same comment applies to the option description below. I suspect
that it is better to remove the last sentence above, leaving "4th
one can be run only with MERGE_HEAD" here, and enhance the
explanation in the option description (see below).
> OPTIONS
> -------
> @@ -99,6 +103,12 @@ commit or stash your changes before running 'git merge'.
> 'git merge --abort' is equivalent to 'git reset --merge' when
> `MERGE_HEAD` is present.
>
> +--continue::
> + Take the currently staged changes and complete the merge.
> ++
> +'git merge --continue' is equivalent to 'git commit' when
> +`MERGE_HEAD` is present.
> +
These two sentences are even more technical and unfriendly to new
readers, I am afraid. How about giving a hint by referring to an
existing section, like this?
--continue::
After a "git merge" stops due to conflicts you can conclude
the merge by running "git merge --continue" (see "How to
resolve conflicts" section below).
> @@ -277,7 +287,8 @@ After seeing a conflict, you can do two things:
>
> * Resolve the conflicts. Git will mark the conflicts in
> the working tree. Edit the files into shape and
> - 'git add' them to the index. Use 'git commit' to seal the deal.
> + 'git add' them to the index. Use 'git merge --continue' to seal the
> + deal.
Why do we want to make it harder to discover "git commit" here?
I would understand:
... Use 'git commit' to conclude (you can also say 'git
merge --continue').
though. After all, we are merely introducing a synonym for those
who want to type more. There is no plan to deprecate the use of
'git commit', which is a perfectly reasonable way to conclude an
interrupted merge, that has worked well for us for the past 10 years
and still works.
> @@ -65,6 +66,7 @@ static int option_renormalize;
> static int verbosity;
> static int allow_rerere_auto;
> static int abort_current_merge;
> +static int continue_current_merge;
> static int allow_unrelated_histories;
> static int show_progress = -1;
> static int default_to_upstream = 1;
> @@ -223,6 +225,8 @@ static struct option builtin_merge_options[] = {
> OPT__VERBOSITY(&verbosity),
> OPT_BOOL(0, "abort", &abort_current_merge,
> N_("abort the current in-progress merge")),
> + OPT_BOOL(0, "continue", &continue_current_merge,
> + N_("continue the current in-progress merge")),
> OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
> N_("allow merging unrelated histories")),
> OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
> @@ -739,7 +743,7 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)
> if (err_msg)
> error("%s", err_msg);
> fprintf(stderr,
> - _("Not committing merge; use 'git commit' to complete the merge.\n"));
> + _("Not committing merge; use 'git merge --continue' to complete the merge.\n"));
Likewise. I do not see a need to change this one at all.
> @@ -1166,6 +1170,22 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> goto done;
> }
>
> + if (continue_current_merge) {
> + int nargc = 1;
> + const char *nargv[] = {"commit", NULL};
> +
> + if (argc)
> + usage_msg_opt("--continue expects no arguments",
> + builtin_merge_usage, builtin_merge_options);
Peff already commented on "what about other options?", and I think
his "check the number of args before parse-options ran to ensure
that the '--abort' or '--continue' was the only thing" is probably
a workable hack.
The "right" way to fix it would be way too involved to be worth for
just this single change (and also fixing "--abort"). Just thinking
aloud:
* Update parse-options API to:
- extend "struct option" with one field that holds what "command
modes" the option the "struct option" describes is incompatible
with.
- make parse_options() to keep track of the set of command modes
that are compatible with the options seen so far, and complain
if an option that is not compatible with the command mode is
given.
* Use the above facility to update "git merge" so that --abort and
--continue becomes OPT_CMDMODE.
Then, the updated parse_options() would:
- start by making the "incompatible command modes" an empty set.
- while it processes each option on the command line:
- if it is not an OPTION_CMDMODE, add the set of command modes
that are incompatible with the option to the "incompatible
command modes".
- if it is an OPTION_CMDMODE and we already saw another
OPTION_CMDMODE, error out (we already do this).
- after all options are read, check the final command mode and see
if that is in "incompatible command modes".
You can mark almost all options "git merge" takes except a selected
few like "--quiet" as incompatible with "--abort" and "--continue"
and let parse_options() catch incompatible options. Of course you
still need to check argc for non-option here.
^ permalink raw reply
* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Stefan Beller @ 2016-12-13 17:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <xmqqr35c5luq.fsf@gitster.mtv.corp.google.com>
On Mon, Dec 12, 2016 at 11:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> The "checkout --recurse-submodules" series got too large to comfortably send
>> it out for review, so I had to break it up into smaller series'; this is the
>> first subseries, but it makes sense on its own.
>>
>> This series teaches git-rm to absorb the git directory of a submodule instead
>> of failing and complaining about the git directory preventing deletion.
>>
>> It applies on origin/sb/submodule-embed-gitdir.
>
> Thanks. I probably should rename the topic again with s/embed/absorb/;
I mostly renamed it in the hope to settle our dispute what embedding means. ;)
So in case we want to further discuss on the name of the function, we should
do that before doing actual work such as renaming.
Note that sb/t3600-cleanup is part of this series now,
(The first commit of that series is in patch 6/6 of this series, and patch 5 is
the modernization effort.)
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 6/6] rm: add absorb a submodules git dir before deletion
From: Stefan Beller @ 2016-12-13 17:51 UTC (permalink / raw)
To: brian m. carlson, Stefan Beller, Junio C Hamano,
git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <20161213032848.4ps42jinix6fdgdc@genre.crustytoothpaste.net>
On Mon, Dec 12, 2016 at 7:28 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Mon, Dec 12, 2016 at 05:40:55PM -0800, Stefan Beller wrote:
>> When deleting a submodule we need to keep the actual git directory around,
>> such that we do not lose local changes in there and at a later checkout
>> of the submodule we don't need to clone it again.
>>
>> Implement `depopulate_submodule`, that migrates the git directory before
>> deletion of a submodule and afterwards the equivalent of "rm -rf", which
>> is already found in entry.c, so expose that and for clarity add a suffix
>> "_or_dir" to it.
>
> I think you might have meant "_or_die" here.
indeed, will fix in a reroll. Thanks for the review!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox