Git development
 help / color / mirror / Atom feed
* Re: [PATCH v4 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Brandon Williams @ 2016-11-21 18:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <20161118225220.GA76630@google.com>

On 11/18, Brandon Williams wrote:

> Also, in order to use the tree_entry_interesting code it looks like I'll
> either have to pipe through a flag saying 'yes i want to match against
> submodules' like I did for the other pathspec codepath.  Either that or
> add functionality to perform wildmatching against partial matches (ie
> directories and submodules) since currently the tree_entry_interesting
> code path just punts and says 'well say it matches for now and check
> again later' whenever it runs into a directory (I can't really make it
> do that for submodules without a flag of somesort as tests could break).
> Or maybe both?

Looks like my initial assumption was incorrect, I just needed to be
smarter than punting when running into a submodule.  Should be able to
just ensure that the entry matches up to at least the first wildcard
character before punting and all should be good.

-- 
Brandon Williams

^ permalink raw reply

* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 18:15 UTC (permalink / raw)
  To: Johannes Schindelin, Jeff King
  Cc: git, Ralf Thielow, Nguyễn Thái Ngọc Duy,
	Taufiq Hoven
In-Reply-To: <f47dce15719954d6d4d8a550856757366871143e.1479737858.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d8..202ac07 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,6 +432,13 @@ test_expect_success '-c with changed comment char' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_failure '-c with comment char defined in .git/config' '
> +	test_config core.commentchar = &&
> +	printf "= foo\n" >expect &&
> +	printf "foo" | git stripspace -c >actual &&

We'd want "\n" on this printf to match the one before as well, as
this test is not about "does stripspace complete an incomplete
line?", I think.  

I could amend it while queuing, but I need to know if I am missing a
reason why this must be an incomplete line before doing so.

> +	test_cmp expect actual
> +'
> +

Is this a recent regression?  When applied on top of 'maint' or
older, it seems to pass just fine.

    ... Goes and looks ...

Interesting.  Peff's b9605bc4f2 ("config: only read .git/config from
configured repos", 2016-09-12) is where this starts failing, which
is understandable given the code change to builtin/stripspace.c in
[2/3].  

The analysis of the log message in [2/3] is wrong and needs
updating, though.  In the old world order it worked by accident to
call git_config() without calling setup_git_directory(); after
b9605bc4f2, that no longer is valid and is exposed as a bug.  

Your [2/3] is a good fix for that change.

In any case, well spotted.

>  test_expect_success 'avoid SP-HT sequence in commented line' '
>  	printf "#\tone\n#\n# two\n" >expect &&
>  	printf "\tone\n\ntwo\n" | git stripspace -c >actual &&
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index e38e296..e080399 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -976,6 +976,18 @@ test_expect_success 'rebase -i respects core.commentchar' '
>  	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
>  '
>  
> +test_expect_failure 'rebase -i respects core.commentchar=auto' '
> +	test_config core.commentchar auto &&
> +	write_script copy-edit-script.sh <<-\EOF &&
> +	cp "$1" edit-script
> +	EOF
> +	test_set_editor "$(pwd)/copy-edit-script.sh" &&
> +	test_when_finished "git rebase --abort || :" &&
> +	git rebase -i HEAD^ &&
> +	grep "^#" edit-script &&

This was added for debugging that was forgotten?

> +	test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)"

This says "There shouldn't be any line left once we remove
'#'-commented lines, empty lines and pick insns.".  OK.

The correction in [3/3] seems good.

> +'
> +
>  test_expect_success 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
>  	test_when_finished "git branch -D torebase" &&
>  	git checkout -b torebase branch1 &&

^ permalink raw reply

* Re: Feature request: Improve diff algorithm
From: Stefan Beller @ 2016-11-21 18:17 UTC (permalink / raw)
  To: Jacob Keller; +Cc: KES, git
In-Reply-To: <CA+P7+xrKfqOb-3FhKxXdnF6g9PktNWNzY+ZnHJS=yuVo1YdXzg@mail.gmail.com>

On Mon, Nov 21, 2016 at 8:56 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Mon, Nov 21, 2016 at 12:11 AM, KES <kes-kes@yandex.ru> wrote:
>> Hi.
>>
>
> Hi,
>
>> I have some question about how diff works then give proposal:
>>
>> it will be very useful for each "symbol" store additional meta info as source line length. So in this case when git counter two equal sequence of commands it will do further comparison: Adds 23 chars deletes none VS adds 75 chars deletes 46
>>
>> Actually I got this:
>>
>> @@ -129,8 +132,9 @@ sub _preprocess_message {
>>  sub _process_message {
>>      my ($self, $message) = @_;
>>
>> -    my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>> +    my $time =  [ gettimeofday ];
>>
>> +    my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>>      return $self->send_error(ERROR_REQUEST_INVALID)
>>          unless defined($method);
>>
>> Instead of expected:
>> @@ -129,6 +132,8 @@ sub _preprocess_message {
>>  sub _process_message {
>>      my ($self, $message) = @_;
>>
>> +    my $time =  [ gettimeofday ];
>> +
>>      my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>> -
>>      return $self->send_error(ERROR_REQUEST_INVALID)
>>
>
> Have you tried the various options for git to search for smaller
> diffs? Or using the other diff algorithms such as histogram instead of
> patience?
>

The newest version of Git comes with a flag to move around the diff
better, based on the work at https://github.com/mhagger/diff-slider-tools

^ permalink raw reply

* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 18:24 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jeff King, git, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqbmx8k8c0.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

>> +test_expect_failure '-c with comment char defined in .git/config' '
>> +	test_config core.commentchar = &&
>> +	printf "= foo\n" >expect &&
>> +	printf "foo" | git stripspace -c >actual &&
>
> We'd want "\n" on this printf to match the one before as well, as
> this test is not about "does stripspace complete an incomplete
> line?", I think.  
>
> I could amend it while queuing, but I need to know if I am missing a
> reason why this must be an incomplete line before doing so.
>
>> +	test_cmp expect actual
>> +'
>> +
>
> Is this a recent regression?  When applied on top of 'maint' or
> older, it seems to pass just fine.

I think we can force failure by running this test somewhere other
than the top level of the working tree.  A set of proposed amends
incoming ...

^ permalink raw reply

* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Johannes Sixt @ 2016-11-21 18:26 UTC (permalink / raw)
  To: Johannes Schindelin, git
  Cc: Junio C Hamano, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <9ef529a4fbb60990a91d7bbfdd49c6d20d49e442.1479737858.git.johannes.schindelin@gmx.de>

Am 21.11.2016 um 15:18 schrieb Johannes Schindelin:
> -comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> -: ${comment_char:=#}
> +comment_char=$(git config --get core.commentchar 2>/dev/null)
> +case "$comment_char" in
> +''|auto)
> +	comment_char=#
> +	;;
> +?)
> +	;;
> +*)
> +	comment_char=$(comment_char | cut -c1)

comment_char is a command? Did you mean

	comment_char=$(echo "$comment_char" | cut -c1)

It could be written without forking a process:

	comment_char=${comment_char%${comment_char#?}}

(aka "remove from the end what remains after removing the first character")

-- Hannes


^ permalink raw reply

* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Junio C Hamano @ 2016-11-21 18:40 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Johannes Schindelin, git, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <975c0002-8afe-efa6-d967-72f2a0d21169@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> comment_char is a command? Did you mean
>
> 	comment_char=$(echo "$comment_char" | cut -c1)

;-)

> It could be written without forking a process:
>
> 	comment_char=${comment_char%${comment_char#?}}
>
> (aka "remove from the end what remains after removing the first character")

Hopefully nobody would include any glob metacharacters in there,
e.g. "core.commentchar='=*'", which would break that?



^ permalink raw reply

* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Jeff King @ 2016-11-21 18:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, git, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqbmx8k8c0.fsf@gitster.mtv.corp.google.com>

On Mon, Nov 21, 2016 at 10:15:43AM -0800, Junio C Hamano wrote:

> > +	test_cmp expect actual
> > +'
> > +
> 
> Is this a recent regression?  When applied on top of 'maint' or
> older, it seems to pass just fine.
> 
>     ... Goes and looks ...
> 
> Interesting.  Peff's b9605bc4f2 ("config: only read .git/config from
> configured repos", 2016-09-12) is where this starts failing, which
> is understandable given the code change to builtin/stripspace.c in
> [2/3]. 
> 
> The analysis of the log message in [2/3] is wrong and needs
> updating, though.  In the old world order it worked by accident to
> call git_config() without calling setup_git_directory(); after
> b9605bc4f2, that no longer is valid and is exposed as a bug.  

Yeah, I noticed that while reading the patch. My b9605bc4f2 did regress
this case, but called out the fact that "cd subdir && git stripspace"
would never have worked. So one step back, 2 steps forward, and Dscho's
patch is the right step forward.

-Peff

^ permalink raw reply

* Re: Feature request: Improve diff algorithm
From: Jacob Keller @ 2016-11-21 18:55 UTC (permalink / raw)
  To: Stefan Beller; +Cc: KES, git
In-Reply-To: <CAGZ79kZbGPneUXVEqJYhQAn+dfYve7qCjhO7QFaV1JBs3HD1aA@mail.gmail.com>

On Mon, Nov 21, 2016 at 10:17 AM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Nov 21, 2016 at 8:56 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Mon, Nov 21, 2016 at 12:11 AM, KES <kes-kes@yandex.ru> wrote:
>>> Hi.
>>>
>>
>> Hi,
>>
>>> I have some question about how diff works then give proposal:
>>>
>>> it will be very useful for each "symbol" store additional meta info as source line length. So in this case when git counter two equal sequence of commands it will do further comparison: Adds 23 chars deletes none VS adds 75 chars deletes 46
>>>
>>> Actually I got this:
>>>
>>> @@ -129,8 +132,9 @@ sub _preprocess_message {
>>>  sub _process_message {
>>>      my ($self, $message) = @_;
>>>
>>> -    my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>>> +    my $time =  [ gettimeofday ];
>>>
>>> +    my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>>>      return $self->send_error(ERROR_REQUEST_INVALID)
>>>          unless defined($method);
>>>
>>> Instead of expected:
>>> @@ -129,6 +132,8 @@ sub _preprocess_message {
>>>  sub _process_message {
>>>      my ($self, $message) = @_;
>>>
>>> +    my $time =  [ gettimeofday ];
>>> +
>>>      my $method = ref($message) eq 'HASH' ? $message->{method} : undef;
>>> -
>>>      return $self->send_error(ERROR_REQUEST_INVALID)
>>>
>>
>> Have you tried the various options for git to search for smaller
>> diffs? Or using the other diff algorithms such as histogram instead of
>> patience?
>>
>
> The newest version of Git comes with a flag to move around the diff
> better, based on the work at https://github.com/mhagger/diff-slider-tools

Unfortunately in this case, I'm not convinced that it will improve the
diff. It's worth a try as well though.

Thanks,
Jake

^ permalink raw reply

* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Johannes Sixt @ 2016-11-21 18:58 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, git, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqq37ikk762.fsf@gitster.mtv.corp.google.com>

Am 21.11.2016 um 19:40 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>> It could be written without forking a process:
>>
>> 	comment_char=${comment_char%${comment_char#?}}
>>
>> (aka "remove from the end what remains after removing the first character")
>
> Hopefully nobody would include any glob metacharacters in there,
> e.g. "core.commentchar='=*'", which would break that?

Heh. I tested a few variations, but not this one. Make it

  	comment_char=${comment_char%"${comment_char#?}"}

;)

-- Hannes


^ permalink raw reply

* [PATCH 1/3] rebase -i: highlight problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 19:05 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <xmqq7f7wk7x9.fsf@gitster.mtv.corp.google.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The interactive rebase does not currently play well with
core.commentchar. Let's add some tests to highlight those problems
that will be fixed in the remainder of the series.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0030-stripspace.sh         |  9 +++++++++
 t/t3404-rebase-interactive.sh | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index 29e91d861c..c1f6411eb2 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -432,6 +432,15 @@ test_expect_success '-c with changed comment char' '
 	test_cmp expect actual
 '
 
+test_expect_failure '-c with comment char defined in .git/config' '
+	test_config core.commentchar = &&
+	printf "= foo\n" >expect &&
+	printf "foo" | (
+		mkdir sub && cd sub && git stripspace -c
+	) >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'avoid SP-HT sequence in commented line' '
 	printf "#\tone\n#\n# two\n" >expect &&
 	printf "\tone\n\ntwo\n" | git stripspace -c >actual &&
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d6d65a3a94..d941f0a69f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -983,6 +983,17 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
+test_expect_failure 'rebase -i respects core.commentchar=auto' '
+	test_config core.commentchar auto &&
+	write_script copy-edit-script.sh <<-\EOF &&
+	cp "$1" edit-script
+	EOF
+	test_set_editor "$(pwd)/copy-edit-script.sh" &&
+	test_when_finished "git rebase --abort || :" &&
+	git rebase -i HEAD^ &&
+	test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)"
+'
+
 test_expect_success 'rebase -i, with <onto> and <upstream> specified as :/quuxery' '
 	test_when_finished "git branch -D torebase" &&
 	git checkout -b torebase branch1 &&
-- 
2.11.0-rc2-154-g95ba452916


^ permalink raw reply related

* [PATCH 2/3] stripspace: respect repository config
From: Junio C Hamano @ 2016-11-21 19:05 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <20161121190514.18574-1-gitster@pobox.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The way "git stripspace" reads the configuration was not quite
correct, in that it forgot to probe for a possibly existing
repository (note: stripspace is designed to be usable outside the
repository as well) before doing so.  Due to this, .git/config was
read only when the command was run from the top-level of the working
tree.  

A recent change b9605bc4f2 ("config: only read .git/config from
configured repos", 2016-09-12) stopped reading the repository-local
configuration file ".git/config" unless the repository discovery
process is done, and ".git/config" is no longer read even when run
from the top-level, which exposed the bug even more.

When rebasing interactively with a commentChar defined in the
current repository's config, the help text at the bottom of the edit
script potentially used an incorrect comment character. This was not
only funny-looking, but also resulted in tons of warnings like this
one:

	Warning: the command isn't recognized in the following line
	 - #

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/stripspace.c  | 4 +++-
 t/t0030-stripspace.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 15e716ef43..1e62a008cb 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
 	if (argc)
 		usage_with_options(stripspace_usage, options);
 
-	if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
+	if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
+		setup_git_directory_gently(NULL);
 		git_config(git_default_config, NULL);
+	}
 
 	if (strbuf_read(&buf, 0, 1024) < 0)
 		die_errno("could not read the input");
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index c1f6411eb2..bbf3e39e3d 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -432,7 +432,7 @@ test_expect_success '-c with changed comment char' '
 	test_cmp expect actual
 '
 
-test_expect_failure '-c with comment char defined in .git/config' '
+test_expect_success '-c with comment char defined in .git/config' '
 	test_config core.commentchar = &&
 	printf "= foo\n" >expect &&
 	printf "foo" | (
-- 
2.11.0-rc2-154-g95ba452916


^ permalink raw reply related

* [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Junio C Hamano @ 2016-11-21 19:05 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <20161121190514.18574-1-gitster@pobox.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When 84c9dc2 (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) extended the core.commentChar functionality to
allow for the value 'auto', it forgot that rebase -i was already taught to
handle core.commentChar, and in turn forgot to let rebase -i handle that
new value gracefully.

Reported by Taufiq Hoven.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-rebase--interactive.sh    | 13 +++++++++++--
 t/t3404-rebase-interactive.sh |  2 +-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 655ebaa471..c167bc36b3 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -93,8 +93,17 @@ eval '
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 
-comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
-: ${comment_char:=#}
+comment_char=$(git config --get core.commentchar 2>/dev/null)
+case "$comment_char" in
+'' | auto)
+	comment_char="#"
+	;;
+?)
+	;;
+*)
+	comment_char=$(echo "$comment_char" | cut -c1)
+	;;
+esac
 
 warn () {
 	printf '%s\n' "$*" >&2
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d941f0a69f..5d0a7dca9d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -983,7 +983,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
-test_expect_failure 'rebase -i respects core.commentchar=auto' '
+test_expect_success 'rebase -i respects core.commentchar=auto' '
 	test_config core.commentchar auto &&
 	write_script copy-edit-script.sh <<-\EOF &&
 	cp "$1" edit-script
-- 
2.11.0-rc2-154-g95ba452916


^ permalink raw reply related

* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Junio C Hamano @ 2016-11-21 19:07 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Johannes Schindelin, git, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <8b783f3d-8fda-b5cb-239f-895b10a22a73@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> Am 21.11.2016 um 19:40 schrieb Junio C Hamano:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>> It could be written without forking a process:
>>>
>>> 	comment_char=${comment_char%${comment_char#?}}
>>>
>>> (aka "remove from the end what remains after removing the first character")
>>
>> Hopefully nobody would include any glob metacharacters in there,
>> e.g. "core.commentchar='=*'", which would break that?
>
> Heh. I tested a few variations, but not this one. Make it
>
>  	comment_char=${comment_char%"${comment_char#?}"}

I tried a few implementations of shells, and decided that this is
not worth the portability hassle.

Setting comment-char to multi-letter sequence is not supported at
all, and this code is protecting itself from that nonsense---it does
not need to be fast, and Dscho already gives a fast path for a
single letter case in his patch.



^ permalink raw reply

* Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 19:12 UTC (permalink / raw)
  To: Jeff King
  Cc: Johannes Schindelin, git, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <20161121184931.36nuhg2h7u6mjsld@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Nov 21, 2016 at 10:15:43AM -0800, Junio C Hamano wrote:
>
>> > +	test_cmp expect actual
>> > +'
>> > +
>> 
>> Is this a recent regression?  When applied on top of 'maint' or
>> older, it seems to pass just fine.
>> 
>>     ... Goes and looks ...
>> 
>> Interesting.  Peff's b9605bc4f2 ("config: only read .git/config from
>> configured repos", 2016-09-12) is where this starts failing, which
>> is understandable given the code change to builtin/stripspace.c in
>> [2/3]. 
>> 
>> The analysis of the log message in [2/3] is wrong and needs
>> updating, though.  In the old world order it worked by accident to
>> call git_config() without calling setup_git_directory(); after
>> b9605bc4f2, that no longer is valid and is exposed as a bug.  
>
> Yeah, I noticed that while reading the patch. My b9605bc4f2 did regress
> this case, but called out the fact that "cd subdir && git stripspace"
> would never have worked. So one step back, 2 steps forward, and Dscho's
> patch is the right step forward.

Yes, absolutely.

I sent out a set of proposed amends, and the one for this step 1/3
runs the command inside a subdirectory to force it not to find the
.git/config file relative to its pwd, which can reveal the existing
breakage without help by b9605bc4f2 ;-) hence can be forked for
older maintenance tracks.

^ permalink raw reply

* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Johannes Sixt @ 2016-11-21 19:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, git, Ralf Thielow,
	Nguyễn Thái Ngọc Duy, Taufiq Hoven
In-Reply-To: <xmqqy40circz.fsf@gitster.mtv.corp.google.com>

Am 21.11.2016 um 20:07 schrieb Junio C Hamano:
> Setting comment-char to multi-letter sequence is not supported at
> all, and this code is protecting itself from that nonsense---it does
> not need to be fast, and Dscho already gives a fast path for a
> single letter case in his patch.

Fair enough, no problem.

-- Hannes


^ permalink raw reply

* Re: [PATCH 1/3] rebase -i: highlight problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 20:25 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <20161121190514.18574-1-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The interactive rebase does not currently play well with
> core.commentchar. Let's add some tests to highlight those problems
> that will be fixed in the remainder of the series.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---

Sorry, I should have commented here after --- line what changes were
proposed by this set of amends.

>  t/t0030-stripspace.sh         |  9 +++++++++
>  t/t3404-rebase-interactive.sh | 11 +++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d861c..c1f6411eb2 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,6 +432,15 @@ test_expect_success '-c with changed comment char' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_failure '-c with comment char defined in .git/config' '
> +	test_config core.commentchar = &&
> +	printf "= foo\n" >expect &&
> +	printf "foo" | (
> +		mkdir sub && cd sub && git stripspace -c
> +	) >actual &&
> +	test_cmp expect actual
> +'

I noticed that the lack of "\n" at the end was merely copied from the
previous existing test (i.e. "-c with with changed comment char"),
which was already wrong the same way, so I kept that part as-is.

Running "stripspace" in a subdirectory is to avoid the ".git/config
was read without repository discovery as long as the command runs at
the top-level of the working tree" accident so that this highlights
the breakage with or without Peff's b9605bc4f2 ("config: only read
.git/config from configured repos", 2016-09-12).


> +
>  test_expect_success 'avoid SP-HT sequence in commented line' '
>  	printf "#\tone\n#\n# two\n" >expect &&
>  	printf "\tone\n\ntwo\n" | git stripspace -c >actual &&
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index d6d65a3a94..d941f0a69f 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -983,6 +983,17 @@ test_expect_success 'rebase -i respects core.commentchar' '
>  	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
>  '
>  
> +test_expect_failure 'rebase -i respects core.commentchar=auto' '
> +	test_config core.commentchar auto &&
> +	write_script copy-edit-script.sh <<-\EOF &&
> +	cp "$1" edit-script
> +	EOF
> +	test_set_editor "$(pwd)/copy-edit-script.sh" &&
> +	test_when_finished "git rebase --abort || :" &&
> +	git rebase -i HEAD^ &&
> +	test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)"
> +'
> +

Removed from here is a leftover debugging bit (grep "^#"
edit-script).


^ permalink raw reply

* Re: [PATCH 2/3] stripspace: respect repository config
From: Junio C Hamano @ 2016-11-21 20:28 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin
In-Reply-To: <20161121190514.18574-2-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The way "git stripspace" reads the configuration was not quite
> correct, in that it forgot to probe for a possibly existing
> repository (note: stripspace is designed to be usable outside the
> repository as well) before doing so.  Due to this, .git/config was
> read only when the command was run from the top-level of the working
> tree.  
>
> A recent change b9605bc4f2 ("config: only read .git/config from
> configured repos", 2016-09-12) stopped reading the repository-local
> configuration file ".git/config" unless the repository discovery
> process is done, and ".git/config" is no longer read even when run
> from the top-level, which exposed the bug even more.

The above two paragraphs are rewritten from the original to explain
how this seemed to work (by accident) and its breakage surfaced in
"rebase -i" after b9605bc4f2 ("config: only read .git/config from
configured repos", 2016-09-12) better.  The use of stripspace in
"rebase-i" was done after cd_to_toplevel and it happened to work
before that commit.

Otherwise there is no change from the original.

> When rebasing interactively with a commentChar defined in the
> current repository's config, the help text at the bottom of the edit
> script potentially used an incorrect comment character. This was not
> only funny-looking, but also resulted in tons of warnings like this
> one:
>
> 	Warning: the command isn't recognized in the following line
> 	 - #
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/stripspace.c  | 4 +++-
>  t/t0030-stripspace.sh | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/stripspace.c b/builtin/stripspace.c
> index 15e716ef43..1e62a008cb 100644
> --- a/builtin/stripspace.c
> +++ b/builtin/stripspace.c
> @@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
>  	if (argc)
>  		usage_with_options(stripspace_usage, options);
>  
> -	if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
> +	if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
> +		setup_git_directory_gently(NULL);
>  		git_config(git_default_config, NULL);
> +	}
>  
>  	if (strbuf_read(&buf, 0, 1024) < 0)
>  		die_errno("could not read the input");
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index c1f6411eb2..bbf3e39e3d 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,7 +432,7 @@ test_expect_success '-c with changed comment char' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_failure '-c with comment char defined in .git/config' '
> +test_expect_success '-c with comment char defined in .git/config' '
>  	test_config core.commentchar = &&
>  	printf "= foo\n" >expect &&
>  	printf "foo" | (

^ permalink raw reply

* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Junio C Hamano @ 2016-11-21 20:29 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Sixt
In-Reply-To: <20161121190514.18574-3-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> writes:

> @@ -93,8 +93,17 @@ eval '
>  GIT_CHERRY_PICK_HELP="$resolvemsg"
>  export GIT_CHERRY_PICK_HELP
>  
> -comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> -: ${comment_char:=#}
> +comment_char=$(git config --get core.commentchar 2>/dev/null)
> +case "$comment_char" in
> +'' | auto)
> +	comment_char="#"
> +	;;
> +?)
> +	;;
> +*)
> +	comment_char=$(echo "$comment_char" | cut -c1)
> +	;;
> +esac

Amended in is a fix for a typo the other Johannes noticed.

Thanks.

> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index d941f0a69f..5d0a7dca9d 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -983,7 +983,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
>  	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
>  '
>  
> -test_expect_failure 'rebase -i respects core.commentchar=auto' '
> +test_expect_success 'rebase -i respects core.commentchar=auto' '
>  	test_config core.commentchar auto &&
>  	write_script copy-edit-script.sh <<-\EOF &&
>  	cp "$1" edit-script

^ permalink raw reply

* [PATCH 1/3] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
  To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>

This addresses a similar concern as in f8eaa0ba98b (submodule--helper,
module_clone: always operate on absolute paths, 2016-03-31)

When computing the relative path from one to another location, we
need to provide both locations as absolute paths to make sure the
computation of the relative path is correct.

While at it, change `real_work_tree` to be non const as we own
the memory.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/submodule.c b/submodule.c
index 6f7d883de9..66c5ce5a24 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
 {
 	struct strbuf file_name = STRBUF_INIT;
 	struct strbuf rel_path = STRBUF_INIT;
-	const char *real_work_tree = xstrdup(real_path(work_tree));
+	char *real_git_dir = xstrdup(real_path(git_dir));
+	char *real_work_tree = xstrdup(real_path(work_tree));
 
 	/* Update gitfile */
 	strbuf_addf(&file_name, "%s/.git", work_tree);
 	write_file(file_name.buf, "gitdir: %s",
-		   relative_path(git_dir, real_work_tree, &rel_path));
+		   relative_path(real_git_dir, real_work_tree, &rel_path));
 
 	/* Update core.worktree setting */
 	strbuf_reset(&file_name);
-	strbuf_addf(&file_name, "%s/config", git_dir);
+	strbuf_addf(&file_name, "%s/config", real_git_dir);
 	git_config_set_in_file(file_name.buf, "core.worktree",
-			       relative_path(real_work_tree, git_dir,
+			       relative_path(real_work_tree, real_git_dir,
 					     &rel_path));
 
 	strbuf_release(&file_name);
 	strbuf_release(&rel_path);
-	free((void *)real_work_tree);
+	free(real_work_tree);
+	free(real_git_dir);
 }
 
 int parallel_submodules(void)
-- 
2.11.0.rc2.18.g0126045.dirty


^ permalink raw reply related

* [PATCH 2/3] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
  To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>

Specifically when setting up submodule tests, it comes in handy if
we can create commits in repositories that are not at the root of
the tested trash dir. Add "-C <dir>" similar to gits -C parameter
that will perform the operation in the given directory.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/test-lib-functions.sh | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..579e812506 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -157,16 +157,21 @@ debug () {
 	 GIT_TEST_GDB=1 "$@"
 }
 
-# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
+# Call test_commit with the arguments
+# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
 #
 # This will commit a file with the given contents and the given commit
 # message, and tag the resulting commit with the given tag name.
 #
 # <file>, <contents>, and <tag> all default to <message>.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
 
 test_commit () {
 	notick= &&
 	signoff= &&
+	indir= &&
 	while test $# != 0
 	do
 		case "$1" in
@@ -176,21 +181,26 @@ test_commit () {
 		--signoff)
 			signoff="$1"
 			;;
+		-C)
+			indir="$2"
+			shift
+			;;
 		*)
 			break
 			;;
 		esac
 		shift
 	done &&
+	indir=${indir:+"$indir"/} &&
 	file=${2:-"$1.t"} &&
-	echo "${3-$1}" > "$file" &&
-	git add "$file" &&
+	echo "${3-$1}" > "$indir$file" &&
+	git ${indir:+ -C "$indir"} add "$file" &&
 	if test -z "$notick"
 	then
 		test_tick
 	fi &&
-	git commit $signoff -m "$1" &&
-	git tag "${4:-$1}"
+	git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+	git ${indir:+ -C "$indir"} tag "${4:-$1}"
 }
 
 # Call test_merge with the arguments "<message> <commit>", where <commit>
-- 
2.11.0.rc2.18.g0126045.dirty


^ permalink raw reply related

* [PATCH 0/3] Introduce `submodule interngitdirs`
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
  To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller

The discussion of the submodule checkout series revealed to me that a command
is needed to move the git directory from the submodules working tree to be
embedded into the superprojects git directory. 

So I wrote the code to intern the submodules git dir into the superproject,
but whilst writing the code I realized this could be valueable for our use
in testing too. So I exposed it via the submodule--helper. But as the
submodule helper ought to be just an internal API, we could also
offer it via the proper submodule command.

The command as it is has little value to the end user for now, but
breaking it out of the submodule checkout series hopefully makes review easier.

Thanks,
Stefan

Stefan Beller (3):
  submodule: use absolute path for computing relative path connecting
  test-lib-functions.sh: teach test_commit -C <dir>
  submodule--helper: add intern-git-dir function

 Documentation/git-submodule.txt    | 15 ++++++++++-
 builtin/submodule--helper.c        | 33 ++++++++++++++++++++++-
 git-submodule.sh                   |  7 ++++-
 submodule.c                        | 55 ++++++++++++++++++++++++++++++++++----
 submodule.h                        |  1 +
 t/t7412-submodule-interngitdirs.sh | 41 ++++++++++++++++++++++++++++
 t/test-lib-functions.sh            | 20 ++++++++++----
 7 files changed, 159 insertions(+), 13 deletions(-)
 create mode 100755 t/t7412-submodule-interngitdirs.sh

-- 
2.11.0.rc2.18.g0126045.dirty


^ permalink raw reply

* [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
  To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>

When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.

Add functionality to migrate the git directory to be embedded
into the superprojects git directory.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/git-submodule.txt    | 15 ++++++++++++-
 builtin/submodule--helper.c        | 33 ++++++++++++++++++++++++++++-
 git-submodule.sh                   |  7 ++++++-
 submodule.c                        | 43 ++++++++++++++++++++++++++++++++++++++
 submodule.h                        |  1 +
 t/t7412-submodule-interngitdirs.sh | 41 ++++++++++++++++++++++++++++++++++++
 6 files changed, 137 insertions(+), 3 deletions(-)
 create mode 100755 t/t7412-submodule-interngitdirs.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..80d55350eb 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,7 +22,7 @@ SYNOPSIS
 	      [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
-
+'git submodule' [--quiet] interngitdirs [--] [<path>...]
 
 DESCRIPTION
 -----------
@@ -245,6 +245,19 @@ sync::
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and sync any nested submodules within.
 
+interngitdirs::
+	Move the git directory of submodules into its superprojects
+	`$GIT_DIR/modules` path and then connect the git directory and
+	its working directory by setting the `core.worktree` and adding
+	a .git file pointing to the git directory interned into the
+	superproject.
++
+	A repository that was cloned independently and later added
+	as a submodule or old setups have the submodules git directory
+	inside the submodule instead of the
++
+	This command is recursive by default.
+
 OPTIONS
 -------
 -q::
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5f9f..256f8e9439 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,36 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
 	return 0;
 }
 
+static int intern_git_dir(int argc, const char **argv, const char *prefix)
+{
+	int i;
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+
+	struct option intern_gitdir_options[] = {
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule--helper intern-git-dir [<path>...]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, intern_gitdir_options,
+			     git_submodule_helper_usage, 0);
+
+	gitmodules_config();
+	git_config(submodule_config, NULL);
+
+	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		return 1;
+
+	for (i = 0; i < list.nr; i++)
+		migrate_submodule_gitdir(list.entries[i]->name);
+
+	return 0;
+}
+
 struct cmd_struct {
 	const char *cmd;
 	int (*fn)(int, const char **, const char *);
@@ -1090,7 +1120,8 @@ static struct cmd_struct commands[] = {
 	{"resolve-relative-url", resolve_relative_url},
 	{"resolve-relative-url-test", resolve_relative_url_test},
 	{"init", module_init},
-	{"remote-branch", resolve_remote_submodule_branch}
+	{"remote-branch", resolve_remote_submodule_branch},
+	{"intern-git-dir", intern_git_dir}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..747e934df2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
 	done
 }
 
+cmd_interngitdirs()
+{
+	git submodule--helper intern-git-dir "$@"
+}
+
 # This loop parses the command line arguments to find the
 # subcommand name to dispatch.  Parsing of the subcommand specific
 # options are primarily done by the subcommand implementations.
@@ -1140,7 +1145,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | deinit | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync | interngitdirs)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 66c5ce5a24..99befdba85 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1263,3 +1263,46 @@ void prepare_submodule_repo_env(struct argv_array *out)
 	}
 	argv_array_push(out, "GIT_DIR=.git");
 }
+
+/*
+ * Migrate the given submodule (and all its submodules recursively) from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void migrate_submodule_gitdir(const char *path)
+{
+	char *old_git_dir;
+	const char *new_git_dir;
+	const struct submodule *sub;
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.dir = path;
+	argv_array_pushl(&cp.args, "submodule", "foreach", "--recursive",
+			"git", "submodule--helper" "intern-git-dir", NULL);
+
+	if (run_command(&cp))
+		die(_("Could not migrate git directory in submodule '%s'"),
+		    path);
+
+	old_git_dir = xstrfmt("%s/.git", path);
+	if (read_gitfile(old_git_dir))
+		/* If it is an actual gitfile, it doesn't need migration. */
+		goto out;
+
+	sub = submodule_from_path(null_sha1, path);
+	if (!sub)
+		die(_("Could not lookup name for submodule '%s'"),
+		      path);
+	new_git_dir = git_common_path("modules/%s", sub->name);
+	mkdir_in_gitdir(".git/modules");
+
+	if (rename(old_git_dir, new_git_dir) < 0)
+		die_errno(_("Could not migrate git directory from '%s' to '%s'"),
+			old_git_dir, new_git_dir);
+
+	connect_work_tree_and_git_dir(path, new_git_dir);
+out:
+	free(old_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index d9e197a948..859026ecfa 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,4 +75,5 @@ int parallel_submodules(void);
  */
 void prepare_submodule_repo_env(struct argv_array *out);
 
+extern void migrate_submodule_gitdir(const char *path);
 #endif
diff --git a/t/t7412-submodule-interngitdirs.sh b/t/t7412-submodule-interngitdirs.sh
new file mode 100755
index 0000000000..8938a4c8b7
--- /dev/null
+++ b/t/t7412-submodule-interngitdirs.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Test submodule interngitdirs
+
+This test verifies that `git submodue interngitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+	git init sub1 &&
+	test_commit -C sub1 first &&
+	git submodule add ./sub1 &&
+	test_tick &&
+	git commit -m superproject
+'
+
+test_expect_success 'intern the git dir' '
+	git submodule interngitdirs &&
+	test -f sub1/.git &&
+	test -d .git/modules/sub1 &&
+	# check that we did not break the repository:
+	git status
+'
+
+test_expect_success 'setup a gitlink with missing .gitmodules entry' '
+	git init sub2 &&
+	test_commit -C sub2 first &&
+	git add sub2 &&
+	git commit -m superproject
+'
+
+test_expect_success 'intern the git dir fails for incomplete submodules' '
+	test_must_fail git submodule interngitdirs &&
+	# check that we did not break the repository:
+	git status
+'
+
+test_done
+
-- 
2.11.0.rc2.18.g0126045.dirty


^ permalink raw reply related

* [PATCH] doc: mention user-configured trailers
From: Jonathan Tan @ 2016-11-21 20:47 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, gitster, jacob.e.keller
In-Reply-To: <CA+P7+xrQEBYQQhqJQQCpLrs+4WOJOvH1X27w5Ou=2VPT=FegGQ@mail.gmail.com>

In commit 1462450 ("trailer: allow non-trailers in trailer block",
2016-10-21), functionality was added (and tested [1]) to allow
non-trailer lines in trailer blocks, as long as those blocks contain at
least one Git-generated or user-configured trailer, and consists of at
least 25% trailers. The documentation was updated to mention this new
functionality, but did not mention "user-configured trailer".

Further update the documentation to also mention "user-configured
trailer".

[1] "with non-trailer lines mixed with a configured trailer" in
t/t7513-interpret-trailers.sh

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---

Yes, mentioning a trailer in a Git config will cause interpret-trailers
to treat it similarly to a Git-generated trailer (in that its presence
causes a block partially consisting of trailers to be considered a
trailer block). See the commit message above for a test case that
verifies that.

I took a look at the documentation, and it wasn't completely documented,
so here is a patch to correct that.

 Documentation/git-interpret-trailers.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index e99bda6..09074c7 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -49,7 +49,8 @@ will be added before the new trailer.
 
 Existing trailers are extracted from the input message by looking for
 a group of one or more lines that (i) are all trailers, or (ii) contains at
-least one Git-generated trailer and consists of at least 25% trailers.
+least one Git-generated or user-configured trailer and consists of at
+least 25% trailers.
 The group must be preceded by one or more empty (or whitespace-only) lines.
 The group must either be at the end of the message or be the last
 non-whitespace lines before a line that starts with '---'. Such three
-- 
2.8.0.rc3.226.g39d4020


^ permalink raw reply related

* Re: [PATCH 0/3] Introduce `submodule interngitdirs`
From: Junio C Hamano @ 2016-11-21 20:48 UTC (permalink / raw)
  To: Stefan Beller; +Cc: bmwill, jrnieder, git, Jens.Lehmann, hvoigt
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> The discussion of the submodule checkout series revealed to me
> that a command is needed to move the git directory from the
> submodules working tree to be embedded into the superprojects git
> directory.

You used "move" here, and "migrate" in the function name in 3/3,
both of which make sense.  

But "intern" sounds funny.  Who is confined as a prisoner here?
North American English uses that verb as "serve as an intern", but
that does not apply here.  The verb also is used in Lisp-ish
languages to mean the act of turning a string into a symbol, but
that does not apply here, either.

^ permalink raw reply

* Re: [PATCH 0/3] Introduce `submodule interngitdirs`
From: Stefan Beller @ 2016-11-21 20:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
	Jens Lehmann, Heiko Voigt
In-Reply-To: <xmqqd1hoimov.fsf@gitster.mtv.corp.google.com>

On Mon, Nov 21, 2016 at 12:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> The discussion of the submodule checkout series revealed to me
>> that a command is needed to move the git directory from the
>> submodules working tree to be embedded into the superprojects git
>> directory.
>
> You used "move" here, and "migrate" in the function name in 3/3,
> both of which make sense.
>
> But "intern" sounds funny.  Who is confined as a prisoner here?
> North American English uses that verb as "serve as an intern", but
> that does not apply here.  The verb also is used in Lisp-ish
> languages to mean the act of turning a string into a symbol, but
> that does not apply here, either.

I was inspired by the latter as we ask Git to make the submodule
"properly embedded" into the superproject, which is what I imagined
is similar to the lisp interning.

So I guess my imagination went to far and we rather want to invoke it via
"git submodule migrategitdirs" ?

But there you would ask "where are we migrating the git dirs to?", so
it would be reasonable to expect 2 parameters (just like the mv
command).

So maybe "git submodule embedgitdirs" ?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox