Re: git-apply does not work in a sub-directory of a Git repository
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
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
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 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
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
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
+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
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
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 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
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
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
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
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