[PATCH 1/3] pathspec: catch prepending :(prefix) on pathspec with short magic

Subsystems: the rest

DORMANTno replies

3 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH 1/3] pathspec: catch prepending :(prefix) on pathspec with short magic

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:58:38

:(prefix) is in the long form. Suppose people pass :!foo with '!'
being the short form of magic 'bar', the code will happily turn it to
:(prefix..)!foo, which makes '!' part of the path and no longer a magic.

The correct form must be ':(prefix..,bar)foo', but as so far we
haven't had any magic in short form yet (*), the code to convert from
short form to long one will be inactive anyway. Let's postpone it
until a real short form magic appears.

(*) The short form magic '/' is a special case and won't be caught by
this die(), which is correct. When '/' magic is detected, prefixlen is
set back to 0 and the whole "if (prefixlen..)" block is skipped.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 fixes on top of nd/magic-pathspec.

 pathspec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pathspec.c b/pathspec.c
index d9f4143..62fde50 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -231,7 +231,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
 		const char *start = elt;
 		if (prefixlen && !literal_global) {
 			/* Preserve the actual prefix length of each pattern */
-			if (long_magic_end) {
+			if (short_magic)
+				die("BUG: prefixing on short magic is not supported");
+			else if (long_magic_end) {
 				strbuf_add(&sb, start, long_magic_end - start);
 				strbuf_addf(&sb, ",prefix:%d", prefixlen);
 				start = long_magic_end;
-- 
1.8.2.83.gc99314b

[PATCH 2/3] add: lift the pathspec magic restriction on "add -p"

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:58:38

Since 480ca64 (convert run_add_interactive to use struct pathspec -
2013-07-14), we have unconditionally passed :(prefix)xxx to
add-interactive.perl. It implies that all commands
add-interactive.perl calls must be aware of pathspec magic, or
:(prefix) is barfed. The restriction to :/ only becomes unnecessary.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/add.c      | 8 +-------
 builtin/checkout.c | 7 -------
 2 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 9d52fc7..b4035f6 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -270,13 +270,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch)
 {
 	struct pathspec pathspec;
 
-	/*
-	 * git-add--interactive itself does not parse pathspec. It
-	 * simply passes the pathspec to other builtin commands. Let's
-	 * hope all of them support all magic, or we'll need to limit
-	 * the magic here.
-	 */
-	parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,
+	parse_pathspec(&pathspec, 0,
 		       PATHSPEC_PREFER_FULL |
 		       PATHSPEC_SYMLINK_LEADING_PATH |
 		       PATHSPEC_PREFIX_ORIGIN,
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7ea1100..b235e04 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1149,13 +1149,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	}
 
 	if (argc) {
-		/*
-		 * In patch mode (opts.patch_mode != 0), we pass the
-		 * pathspec to an external program, git-add--interactive.
-		 * Do not accept any kind of magic that that program
-		 * cannot handle. Magic mask is pretty safe to be
-		 * lifted for new magic when opts.patch_mode == 0.
-		 */
 		parse_pathspec(&opts.pathspec, 0,
 			       opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
 			       prefix, argv);
-- 
1.8.2.83.gc99314b

[PATCH 3/3] add--interactive: fix external command invocation on Windows

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:58:38

From: Johannes Sixt <redacted>

Back in 21e9757e (Hack git-add--interactive to make it work with
ActiveState Perl, 2007-08-01), the invocation of external commands was
changed to use qx{} on Windows. The rationale was that the command
interpreter on Windows is not a POSIX shell, but rather Windows's CMD.
That patch was wrong to include 'msys' in the check whether to use qx{}
or not: 'msys' identifies MSYS perl as shipped with Git for Windows,
which does not need the special treatment; qx{} should be used only with
ActiveState perl, which is identified by 'MSWin32'.

Signed-off-by: Johannes Sixt <redacted>
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 git-add--interactive.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 75a991f..5156384 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -169,7 +169,7 @@ my %patch_modes = (
 my %patch_mode_flavour = %{$patch_modes{stage}};
 
 sub run_cmd_pipe {
-	if ($^O eq 'MSWin32' || $^O eq 'msys') {
+	if ($^O eq 'MSWin32') {
 		my @invalid = grep {m/[":*]/} @_;
 		die "$^O does not support: @invalid\n" if @invalid;
 		my @args = map { m/ /o ? "\"$_\"": $_ } @_;
-- 
1.8.2.83.gc99314b
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help