# "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" 6 messages from 2016-11-30 to 2016-12-06. Participants: Peter Urda, Kevin Daudt, Junio C Hamano, Brandon Williams. Thread: https://gitlist.dev/t/44573 ## Peter Urda, 2016-11-30 20:31 Subject: "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" Message-ID: URL: https://gitlist.dev/e/CAEnOLdvG%3DSoKFxeJ_pLmamGj_8osC%2B28TSg%2BpbFLLTr%2BZLcpQA%40mail.gmail.com ``` After upgrading to version 2.11.0 I am getting a warning about empty strings as pathspecs while using 'patch' - Ran 'git add -p .' from the root of my git repository. - I was able to normally stage my changes, but was presented with a "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" message. - I expected no warning message since I included a "." with my original command. I believe that I should not be seeing this warning message as I included the requested "." pathspec. ~ Peter Urda http://urda.cc ``` ## Kevin Daudt, 2016-11-30 21:11 Subject: Re: "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" Message-ID: <20161130211100.GA18680@ikke.info> URL: https://gitlist.dev/e/20161130211100.GA18680%40ikke.info In-Reply-To: ``` On Wed, Nov 30, 2016 at 12:31:49PM -0800, Peter Urda wrote: > After upgrading to version 2.11.0 I am getting a warning about empty > strings as pathspecs while using 'patch' > > - Ran 'git add -p .' from the root of my git repository. > > - I was able to normally stage my changes, but was presented with a > "warning: empty strings as pathspecs will be made invalid in upcoming > releases. please use . instead if you meant to match all paths" > message. > > - I expected no warning message since I included a "." with my original command. > > I believe that I should not be seeing this warning message as I > included the requested "." pathspec. > > ~ Peter Urda > > http://urda.cc I can reproduce this. Note that it only happens when you specify '-p'. Without the --patch option, the warning does not appear. ``` ## Junio C Hamano, 2016-11-30 22:04 Subject: Re: "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" Message-ID: URL: https://gitlist.dev/e/xmqq7f7kd3pj.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20161130211100.GA18680@ikke.info> ``` Kevin Daudt writes: > On Wed, Nov 30, 2016 at 12:31:49PM -0800, Peter Urda wrote: >> After upgrading to version 2.11.0 I am getting a warning about empty >> strings as pathspecs while using 'patch' >> >> - Ran 'git add -p .' from the root of my git repository. >> >> - I was able to normally stage my changes, but was presented with a >> "warning: empty strings as pathspecs will be made invalid in upcoming >> releases. please use . instead if you meant to match all paths" >> message. >> >> - I expected no warning message since I included a "." with my original command. >> >> I believe that I should not be seeing this warning message as I >> included the requested "." pathspec. Yes, this seems to be caused by pathspec.c::prefix_pathspec() overwriting the original pathspec "." into "". The callchain looks like this: builtin/add.c::interactive_add() -> parse_pathspec() passes argv[] that has "." to the caller, receives pathspec whose pathspec->items[].original is supposed to point at the unmolested original, but prefix_pathspec() munges "." into "" -> run_add_interactive() which runs "git add--interactive" with pathspec->items[].original as pathspecs Perhaps this would work it around, but there should be a better way to fix it (like, making sure that what we call "original" indeed stays "original"). builtin/add.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index e8fb80b36e..137097192d 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -167,9 +167,18 @@ int run_add_interactive(const char *revision, const char *patch_mode, if (revision) argv_array_push(&argv, revision); argv_array_push(&argv, "--"); - for (i = 0; i < pathspec->nr; i++) + for (i = 0; i < pathspec->nr; i++) { /* pass original pathspec, to be re-parsed */ + if (!*pathspec->items[i].original) { + /* + * work around a misfeature in parse_pathspecs() + * that munges "." into "". + */ + argv_array_push(&argv, "."); + continue; + } argv_array_push(&argv, pathspec->items[i].original); + } status = run_command_v_opt(argv.argv, RUN_GIT_CMD); argv_array_clear(&argv); @@ -180,7 +189,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch) { struct pathspec pathspec; - parse_pathspec(&pathspec, 0, + parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_FULL | PATHSPEC_SYMLINK_LEADING_PATH | PATHSPEC_PREFIX_ORIGIN, ``` ## Junio C Hamano, 2016-11-30 22:40 Subject: Re: "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" Message-ID: URL: https://gitlist.dev/e/xmqqtwaobni5.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Junio C Hamano forgot to Cc: the author of the most relevant change to the issue, d426430e6e ("pathspec: warn on empty strings as pathspec", 2016-06-22). > Kevin Daudt writes: > >> On Wed, Nov 30, 2016 at 12:31:49PM -0800, Peter Urda wrote: >>> After upgrading to version 2.11.0 I am getting a warning about empty >>> strings as pathspecs while using 'patch' >>> >>> - Ran 'git add -p .' from the root of my git repository. >>> >>> - I was able to normally stage my changes, but was presented with a >>> "warning: empty strings as pathspecs will be made invalid in upcoming >>> releases. please use . instead if you meant to match all paths" >>> message. >>> >>> - I expected no warning message since I included a "." with my original command. >>> >>> I believe that I should not be seeing this warning message as I >>> included the requested "." pathspec. > > Yes, this seems to be caused by pathspec.c::prefix_pathspec() > overwriting the original pathspec "." into "". The callchain > looks like this: > > builtin/add.c::interactive_add() > -> parse_pathspec() > passes argv[] that has "." to the caller, > receives pathspec whose pathspec->items[].original > is supposed to point at the unmolested original, > but prefix_pathspec() munges "." into "" > -> run_add_interactive() > which runs "git add--interactive" with > pathspec->items[].original as pathspecs > > > Perhaps this would work it around, but there should be a better way > to fix it (like, making sure that what we call "original" indeed > stays "original"). > > builtin/add.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/builtin/add.c b/builtin/add.c > index e8fb80b36e..137097192d 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -167,9 +167,18 @@ int run_add_interactive(const char *revision, const char *patch_mode, > if (revision) > argv_array_push(&argv, revision); > argv_array_push(&argv, "--"); > - for (i = 0; i < pathspec->nr; i++) > + for (i = 0; i < pathspec->nr; i++) { > /* pass original pathspec, to be re-parsed */ > + if (!*pathspec->items[i].original) { > + /* > + * work around a misfeature in parse_pathspecs() > + * that munges "." into "". > + */ > + argv_array_push(&argv, "."); > + continue; > + } > argv_array_push(&argv, pathspec->items[i].original); > + } > > status = run_command_v_opt(argv.argv, RUN_GIT_CMD); > argv_array_clear(&argv); > @@ -180,7 +189,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch) > { > struct pathspec pathspec; > > - parse_pathspec(&pathspec, 0, > + parse_pathspec(&pathspec, 0, > PATHSPEC_PREFER_FULL | > PATHSPEC_SYMLINK_LEADING_PATH | > PATHSPEC_PREFIX_ORIGIN, ``` ## Brandon Williams, 2016-12-06 18:04 Subject: Re: "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" Message-ID: <20161206180400.GA103573@google.com> URL: https://gitlist.dev/e/20161206180400.GA103573%40google.com In-Reply-To: ``` On 11/30, Junio C Hamano wrote: > Junio C Hamano forgot to Cc: the author of the > most relevant change to the issue, d426430e6e ("pathspec: warn on > empty strings as pathspec", 2016-06-22). > > > Kevin Daudt writes: > > > >> On Wed, Nov 30, 2016 at 12:31:49PM -0800, Peter Urda wrote: > >>> After upgrading to version 2.11.0 I am getting a warning about empty > >>> strings as pathspecs while using 'patch' > >>> > >>> - Ran 'git add -p .' from the root of my git repository. > >>> > >>> - I was able to normally stage my changes, but was presented with a > >>> "warning: empty strings as pathspecs will be made invalid in upcoming > >>> releases. please use . instead if you meant to match all paths" > >>> message. > >>> > >>> - I expected no warning message since I included a "." with my original command. > >>> > >>> I believe that I should not be seeing this warning message as I > >>> included the requested "." pathspec. > > > > Yes, this seems to be caused by pathspec.c::prefix_pathspec() > > overwriting the original pathspec "." into "". The callchain > > looks like this: > > > > builtin/add.c::interactive_add() > > -> parse_pathspec() > > passes argv[] that has "." to the caller, > > receives pathspec whose pathspec->items[].original > > is supposed to point at the unmolested original, > > but prefix_pathspec() munges "." into "" > > -> run_add_interactive() > > which runs "git add--interactive" with > > pathspec->items[].original as pathspecs > > > > > > Perhaps this would work it around, but there should be a better way > > to fix it (like, making sure that what we call "original" indeed > > stays "original"). > > > > builtin/add.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/builtin/add.c b/builtin/add.c > > index e8fb80b36e..137097192d 100644 > > --- a/builtin/add.c > > +++ b/builtin/add.c > > @@ -167,9 +167,18 @@ int run_add_interactive(const char *revision, const char *patch_mode, > > if (revision) > > argv_array_push(&argv, revision); > > argv_array_push(&argv, "--"); > > - for (i = 0; i < pathspec->nr; i++) > > + for (i = 0; i < pathspec->nr; i++) { > > /* pass original pathspec, to be re-parsed */ > > + if (!*pathspec->items[i].original) { > > + /* > > + * work around a misfeature in parse_pathspecs() > > + * that munges "." into "". > > + */ > > + argv_array_push(&argv, "."); > > + continue; > > + } > > argv_array_push(&argv, pathspec->items[i].original); > > + } > > > > status = run_command_v_opt(argv.argv, RUN_GIT_CMD); > > argv_array_clear(&argv); > > @@ -180,7 +189,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch) > > { > > struct pathspec pathspec; > > > > - parse_pathspec(&pathspec, 0, > > + parse_pathspec(&pathspec, 0, > > PATHSPEC_PREFER_FULL | > > PATHSPEC_SYMLINK_LEADING_PATH | > > PATHSPEC_PREFIX_ORIGIN, I've been doing a bit of work trying to clean up the pathspec initialization code and I believe this can be fixed without having to add in this work around. The code which does the munging is always trying to prefix the pathspec regardless if there is a prefix or not. If instead its changed to only try and prefix the original if there is indeed a prefix, then it should fix the munging. I'll try to get the series I'm working on out in the next day. -- Brandon Williams ``` ## Junio C Hamano, 2016-12-06 21:21 Subject: Re: "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths" Message-ID: URL: https://gitlist.dev/e/xmqqlgvszrco.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20161206180400.GA103573@google.com> ``` Brandon Williams writes: > On 11/30, Junio C Hamano wrote: >> Junio C Hamano forgot to Cc: the author of the >> most relevant change to the issue, d426430e6e ("pathspec: warn on >> empty strings as pathspec", 2016-06-22). >> ... > > I've been doing a bit of work trying to clean up the pathspec > initialization code and I believe this can be fixed without > having to add in this work around. The code which does the munging is > always trying to prefix the pathspec regardless if there is a prefix or > not. If instead its changed to only try and prefix the original if > there is indeed a prefix, then it should fix the munging. Thanks; sounds like a plan. ```