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

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Stefan Beller-4
On Tue, Mar 22, 2016 at 5:10 AM, Mehul Jain <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
Duy Nguyen <[hidden email]> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
Junio C Hamano <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Mehul Jain
In reply to this post by Junio C Hamano
On Wed, Mar 23, 2016 at 8:51 PM, Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
In reply to this post by Junio C Hamano
On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <[hidden email]> wrote:

> Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
+Brian who also had issues with git-apply.

On Thu, Mar 24, 2016 at 5:49 PM, Duy Nguyen <[hidden email]> wrote:

> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <[hidden email]> wrote:
>> Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
In reply to this post by Duy Nguyen
Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
In reply to this post by Duy Nguyen
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 <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
In reply to this post by Duy Nguyen
Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
In reply to this post by Duy Nguyen
Duy Nguyen <[hidden email]> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
In reply to this post by Duy Nguyen
Nguyễn Thái Ngọc Duy  <[hidden email]> writes:

>> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
Junio C Hamano <[hidden email]> 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))
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
In reply to this post by Junio C Hamano
On Thu, Mar 24, 2016 at 11:50 PM, Junio C Hamano <[hidden email]> wrote:

> Nguyễn Thái Ngọc Duy  <[hidden email]> writes:
>
>>> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Duy Nguyen
In reply to this post by Junio C Hamano
On Thu, Mar 24, 2016 at 11:50 PM, Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
In reply to this post by Duy Nguyen
Duy Nguyen <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html