Re: [PATCH] Turn off pathspec magic on {checkout,reset,add} -p on native Windows builds

2013-09-02 Thread Johannes Sixt
Am 9/1/2013 4:08, schrieb Nguyễn Thái Ngọc Duy:
 git-add--interactive.perl rejects arguments with colons in 21e9757
 (Hack git-add--interactive to make it work with ActiveState Perl -
 2007-08-01). Pathspec magic starts with a colon, so it won't work if
 these pathspecs are passed to git-add--interactive.perl running with
 ActiveState Perl. Make sure we only pass plain paths in this case.
 
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Johannes, can you check the test suite passes for you with this
  patch? I assume that Cygwin Perl behaves differently and does not hit
  this limit. So I keep the special case to GIT_WINDOWS_NATIVE only.
  I'll resend the patch with a few others on the same topic if it works
  for you.

It does not help. The error in git-add--interactive is avoided, but the
failure in t2016-checkout-patch.sh is now:

expecting success:
set_state dir/foo work head 
# the third n is to get out in case it mistakenly does not apply
(echo y; echo n; echo n) | (cd dir  git checkout -p foo) 
verify_saved_state bar 
verify_state dir/foo head head

No changes.
not ok 13 - path limiting works: foo inside dir

and the same No changes. happens in t7105-reset-patch.sh

 +#ifdef GIT_WINDOWS_NATIVE
 + /*
 +  * Pathspec magic is completely turned off on native Windows
 +  * builds because git-add-interactive.perl won't accept
 +  * arguments with colons in them. :/foo is an exception
 +  * because no colons remain after parsing.
 +  */
 + parse_pathspec(pathspec, PATHSPEC_ALL_MAGIC  ~PATHSPEC_FROMTOP,
 +PATHSPEC_PREFER_FULL |
 +PATHSPEC_SYMLINK_LEADING_PATH,
 +prefix, argv);
 +#else
   /*
* git-add--interactive itself does not parse pathspec. It
* simply passes the pathspec to other builtin commands. Let's
 @@ -281,6 +293,7 @@ int interactive_add(int argc, const char **argv, const 
 char *prefix, int patch)
  PATHSPEC_SYMLINK_LEADING_PATH |
  PATHSPEC_PREFIX_ORIGIN,
  prefix, argv);
 +#endif
 [etc.]

-- Hannes
--
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: [PATCH] Turn off pathspec magic on {checkout,reset,add} -p on native Windows builds

2013-09-02 Thread Duy Nguyen
On Mon, Sep 02, 2013 at 08:42:18AM +0200, Johannes Sixt wrote:
 Am 9/1/2013 4:08, schrieb Nguyễn Thái Ngọc Duy:
  git-add--interactive.perl rejects arguments with colons in 21e9757
  (Hack git-add--interactive to make it work with ActiveState Perl -
  2007-08-01). Pathspec magic starts with a colon, so it won't work if
  these pathspecs are passed to git-add--interactive.perl running with
  ActiveState Perl. Make sure we only pass plain paths in this case.
  
  Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
  ---
   Johannes, can you check the test suite passes for you with this
   patch? I assume that Cygwin Perl behaves differently and does not hit
   this limit. So I keep the special case to GIT_WINDOWS_NATIVE only.
   I'll resend the patch with a few others on the same topic if it works
   for you.
 
 It does not help. The error in git-add--interactive is avoided, but the
 failure in t2016-checkout-patch.sh is now:
 
 expecting success:
 set_state dir/foo work head 
 # the third n is to get out in case it mistakenly does not apply
 (echo y; echo n; echo n) | (cd dir  git checkout -p foo) 
 verify_saved_state bar 
 verify_state dir/foo head head
 
 No changes.
 not ok 13 - path limiting works: foo inside dir
 
 and the same No changes. happens in t7105-reset-patch.sh

Right. Because I got rid of ':(prefix)foo' form but I passed 'foo'
instead of 'dir/foo'. How about this on top?

-- 8 --
diff --git a/builtin/add.c b/builtin/add.c
index 3402239..a138360 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -257,9 +257,15 @@ int run_add_interactive(const char *revision, const char 
*patch_mode,
if (revision)
args[ac++] = revision;
args[ac++] = --;
+#ifdef GIT_WINDOWS_NATIVE
+   GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP);
+   for (i = 0; i  pathspec-nr; i++)
+   args[ac++] = pathspec-items[i].match;
+#else
for (i = 0; i  pathspec-nr; i++)
/* pass original pathspec, to be re-parsed */
args[ac++] = pathspec-items[i].original;
+#endif
 
status = run_command_v_opt(args, RUN_GIT_CMD);
free(args);
-- 8 --
--
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: [PATCH] Turn off pathspec magic on {checkout,reset,add} -p on native Windows builds

2013-09-02 Thread Johannes Sixt
Am 9/2/2013 11:30, schrieb Duy Nguyen:
 On Mon, Sep 02, 2013 at 08:42:18AM +0200, Johannes Sixt wrote:
 Am 9/1/2013 4:08, schrieb Nguyễn Thái Ngọc Duy:
 git-add--interactive.perl rejects arguments with colons in 21e9757
 (Hack git-add--interactive to make it work with ActiveState Perl -
 2007-08-01). Pathspec magic starts with a colon, so it won't work if
 these pathspecs are passed to git-add--interactive.perl running with
 ActiveState Perl. Make sure we only pass plain paths in this case.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Johannes, can you check the test suite passes for you with this
  patch? I assume that Cygwin Perl behaves differently and does not hit
  this limit. So I keep the special case to GIT_WINDOWS_NATIVE only.
  I'll resend the patch with a few others on the same topic if it works
  for you.

 It does not help. The error in git-add--interactive is avoided, but the
 failure in t2016-checkout-patch.sh is now:

 expecting success:
 set_state dir/foo work head 
 # the third n is to get out in case it mistakenly does not apply
 (echo y; echo n; echo n) | (cd dir  git checkout -p foo) 
 verify_saved_state bar 
 verify_state dir/foo head head

 No changes.
 not ok 13 - path limiting works: foo inside dir

 and the same No changes. happens in t7105-reset-patch.sh
 
 Right. Because I got rid of ':(prefix)foo' form but I passed 'foo'
 instead of 'dir/foo'. How about this on top?
 
 -- 8 --
 diff --git a/builtin/add.c b/builtin/add.c
 index 3402239..a138360 100644
 --- a/builtin/add.c
 +++ b/builtin/add.c
 @@ -257,9 +257,15 @@ int run_add_interactive(const char *revision, const char 
 *patch_mode,
   if (revision)
   args[ac++] = revision;
   args[ac++] = --;
 +#ifdef GIT_WINDOWS_NATIVE
 + GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP);
 + for (i = 0; i  pathspec-nr; i++)
 + args[ac++] = pathspec-items[i].match;
 +#else
   for (i = 0; i  pathspec-nr; i++)
   /* pass original pathspec, to be re-parsed */
   args[ac++] = pathspec-items[i].original;
 +#endif
  
   status = run_command_v_opt(args, RUN_GIT_CMD);
   free(args);
 -- 8 --

With this patch, the two tests pass.

Which features do we lose on Windows with the previous patch and this fixup?

-- Hannes
--
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: [PATCH] Turn off pathspec magic on {checkout,reset,add} -p on native Windows builds

2013-09-02 Thread Duy Nguyen
On Mon, Sep 2, 2013 at 5:41 PM, Johannes Sixt j.s...@viscovery.net wrote:
 Which features do we lose on Windows with the previous patch and this fixup?

New pathspec magic :(glob), :(literal) and :(icase). You can still use
them via --*-pathspecs or equivalent env variables. You just can't
enable them per individual pathspec.
-- 
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