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

[Junio C Hamano]

Duy Nguyen  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>>> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Junio C Hamano  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 c99..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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

>> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Duy Nguyen  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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  wrote:
> On Wed, Mar 23, 2016 at 11:55 PM, Junio C Hamano  wrote:
>> Junio C Hamano  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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  wrote:
> Junio C Hamano  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Junio C Hamano  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Duy Nguyen  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html