Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Stefan Beller @ 2016-12-13 20:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <CAGZ79kYM_3NWyRfk42=EshMYVZ=DSWRtn4RU4jkUE7v1EN6ngg@mail.gmail.com>

On Tue, Dec 13, 2016 at 12:09 PM, Stefan Beller <sbeller@google.com> wrote:
>
> So I will reroll it with "absorb" fixing some nits pointed out by David?

I got confused there, Davids nits are for this series, the absorb series itself
doesn't seem to have nits.

So I'll just reroll this series on top of the currently
sb/submodule-embed-gitdir (which you originally noted to be better renamed to
submodule-absorb-gitdir) merged with t3600-cleanup.

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Stefan Beller @ 2016-12-13 20:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <xmqq60mn3937.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 13, 2016 at 11:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> The desired standard for submodules is to have the git dir inside the
>> superprojects git dir (since  501770e, Aug 2011, Move git-dir for
>> submodules), which is why I think an "embedded submodule git dir"
>> is inside the superproject already.
>
> Think how you start a new submodule.  What are the steps you take
> before you say "git submodule add"?  And where does .git for the
> submodule live at that point?

Well there is no way to directly start a submodule
(e.g. "git submodule create"), such that there has to be one repo
that actually has the git dir inside the working tree.

If the submodule exists already somewhere, there are 2 ways to do it
("git submodule add <URL>"  or "git clone && git submodule add")
which lead to different outcomes, where the .git resides.

> With the current system, you as the submodule originator need to do
> something different to make your working tree of the superproject
> match what the others who clone from your public repository.
>
> And comparing the two layout, the one originally held by the
> submodule originator has .git embedded in the working tree, no?

When starting a new submodule repo, yes, the git dir is inside
the working tree.

> All of the above is coming from "submodule" centric mindset.  It
> just is not centric to those who follow what others originated.

ok.

> Another reason why I personally see a .git in each submodule working
> tree is "embedded" has nothing to do with Git.  It is an analogy I
> feel (perhaps it is just me) with the word "embedded reporters in
> warzone".  These people are spread around, assigned to units to
> report from places closer to the front line and being closer to the
> leaf of the hierarchy, as opposed to be assigned to a more central
> place like HQ to do their reporting.

I talked to people in the office and got a heavy rejection on the
the work "embedded" here for another reason:

    "Does it put the submodule on an embedded device?
    What does embedded even mean?
    The end user is super confused"

So I think we should not use embed or un-embed one way or the other.
Instead we need to have another name.

I think absorb is ok-ish, as "git submodule absorb" hints that the
superproject absorbs something from the submodule.

So I will reroll it with "absorb" fixing some nits pointed out by David?

^ permalink raw reply

* Re: [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Junio C Hamano @ 2016-12-13 20:10 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161213181538.6gv4it4b33uhbuud@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> 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.:

Ah, my mistake.  Staring at too many `eval`s does it to me.

Thanks.

^ permalink raw reply

* Re: [RFC/PATCH v3 00/16] Add initial experimental external ODB support
From: Junio C Hamano @ 2016-12-13 20:05 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Jeff King, Nguyen Thai Ngoc Duy, Mike Hommey, Lars Schneider,
	Eric Wong, Christian Couder
In-Reply-To: <CAP8UFD1=-xKWaDnGKrtm2mzVxpH7N-Q3iqnOJeOM5QrtNpitrA@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

> In general I think that having a lot of refs is really a big problem
> right now in Git as many big organizations using Git are facing this
> problem in one form or another.
> So I think that support for a big number of refs is a separate and
> important problem that should and hopefully will be solved.

But you do not have to make it worse.

Is "refs" a good match for the problem you are solving?  Or is it
merely an expedient thing to use?  I think it is the latter, judging
by your mentioning RefTree.  Whatever mechanism we choose, that will
be carved into stone in users' repositories and you'd end up having
to support it, and devise the migration path out of it if the initial
selection is too problematic.

That is why people (not just me) pointed out upfront that using refs
for this purose would not scale.

^ permalink raw reply

* Re: git add -p with unmerged files
From: Junio C Hamano @ 2016-12-13 19:59 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Ariel, git, Duy Nguyen, Jeff King
In-Reply-To: <98817141-fa57-7687-09c4-dc96419d8a35@gmx.net>

Stephan Beyer <s-beyer@gmx.net> writes:

> While we're on the topic that "git add -p" should behave like the
> "normal" "git add" (not "git add -u"): what about unmerged changes?
>
>
> When I have merge conflicts, I almost always use my aliases
> "edit-unmerged" and "add-unmerged":
>
> $ git config --global --list | grep unmerged
> alias.list-unmerged=diff --name-only --diff-filter=U
> alias.edit-unmerged=!vim `git list-unmerged`
> alias.add-unmerged=!git add `git list-unmerged`
> alias.reset-unmerged=!uf=`git list-unmerged`; git reset HEAD $uf; git
> checkout -- $uf
>
> The "add-unmerged" alias is always a little scary because I'd rather
> like to check the changes with the "git add -p" workflow I am used to.
>
> Opinions?

For this, you would NEVER want to use "add -p" to pick and choose.
By definition, while you are in conflicted merge, the path that had
conflicts before you started the merge-y operation (be it "pull",
"am -3", or "cherry-pick") did not have any change since HEAD, and
"pick this hunk, drop that hunk" cannot be correct for the conflict
resolution.

"git diff" while conflicted will highlight what conflicted by
showing the three-way diff (similar to "diff --cc" on a merge
result) and after conflict is resolved you can view "diff HEAD"
on the path to see what the merge brought in.

^ permalink raw reply

* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Junio C Hamano @ 2016-12-13 19:54 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <CAGZ79kYsfybEBnWzv4OjCCLe70fNS=roZdKDbN_DSb4PDVJj7g@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> I guess the latter is the case, so embedding is actually inside
> the working tree and un-embedding is the relocation to the
> superproject.

Another reason why I personally see a .git in each submodule working
tree is "embedded" has nothing to do with Git.  It is an analogy I
feel (perhaps it is just me) with the word "embedded reporters in
warzone".  These people are spread around, assigned to units to
report from places closer to the front line and being closer to the
leaf of the hierarchy, as opposed to be assigned to a more central
place like HQ to do their reporting.

^ permalink raw reply

* Re: git add -p with unmerged files (was: git add -p with new file)
From: Jeff King @ 2016-12-13 19:49 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Ariel, git, Duy Nguyen, Junio C Hamano
In-Reply-To: <98817141-fa57-7687-09c4-dc96419d8a35@gmx.net>

On Tue, Dec 13, 2016 at 08:21:59PM +0100, Stephan Beyer wrote:

> While we're on the topic that "git add -p" should behave like the
> "normal" "git add" (not "git add -u"): what about unmerged changes?

I agree that's a related part of the workflow, though the implementation
is a bit harder.

> When I have merge conflicts, I almost always use my aliases
> "edit-unmerged" and "add-unmerged":
> 
> $ git config --global --list | grep unmerged
> alias.list-unmerged=diff --name-only --diff-filter=U
> alias.edit-unmerged=!vim `git list-unmerged`

You might like contrib/git-jump for that, which makes it easier to go to
the specific spots within files.

When "git jump merge" produces no hits, I know I've dealt with all of
the conflicts (textual ones, anyway). I do often want to run "git add
-p" then to review the changes before staging.

I think what is most helpful there is probably "git diff HEAD" to see
what the merge is bringing in (or the cherry-pick, or "am", or
whatever). If I wanted "add -p" to do anything, I think it would be to
act as if stage 2 ("ours", which should be the same as what is in HEAD)
was already in the index. I.e., show the diff against that, apply any
hunks we select, and store the whole thing as stage 0, losing the
unmerged bit.

When you select all hunks, this is equivalent to "git add unmerge-file".
If you choose only a subset of hunks, it leaves the unselected ones for
you to examine later via "git diff". And if you choose none, it should
probably leave unmerged.

That's just a scheme I thought up, though. I've never actually tried it
in practice.

-Peff

^ permalink raw reply

* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Junio C Hamano @ 2016-12-13 19:47 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <CAGZ79kYsfybEBnWzv4OjCCLe70fNS=roZdKDbN_DSb4PDVJj7g@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> The desired standard for submodules is to have the git dir inside the
> superprojects git dir (since  501770e, Aug 2011, Move git-dir for
> submodules), which is why I think an "embedded submodule git dir"
> is inside the superproject already.

Think how you start a new submodule.  What are the steps you take
before you say "git submodule add"?  And where does .git for the
submodule live at that point?

With the current system, you as the submodule originator need to do
something different to make your working tree of the superproject
match what the others who clone from your public repository.

And comparing the two layout, the one originally held by the
submodule originator has .git embedded in the working tree, no?

All of the above is coming from "submodule" centric mindset.  It
just is not centric to those who follow what others originated.

^ permalink raw reply

* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Stefan Beller @ 2016-12-13 19:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <xmqqtwa73ara.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 13, 2016 at 11:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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".

ok, I will take unembed then.  We could also go with more command line options
such as "embed --reverse" or such, but that is not as nice I'd think.

^ permalink raw reply

* Re: [PATCH v2 00/34] Teach the sequencer to act as rebase -i's backend
From: Junio C Hamano @ 2016-12-13 19:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <cover.1481642927.git.johannes.schindelin@gmx.de>

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

> This marks the count down to '3': two more patch series after this
> (really tiny ones) and we have a faster rebase -i.

Nice.

> Apart from mostly cosmetic patches (and the occasional odd bug that I
> fixed promptly), I used these patches since mid May to perform all of my
> interactive rebases. In mid June, I had the idea to teach rebase -i to
> run *both* scripted rebase and rebase--helper and to cross-validate the
> results. This slowed down all my interactive rebases since, but helped
> me catch three rather obscure bugs (e.g. that git commit --fixup unfolds
> long onelines and rebase -i still finds the correct original commit).
> ...
> Please note that the interdiff vs v1 is only of limited use: too many
> things changed in the meantime, in particular the prepare-sequencer
> branch that went through a couple of iterations before it found its way
> into git.git's master branch. So please take the interdiff with a
> mountain range of salt.
> ...
> Changes since v1:
> ...
> - removed the beautiful ordinal logic (to print out "1st", "2nd", "3rd"
>   etc) and made things consistent with the current `rebase -i`.

It was removed because it was too Anglo-centric and unusable in i18n
context, no?

Judging from the list above, interdiff are pretty much all cosmetic
and that is why you say it is only of limited use, I guess.

    ... goes and reads the remainder and finds that these were
    ... all minor changes, mostly cosmetic, with a helper function
    ... refactored out or two and things of that nature.

It is actually a good thing.  We do not want to see it deviate too
drastically from what you have been testing for some months.

Thanks.

^ permalink raw reply

* Re: [PATCH 0/6] git-rm absorbs submodule git directory before deletion
From: Stefan Beller @ 2016-12-13 19:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, David Turner, Brandon Williams
In-Reply-To: <CAGZ79kZCza=cwtzQ7raU3ch_Z_5TDqt0AGN2fPHiRSTDu66Fag@mail.gmail.com>

On Tue, Dec 13, 2016 at 11:13 AM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Dec 13, 2016 at 11:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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").

I do not agree, yet.
So I thought about this for a while.

The standard in Git is to have the .git directory inside the working tree,
which is why you are convinced that embedded means the .git is in the
working tree, because you approach this discussion as the Git maintainer,
spending only little time on submodule related stuff.

The desired standard for submodules is to have the git dir inside the
superprojects git dir (since  501770e, Aug 2011, Move git-dir for
submodules), which is why I think an "embedded submodule git dir"
is inside the superproject already.

I think both views are legit, and we would want to choose the one that
users find most intuitive (and I think there will be users that find either
viewpoint intuitive).

So when you have typed "git submodule ", I wonder if a user would
assume a submodule-centric mindset of how submodules ought to
work or if they still look at a submodule as its own git repo
that just happens to be embedded into the superproject.

I guess the latter is the case, so embedding is actually inside the working
tree and un-embedding is the relocation to the superproject.

^ permalink raw reply

* git add -p with unmerged files (was: git add -p with new file)
From: Stephan Beyer @ 2016-12-13 19:21 UTC (permalink / raw)
  To: Ariel, git; +Cc: Duy Nguyen, Junio C Hamano, Jeff King
In-Reply-To: <alpine.DEB.2.11.1612062012540.13185@cherryberry.dsgml.com>

Hi,

While we're on the topic that "git add -p" should behave like the
"normal" "git add" (not "git add -u"): what about unmerged changes?

When I have merge conflicts, I almost always use my aliases
"edit-unmerged" and "add-unmerged":

$ git config --global --list | grep unmerged
alias.list-unmerged=diff --name-only --diff-filter=U
alias.edit-unmerged=!vim `git list-unmerged`
alias.add-unmerged=!git add `git list-unmerged`
alias.reset-unmerged=!uf=`git list-unmerged`; git reset HEAD $uf; git
checkout -- $uf

The "add-unmerged" alias is always a little scary because I'd rather
like to check the changes with the "git add -p" workflow I am used to.

Opinions?

Best
  Stephan

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2016, #02; Mon, 12)
From: Junio C Hamano @ 2016-12-13 19:28 UTC (permalink / raw)
  To: Brandon Williams; +Cc: git, Johannes Schindelin
In-Reply-To: <alpine.DEB.2.20.1612131839220.3147@virtualbox>

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

> 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" &&
> ...
> 	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

I think !MINGW prereq is missing?


^ permalink raw reply

* 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


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