git-apply does not work in a sub-directory of a Git repository

git-apply does not work in a sub-directory of a Git repository

[Mehul Jain]

Hello everyone,

Recently while using git-apply, I observed that if git-apply is used in a
sub-directory of a Git repository then it silently dies without doing
anything.

Here's what I did

~ $mkdir example
~ $cd example
example $git init
Initialized empty Git repository in /home/mj/example/.git/
example $echo main >main
example $git add main
example $git commit -m 'main'
[master (root-commit) 97aeda3] main
 1 file changed, 1 insertion(+)
 create mode 100644 main
example $git checkout -b feature
Switched to a new branch 'feature'
example $echo modified >main
example $git add main
example $git commit -m 'modified'
[feature 2551fa2] modified
 1 file changed, 1 insertion(+), 1 deletion(-)
example $mkdir outgoing
example $git diff master >outgoing/feature.patch
example $git checkout master
Switched to branch 'master'
example $cd outgoing/
outgoing $git apply feature.patch
outgoing $cd ..
example $cat main
main

As you observed, patch wasn't applied. Is it intended behaviour of
git-apply? Usually to apply the patch I have to copy it to top directory
and then use git-apply.

I tried out git-am to apply the patch ("git format-patch" was used to
make patch) while being in the "outgoing" sub-directory and it worked
fine. So why does git-apply show this kind of behaviour?

Thanks,
Mehul

Re: git-apply does not work in a sub-directory of a Git repository

[Stefan Beller]

On Tue, Mar 22, 2016 at 5:10 AM, Mehul Jain <mehul.jain2029@gmail.com> wrote:
> Hello everyone,
>
> Recently while using git-apply, I observed that if git-apply is used in a
> sub-directory of a Git repository then it silently dies without doing
> anything.
>
> Here's what I did
>
> ~ $mkdir example
> ~ $cd example
> example $git init
> Initialized empty Git repository in /home/mj/example/.git/
> example $echo main >main
> example $git add main
> example $git commit -m 'main'
> [master (root-commit) 97aeda3] main
>  1 file changed, 1 insertion(+)
>  create mode 100644 main
> example $git checkout -b feature
> Switched to a new branch 'feature'
> example $echo modified >main
> example $git add main
> example $git commit -m 'modified'
> [feature 2551fa2] modified
>  1 file changed, 1 insertion(+), 1 deletion(-)
> example $mkdir outgoing
> example $git diff master >outgoing/feature.patch
> example $git checkout master
> Switched to branch 'master'
> example $cd outgoing/
> outgoing $git apply feature.patch
> outgoing $cd ..
> example $cat main
> main
>
> As you observed, patch wasn't applied. Is it intended behaviour of
> git-apply? Usually to apply the patch I have to copy it to top directory
> and then use git-apply.
>
> I tried out git-am to apply the patch ("git format-patch" was used to
> make patch) while being in the "outgoing" sub-directory and it worked
> fine. So why does git-apply show this kind of behaviour?
>
> Thanks,
> Mehul


Think of git-apply as a specialized version of 'patch', which would also
error out if there are path issues. (Inside outgoing/ there is no file found at
./main)

git-am is the porcelain command which is what is recommended to users
who interact with Git and patches.

Re: git-apply does not work in a sub-directory of a Git repository

[Duy Nguyen]

On Wed, Mar 23, 2016 at 5:14 AM, Stefan Beller <sbeller@google.com> wrote:
>> Hello everyone,
>> As you observed, patch wasn't applied. Is it intended behaviour of
>> git-apply? Usually to apply the patch I have to copy it to top directory
>> and then use git-apply.
>>
>> I tried out git-am to apply the patch ("git format-patch" was used to
>> make patch) while being in the "outgoing" sub-directory and it worked
>> fine. So why does git-apply show this kind of behaviour?
>
>
> Think of git-apply as a specialized version of 'patch', which would also
> error out if there are path issues. (Inside outgoing/ there is no file found at
> ./main)
>
> git-am is the porcelain command which is what is recommended to users
> who interact with Git and patches.

git-am is about patches in mailbox form, not plain patches, isn't it?
In that view, it's not a replacement for git-apply.

How about we start deprecating the old behavior?

1) add --no-index to force git-apply ignore .git, --git (or some other
name) to apply patches as if running from topdir, add a config key to
choose default behavior
2) when git-apply is run without --git, --index or --cached from a
subdir and the said config key is not set, start warning and
recommending --no-index
3) wait X years
4)switch default behavior to --git (if run inside a git repo)
-- 
Duy

Re: git-apply does not work in a sub-directory of a Git repository

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> 1) add --no-index to force git-apply ignore .git, --git (or some other
> name) to apply patches as if running from topdir, add a config key to
> choose default behavior

I think we do have --no-index (which is why I am largely ignoring
the rest of your message as uninformed speculation for now).

See

  http://thread.gmane.org/gmane.comp.version-control.git/288316/focus=288321

I agree it is bad that it silently ignores the path outside the
directory.  When run with --verbose, we should say "Skipped X that
is outside the directory." or something like that, just like we
issue notices when we applied with offset, etc.

Re: git-apply does not work in a sub-directory of a Git repository

[Junio C Hamano]

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

> See
>
>   http://thread.gmane.org/gmane.comp.version-control.git/288316/focus=288321
>
> I agree it is bad that it silently ignores the path outside the
> directory.  When run with --verbose, we should say "Skipped X that
> is outside the directory." or something like that, just like we
> issue notices when we applied with offset, etc.

Another thing we may want to do is to loosen (or redo) the logic
in builtin/apply.c::use_patch()

        static int use_patch(struct patch *p)
        {
                const char *pathname = p->new_name ? p->new_name : p->old_name;
                int i;

                /* Paths outside are not touched regardless of "--include" */
                if (0 < prefix_length) {
                        int pathlen = strlen(pathname);
                        if (pathlen <= prefix_length ||
                            memcmp(prefix, pathname, prefix_length))
                                return 0;
                }

The include/exclude mechanism does use wildmatch() but does not use
the pathspec mechanism (it predates the pathspec machinery that was
made reusable in places like this).  We should be able to

    $ cd d/e/e/p/d/i/r
    $ git apply --include=:/ ../../../../../../../patch

to lift this limitation.  IOW, we can think of the use_patch() to
include only the paths in the subdirectory we are in by default, but
we can make it allow --include/--exclude command line option to
override that default.

That way, the plain-vanilla use would still retain the "when working
in subdirectory, we only touch that subdirectory" behaviour, which
existing scripts may depend on, but users can loosen the default as
necessary.

Re: git-apply does not work in a sub-directory of a Git repository

[Duy Nguyen]

On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> See
>>
>>   http://thread.gmane.org/gmane.comp.version-control.git/288316/focus=288321
>>
>> I agree it is bad that it silently ignores the path outside the
>> directory.  When run with --verbose, we should say "Skipped X that
>> is outside the directory." or something like that, just like we
>> issue notices when we applied with offset, etc.
>
> Another thing we may want to do is to loosen (or redo) the logic
> in builtin/apply.c::use_patch()
>
>         static int use_patch(struct patch *p)
>         {
>                 const char *pathname = p->new_name ? p->new_name : p->old_name;
>                 int i;
>
>                 /* Paths outside are not touched regardless of "--include" */
>                 if (0 < prefix_length) {
>                         int pathlen = strlen(pathname);
>                         if (pathlen <= prefix_length ||
>                             memcmp(prefix, pathname, prefix_length))
>                                 return 0;
>                 }
>
> The include/exclude mechanism does use wildmatch() but does not use
> the pathspec mechanism (it predates the pathspec machinery that was
> made reusable in places like this).  We should be able to
>
>     $ cd d/e/e/p/d/i/r
>     $ git apply --include=:/ ../../../../../../../patch
>
> to lift this limitation.  IOW, we can think of the use_patch() to
> include only the paths in the subdirectory we are in by default, but
> we can make it allow --include/--exclude command line option to
> override that default.

Interesting. Disabling that comment block seems to work ok. So
git-apply works more like git-grep, automatically narrowing to current
subdir, rather than full-tree like git-status. git-apply.txt should
probably mention about this because (at least to me) it sounds more
naturally that if I give a patch, git-apply should apply the whole
patch.

We probably should show a warning if everything file is filtered out
too because silence usually means "good" from a typical unix command.
It could be guarded with advice config key, and should only show if it
looks like there are matching paths on worktree, but filtered out.
Hmm?

> That way, the plain-vanilla use would still retain the "when working
> in subdirectory, we only touch that subdirectory" behaviour, which
> existing scripts may depend on, but users can loosen the default as
> necessary.
-- 
Duy

Re: git-apply does not work in a sub-directory of a Git repository

[Nguyễn Thái Ngọc Duy]

+Brian who also had issues with git-apply.

On Thu, Mar 24, 2016 at 5:49 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> See
>>>
>>>   http://thread.gmane.org/gmane.comp.version-control.git/288316/focus=288321
>>>
>>> I agree it is bad that it silently ignores the path outside the
>>> directory.  When run with --verbose, we should say "Skipped X that
>>> is outside the directory." or something like that, just like we
>>> issue notices when we applied with offset, etc.

Implemented in [04/04] apply: report patch skipping in verbose mode.

>> Another thing we may want to do is to loosen (or redo) the logic
>> in builtin/apply.c::use_patch()
>>
>>         static int use_patch(struct patch *p)
>>         {
>>                 const char *pathname = p->new_name ? p->new_name : p->old_name;
>>                 int i;
>>
>>                 /* Paths outside are not touched regardless of "--include" */
>>                 if (0 < prefix_length) {
>>                         int pathlen = strlen(pathname);
>>                         if (pathlen <= prefix_length ||
>>                             memcmp(prefix, pathname, prefix_length))
>>                                 return 0;
>>                 }
>>
>> The include/exclude mechanism does use wildmatch() but does not use
>> the pathspec mechanism (it predates the pathspec machinery that was
>> made reusable in places like this).  We should be able to
>>
>>     $ cd d/e/e/p/d/i/r
>>     $ git apply --include=:/ ../../../../../../../patch
>>
>> to lift this limitation.  IOW, we can think of the use_patch() to
>> include only the paths in the subdirectory we are in by default, but
>> we can make it allow --include/--exclude command line option to
>> override that default.

I went with a new option instead of changing --include. Making it
pathspec can still bite people. And pathspec is not exactly compatible
with wildmatch either. This is in

  [03/04] apply: add --whole to apply git patch without prefix filtering

> git-apply.txt should
> probably mention about this because (at least to me) it sounds more
> naturally that if I give a patch, git-apply should apply the whole
> patch.

  [02/04] git-apply.txt: mention the behavior inside a subdir

> We probably should show a warning if everything file is filtered out
> too because silence usually means "good" from a typical unix command.
> It could be guarded with advice config key, and should only show if it
> looks like there are matching paths on worktree, but filtered out.

I'm holding this back. Too much heuristics.
--
Duy

[PATCH 1/4] git-apply.txt: remove a space

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-apply.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index d9ed6a1..5444d2f 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	  [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
 	  [--allow-binary-replacement | --binary] [--reject] [-z]
 	  [-p<n>] [-C<n>] [--inaccurate-eof] [--recount] [--cached]
-	  [--ignore-space-change | --ignore-whitespace ]
+	  [--ignore-space-change | --ignore-whitespace]
 	  [--whitespace=(nowarn|warn|fix|error|error-all)]
 	  [--exclude=<path>] [--include=<path>] [--directory=<root>]
 	  [--verbose] [--unsafe-paths] [<patch>...]
-- 
2.8.0.rc0.210.gd302cd2

[PATCH 2/4] git-apply.txt: mention the behavior inside a subdir

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-apply.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 5444d2f..8ddb207 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -21,6 +21,8 @@ SYNOPSIS
 DESCRIPTION
 -----------
 Reads the supplied diff output (i.e. "a patch") and applies it to files.
+When running from a subdirectory in a repository, patched paths
+outside the directory are ignored.
 With the `--index` option the patch is also applied to the index, and
 with the `--cached` option the patch is only applied to the index.
 Without these options, the command applies the patch only to files,
-- 
2.8.0.rc0.210.gd302cd2

[PATCH 3/4] apply: add --whole to apply git patch without prefix filtering

[Nguyễn Thái Ngọc Duy]

Back in edf2e37 (git-apply: work from subdirectory. - 2005-11-25),
git-apply is made to work from a subdir of a worktree. When applying a
git patch this way, only paths in the subdir are patched, the rest is
filtered out. To apply without filtering, the user has to move back to
toplevel. Add --whole to make it more convenient to do so without moving
cwd around.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-apply.txt |  8 +++++++-
 builtin/apply.c             |  4 +++-
 t/t4111-apply-subdir.sh     | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 8ddb207..47cea57 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	  [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
 	  [--allow-binary-replacement | --binary] [--reject] [-z]
 	  [-p<n>] [-C<n>] [--inaccurate-eof] [--recount] [--cached]
-	  [--ignore-space-change | --ignore-whitespace]
+	  [--ignore-space-change | --ignore-whitespace] [--whole]
 	  [--whitespace=(nowarn|warn|fix|error|error-all)]
 	  [--exclude=<path>] [--include=<path>] [--directory=<root>]
 	  [--verbose] [--unsafe-paths] [<patch>...]
@@ -154,6 +154,12 @@ discouraged.
 	flag was the way to do so.  Currently we always allow binary
 	patch application, so this is a no-op.
 
+--whole::
+	Normally when running inside a subdirectory of a working area,
+	patched files outside current directory is filtered out. This option
+	makes `git apply` to apply them all. All paths are still subject
+	to `--exclude` and `--include` fitlering if present.
+
 --exclude=<path-pattern>::
 	Don't apply changes to files matching the given path pattern. This can
 	be useful when importing patchsets, where you want to exclude certain
diff --git a/builtin/apply.c b/builtin/apply.c
index 42c610e..01e1d5e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -80,6 +80,7 @@ static const char *patch_input_file;
 static struct strbuf root = STRBUF_INIT;
 static int read_stdin = 1;
 static int options;
+static int apply_whole;
 
 static void parse_whitespace_option(const char *option)
 {
@@ -1956,7 +1957,7 @@ static int use_patch(struct patch *p)
 	int i;
 
 	/* Paths outside are not touched regardless of "--include" */
-	if (0 < prefix_length) {
+	if (!apply_whole && 0 < prefix_length) {
 		int pathlen = strlen(pathname);
 		if (pathlen <= prefix_length ||
 		    memcmp(prefix, pathname, prefix_length))
@@ -4565,6 +4566,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 		OPT_BIT(0, "recount", &options,
 			N_("do not trust the line counts in the hunk headers"),
 			RECOUNT),
+		OPT_BOOL(0, "whole", &apply_whole, N_("apply whole patch")),
 		{ OPTION_CALLBACK, 0, "directory", NULL, N_("root"),
 			N_("prepend <root> to all filenames"),
 			0, option_parse_directory },
diff --git a/t/t4111-apply-subdir.sh b/t/t4111-apply-subdir.sh
index 1618a6d..e5cd019 100755
--- a/t/t4111-apply-subdir.sh
+++ b/t/t4111-apply-subdir.sh
@@ -153,4 +153,36 @@ test_expect_success 'apply --cached from subdir of .git dir' '
 	test_cmp expected.subdir actual.subdir
 '
 
+test_expect_success 'setup a git patch' '
+	cat >gitpatch <<-\EOF &&
+	diff --git a/file b/file
+	--- a/file
+	+++ b/file
+	@@ -1 +1,2 @@
+	 1
+	+2
+	EOF
+	gitpatch="$(pwd)/gitpatch"
+'
+
+test_expect_success 'apply a git patch from subdir of toplevel' '
+	reset_subdir other preimage &&
+	(
+		cd sub/dir &&
+		git apply "$gitpatch"
+	) &&
+	test_cmp preimage sub/dir/file &&
+	test_cmp preimage file
+'
+
+test_expect_success 'apply the whole git patch from subdir of toplevel' '
+	reset_subdir other preimage &&
+	(
+		cd sub/dir &&
+		git apply --whole "$gitpatch"
+	) &&
+	test_cmp preimage sub/dir/file &&
+	test_cmp postimage file
+'
+
 test_done
-- 
2.8.0.rc0.210.gd302cd2

[PATCH 4/4] apply: report patch skipping in verbose mode

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/apply.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/apply.c b/builtin/apply.c
index 01e1d5e..9cbb186 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4384,6 +4384,8 @@ static int apply_patch(int fd, const char *filename, int options)
 			listp = &patch->next;
 		}
 		else {
+			if (apply_verbosely)
+				say_patch_name(stderr, _("Skipped patch '%s'."), patch);
 			free_patch(patch);
 			skipped_patch++;
 		}
-- 
2.8.0.rc0.210.gd302cd2

Re: git-apply does not work in a sub-directory of a Git repository

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

>> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> The include/exclude mechanism does use wildmatch() but does not use
>>> the pathspec mechanism (it predates the pathspec machinery that was
>>> made reusable in places like this).  We should be able to
>>>
>>>     $ cd d/e/e/p/d/i/r
>>>     $ git apply --include=:/ ../../../../../../../patch
>>>
>>> to lift this limitation.  IOW, we can think of the use_patch() to
>>> include only the paths in the subdirectory we are in by default, but
>>> we can make it allow --include/--exclude command line option to
>>> override that default.
>
> I went with a new option instead of changing --include.

It might be a "workable" band-aid, but would be an unsatisfying UI
if it were the endgame state.  You do not say "git grep --whole" (by
the way, "whole" is a bad option name, as you cannot tell "100% of
*what*" you are referring to--what you are widening is the limit
based on the location in the directory structure, so the option name
should have some hint about it, e.g. "full-tree" or something) and
this command will become an odd-man-out.

I haven't thought things through, but thinking out aloud a few
points...

  An existing user/script may be working in a subdirectory of a huge
  working tree and relies on the current behaviour that outside world
  is excluded by default, and may be passing --exclude to further
  limit the extent of damage by applying the patch to a subset of
  paths in the current directory that itself is also huge.  And that
  use case would not be harmed by such a change.

  On the other hand, an existing user/script would not be passing an
  "--include" that names outside the current subdirectory to force
  them to be included, because it is known for the past 10 years not
  to have any effect at all.

So a better alternative may be to conditionally disable the "Paths
outside are not touched regardless of --include" logic, i.e. we
exclude paths outside by default just as before, but if there is at
least one explicit "--include" given, we skip this "return 0".

That way, we do not have to commit to turning --include/--exclude to
pathspec (which I agree is a huge change in behaviour that may not
be a good idea) and we do not have to add "--full-tree" that is only
understood by "apply" but not other commands that operate on the
current directory by default.

I agree that the "excluded because the path is outside cwd" should
be reported just like we show notices when applying a hunk with
offset, and that the "excluded because the path is outside cwd"
should be documented.

Re: git-apply does not work in a sub-directory of a Git repository

[Junio C Hamano]

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

> So a better alternative may be to conditionally disable the "Paths
> outside are not touched regardless of --include" logic, i.e. we
> exclude paths outside by default just as before, but if there is at
> least one explicit "--include" given, we skip this "return 0".
>
> That way, we do not have to commit to turning --include/--exclude to
> pathspec (which I agree is a huge change in behaviour that may not
> be a good idea) and we do not have to add "--full-tree" that is only
> understood by "apply" but not other commands that operate on the
> current directory by default.

And the necessary change to do so may look like this.  With this:

    $ git show >P
    $ git reset --hard HEAD^
    $ cd t
    $ git apply -v ../P
    $ git apply -v --include=\* ../P

seem to work as expected.

diff --git a/builtin/apply.c b/builtin/apply.c
index c993333..1af3f7e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1955,8 +1955,8 @@ static int use_patch(struct patch *p)
 	const char *pathname = p->new_name ? p->new_name : p->old_name;
 	int i;
 
-	/* Paths outside are not touched regardless of "--include" */
-	if (0 < prefix_length) {
+	/* Paths outside are not touched when there is no explicit "--include" */
+	if (!has_include && 0 < prefix_length) {
 		int pathlen = strlen(pathname);
 		if (pathlen <= prefix_length ||
 		    memcmp(prefix, pathname, prefix_length))

Re: git-apply does not work in a sub-directory of a Git repository

[Duy Nguyen]

On Thu, Mar 24, 2016 at 11:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>>> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> The include/exclude mechanism does use wildmatch() but does not use
>>>> the pathspec mechanism (it predates the pathspec machinery that was
>>>> made reusable in places like this).  We should be able to
>>>>
>>>>     $ cd d/e/e/p/d/i/r
>>>>     $ git apply --include=:/ ../../../../../../../patch
>>>>
>>>> to lift this limitation.  IOW, we can think of the use_patch() to
>>>> include only the paths in the subdirectory we are in by default, but
>>>> we can make it allow --include/--exclude command line option to
>>>> override that default.
>>
>> I went with a new option instead of changing --include.
>
> It might be a "workable" band-aid, but would be an unsatisfying UI
> if it were the endgame state.  You do not say "git grep --whole" (by
> the way, "whole" is a bad option name, as you cannot tell "100% of
> *what*" you are referring to--what you are widening is the limit
> based on the location in the directory structure, so the option name
> should have some hint about it, e.g. "full-tree" or something) and
> this command will become an odd-man-out.
>
> I haven't thought things through, but thinking out aloud a few
> points...
>
>   An existing user/script may be working in a subdirectory of a huge
>   working tree and relies on the current behaviour that outside world
>   is excluded by default, and may be passing --exclude to further
>   limit the extent of damage by applying the patch to a subset of
>   paths in the current directory that itself is also huge.  And that
>   use case would not be harmed by such a change.
>
>   On the other hand, an existing user/script would not be passing an
>   "--include" that names outside the current subdirectory to force
>   them to be included, because it is known for the past 10 years not
>   to have any effect at all.

Real-world .gitignore patterns have taught me that even if it does not
have any effect, it might still be present in some scripts, waiting
for a chance to bite me.

> So a better alternative may be to conditionally disable the "Paths
> outside are not touched regardless of --include" logic, i.e. we
> exclude paths outside by default just as before, but if there is at
> least one explicit "--include" given, we skip this "return 0".
>
> That way, we do not have to commit to turning --include/--exclude to
> pathspec (which I agree is a huge change in behaviour that may not
> be a good idea) and we do not have to add "--full-tree" that is only
> understood by "apply" but not other commands that operate on the
> current directory by default.

But your suggestion is good and I can't think of any better. We could
introduce pathspec as ftiler after "--", but it does not look elegant,
and it overlaps with --include/--exclude.

Perhaps we can start to warn people if --include is specified but has
no effect for a cycle or two, then we can do as you suggested?
-- 
Duy

Re: git-apply does not work in a sub-directory of a Git repository

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> But your suggestion is good and I can't think of any better. We could
> introduce pathspec as ftiler after "--", but it does not look elegant,
> and it overlaps with --include/--exclude.

I was imagining that we would allow the magic pathspec syntax used
in --include/--exclude command line option parameter.  Nobody sane
uses glob special characters in their pathnames and those that do
deserve whatever breakage that comes to them.

> Perhaps we can start to warn people if --include is specified but has
> no effect for a cycle or two, then we can do as you suggested?

I do not think I'd be against going in that direction.

Re: git-apply does not work in a sub-directory of a Git repository

[Duy Nguyen]

On Thu, Mar 24, 2016 at 11:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> So a better alternative may be to conditionally disable the "Paths
> outside are not touched regardless of --include" logic, i.e. we
> exclude paths outside by default just as before, but if there is at
> least one explicit "--include" given, we skip this "return 0".
>
> That way, we do not have to commit to turning --include/--exclude to
> pathspec (which I agree is a huge change in behaviour that may not
> be a good idea) and we do not have to add "--full-tree" that is only
> understood by "apply" but not other commands that operate on the
> current directory by default.

Suppose I don't like git-apply's default behavior, I make an alias.ap
= "apply --include=*". So far so good, but when I want to limit paths
back to "subdir" (it does not have to be the same as cwd), how do I do
it without typing resorting to typing "git apply" explicitly ? I don't
see an option to cancel out --include=*. For "git ap --exclude=*
--include=subdir" to have that effect, we need to change

for (i = 0; i < limit_by_name.nr; i++) {

in use_patch() to

for (i = limit_by_name.nr - 1; i >= 0; i--) {

Simple change, but not exactly harmless.

Off topic, but --include/--exclude should be able to deal with
relative path like --include=../*.c or --include=./*. I guess nobody
has complained about it, so it's not needed.
-- 
Duy

Re: git-apply does not work in a sub-directory of a Git repository

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> The include/exclude mechanism does use wildmatch() but does not use
>> the pathspec mechanism (it predates the pathspec machinery that was
>> made reusable in places like this).  We should be able to
>>
>>     $ cd d/e/e/p/d/i/r
>>     $ git apply --include=:/ ../../../../../../../patch
>>
>> to lift this limitation.  IOW, we can think of the use_patch() to
>> include only the paths in the subdirectory we are in by default, but
>> we can make it allow --include/--exclude command line option to
>> override that default.
>
> Interesting. Disabling that comment block seems to work ok. So
> git-apply works more like git-grep, automatically narrowing to current
> subdir, rather than full-tree like git-status.

We used to have one argument when choosing between "limit to cwd" vs
"work on full-tree" defaults, i.e. "a full-tree thing can easily be
limited to cwd by passing '.' as a pathspec, but limited-to-cwd thing
cannot be widened" before we introduced the magic "full-tree" pathspec.

This "limit to cwd" behaviour of "git apply" predates that by
several years.

> git-apply.txt should
> probably mention about this because (at least to me) it sounds more
> naturally that if I give a patch, git-apply should apply the whole
> patch.

Yes, documentation update is necessary.

> We probably should show a warning if everything file is filtered out
> too because silence usually means "good" from a typical unix command.

We should give an informational message if _anything_ is filtered
out, I would say.

Re: git-apply does not work in a sub-directory of a Git repository

[Mehul Jain]

On Wed, Mar 23, 2016 at 8:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I think we do have --no-index (which is why I am largely ignoring
> the rest of your message as uninformed speculation for now).

--no-index command line flag is there for git-apply but unfortunately not
documented.

Also *auto-completion* for "git apply --no-index" doesn't work.

That is

    git apply --no-i<tab><tab>

should be auto completed and give

   git apply --no-index.

Probably following change will remove this problem.

diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 45ec47f..860dae0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -886,7 +886,7 @@ _git_apply ()
                ;;
        --*)
                __gitcomp "
-                       --stat --numstat --summary --check --index
+                       --stat --numstat --summary --check --index --no-index
                        --cached --index-info --reverse --reject --unidiff-zero
                        --apply --no-add --exclude=
                        --ignore-whitespace --ignore-space-change