* git-apply does not work in a sub-directory of a Git repository
@ 2016-03-22 12:10 Mehul Jain
2016-03-22 22:14 ` Stefan Beller
0 siblings, 1 reply; 18+ messages in thread
From: Mehul Jain @ 2016-03-22 12:10 UTC (permalink / raw)
To: Git Mailing List
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-22 12:10 git-apply does not work in a sub-directory of a Git repository Mehul Jain
@ 2016-03-22 22:14 ` Stefan Beller
2016-03-23 10:15 ` Duy Nguyen
0 siblings, 1 reply; 18+ messages in thread
From: Stefan Beller @ 2016-03-22 22:14 UTC (permalink / raw)
To: Mehul Jain; +Cc: Git Mailing List
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-22 22:14 ` Stefan Beller
@ 2016-03-23 10:15 ` Duy Nguyen
2016-03-23 15:21 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: Duy Nguyen @ 2016-03-23 10:15 UTC (permalink / raw)
To: Stefan Beller; +Cc: Mehul Jain, Git Mailing List
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-23 10:15 ` Duy Nguyen
@ 2016-03-23 15:21 ` Junio C Hamano
2016-03-23 16:55 ` Junio C Hamano
2016-03-23 17:24 ` Mehul Jain
0 siblings, 2 replies; 18+ messages in thread
From: Junio C Hamano @ 2016-03-23 15:21 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Stefan Beller, Mehul Jain, Git Mailing List
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-23 15:21 ` Junio C Hamano
@ 2016-03-23 16:55 ` Junio C Hamano
2016-03-24 10:49 ` Duy Nguyen
2016-03-23 17:24 ` Mehul Jain
1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2016-03-23 16:55 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Stefan Beller, Mehul Jain, Git Mailing List
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-23 15:21 ` Junio C Hamano
2016-03-23 16:55 ` Junio C Hamano
@ 2016-03-23 17:24 ` Mehul Jain
1 sibling, 0 replies; 18+ messages in thread
From: Mehul Jain @ 2016-03-23 17:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Stefan Beller, Git Mailing List
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-23 16:55 ` Junio C Hamano
@ 2016-03-24 10:49 ` Duy Nguyen
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 16:29 ` Junio C Hamano
0 siblings, 2 replies; 18+ messages in thread
From: Duy Nguyen @ 2016-03-24 10:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Mehul Jain, Git Mailing List
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-24 10:49 ` Duy Nguyen
@ 2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 1/4] git-apply.txt: remove a space Nguyễn Thái Ngọc Duy
` (4 more replies)
2016-03-24 16:29 ` Junio C Hamano
1 sibling, 5 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-03-24 11:56 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, sbeller, mehul.jain2029, sandals,
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/4] git-apply.txt: remove a space
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
@ 2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 2/4] git-apply.txt: mention the behavior inside a subdir Nguyễn Thái Ngọc Duy
` (3 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-03-24 11:56 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, sbeller, mehul.jain2029, sandals,
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] git-apply.txt: mention the behavior inside a subdir
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 1/4] git-apply.txt: remove a space Nguyễn Thái Ngọc Duy
@ 2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 3/4] apply: add --whole to apply git patch without prefix filtering Nguyễn Thái Ngọc Duy
` (2 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-03-24 11:56 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, sbeller, mehul.jain2029, sandals,
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] apply: add --whole to apply git patch without prefix filtering
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 1/4] git-apply.txt: remove a space Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 2/4] git-apply.txt: mention the behavior inside a subdir Nguyễn Thái Ngọc Duy
@ 2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 4/4] apply: report patch skipping in verbose mode Nguyễn Thái Ngọc Duy
2016-03-24 16:50 ` git-apply does not work in a sub-directory of a Git repository Junio C Hamano
4 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-03-24 11:56 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, sbeller, mehul.jain2029, sandals,
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] apply: report patch skipping in verbose mode
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
` (2 preceding siblings ...)
2016-03-24 11:56 ` [PATCH 3/4] apply: add --whole to apply git patch without prefix filtering Nguyễn Thái Ngọc Duy
@ 2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 16:50 ` git-apply does not work in a sub-directory of a Git repository Junio C Hamano
4 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-03-24 11:56 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, sbeller, mehul.jain2029, sandals,
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-24 10:49 ` Duy Nguyen
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
@ 2016-03-24 16:29 ` Junio C Hamano
1 sibling, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2016-03-24 16:29 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Stefan Beller, Mehul Jain, Git Mailing List
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
` (3 preceding siblings ...)
2016-03-24 11:56 ` [PATCH 4/4] apply: report patch skipping in verbose mode Nguyễn Thái Ngọc Duy
@ 2016-03-24 16:50 ` Junio C Hamano
2016-03-24 17:32 ` Junio C Hamano
` (2 more replies)
4 siblings, 3 replies; 18+ messages in thread
From: Junio C Hamano @ 2016-03-24 16:50 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, sbeller, mehul.jain2029, sandals
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-24 16:50 ` git-apply does not work in a sub-directory of a Git repository Junio C Hamano
@ 2016-03-24 17:32 ` Junio C Hamano
2016-03-30 1:05 ` Duy Nguyen
2016-03-30 9:33 ` Duy Nguyen
2 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2016-03-24 17:32 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, sbeller, mehul.jain2029, sandals
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))
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-24 16:50 ` git-apply does not work in a sub-directory of a Git repository Junio C Hamano
2016-03-24 17:32 ` Junio C Hamano
@ 2016-03-30 1:05 ` Duy Nguyen
2016-03-30 17:13 ` Junio C Hamano
2016-03-30 9:33 ` Duy Nguyen
2 siblings, 1 reply; 18+ messages in thread
From: Duy Nguyen @ 2016-03-30 1:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git Mailing List, Stefan Beller, Mehul Jain, brian m. carlson
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-24 16:50 ` git-apply does not work in a sub-directory of a Git repository Junio C Hamano
2016-03-24 17:32 ` Junio C Hamano
2016-03-30 1:05 ` Duy Nguyen
@ 2016-03-30 9:33 ` Duy Nguyen
2 siblings, 0 replies; 18+ messages in thread
From: Duy Nguyen @ 2016-03-30 9:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git Mailing List, Stefan Beller, Mehul Jain, brian m. carlson
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: git-apply does not work in a sub-directory of a Git repository
2016-03-30 1:05 ` Duy Nguyen
@ 2016-03-30 17:13 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2016-03-30 17:13 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Stefan Beller, Mehul Jain, brian m. carlson
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-03-30 17:14 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-22 12:10 git-apply does not work in a sub-directory of a Git repository Mehul Jain
2016-03-22 22:14 ` Stefan Beller
2016-03-23 10:15 ` Duy Nguyen
2016-03-23 15:21 ` Junio C Hamano
2016-03-23 16:55 ` Junio C Hamano
2016-03-24 10:49 ` Duy Nguyen
2016-03-24 11:56 ` Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 1/4] git-apply.txt: remove a space Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 2/4] git-apply.txt: mention the behavior inside a subdir Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 3/4] apply: add --whole to apply git patch without prefix filtering Nguyễn Thái Ngọc Duy
2016-03-24 11:56 ` [PATCH 4/4] apply: report patch skipping in verbose mode Nguyễn Thái Ngọc Duy
2016-03-24 16:50 ` git-apply does not work in a sub-directory of a Git repository Junio C Hamano
2016-03-24 17:32 ` Junio C Hamano
2016-03-30 1:05 ` Duy Nguyen
2016-03-30 17:13 ` Junio C Hamano
2016-03-30 9:33 ` Duy Nguyen
2016-03-24 16:29 ` Junio C Hamano
2016-03-23 17:24 ` Mehul Jain
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.