threads / patch / 34142

v30/2] module_list enhancements

Subject: [[PATCH v3] 0/2] module_list enhancements

## tl;dr

6 messages between Jun 14, 2013 and Jun 14, 2013. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Fredrik Gustafsson· Jun 14, 2013, 15:56 UTC · lore

Reworded commit message for [submodule] handle multibyte characters in name as suggested by Junio.

Previous iteration can be found here: http://thread.gmane.org/gmane.comp.version-control.git/227786/

Fredrik Gustafsson (2):
  [submodule] handle multibyte characters in name
  [submodule] Replace perl-code with sh
 git-submodule.sh           | 53 ++++++++++++++++++++--------------------------
 t/t7400-submodule-basic.sh | 12 +++++++++++
 2 files changed, 35 insertions(+), 30 deletions(-)
-- 
1.8.3.1.381.g2ab719e.dirty
Fredrik Gustafsson· Jun 14, 2013, 15:56 UTC · re: Fredrik Gustafsson · lore

[[PATCH v3] 1/2] [submodule] handle multibyte characters in name

Many "git submodule" operations do not work on a submodule at a path whose name is not in ASCII.

This is because "git ls-files" is used to find which paths are bound to submodules to the current working tree, and the output is C-quoted by default for non ASCII pathnames and pathnames that has a double-quote, a backslash or a control character like a newline or a tab in thme.

Tell "git ls-files" to not C-quote its output, which is easier than unwrapping C-quote ourselves.

This patch still does not allow pathnames with characters that do need C-quote, but the code didn't handle them before, so it is not making things worse. The correct approach to solve the problem for all pathnames may be to use "ls-files -z" and tell the Perl script that reads its output to read NUL separated records by using $/ = "\0".

Solution-suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
---
 git-submodule.sh           |  2 +-
 t/t7400-submodule-basic.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
Show changes to 2 files +13 −1

git-submodule.sh, t/t7400-submodule-basic.sh

diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..bad051e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -113,7 +113,7 @@ resolve_relative_url ()
 module_list()
 {
 	(
-		git ls-files --error-unmatch --stage -- "$@" ||
+		git -c core.quotepath=false ls-files --error-unmatch --stage -- "$@" ||
 		echo "unmatched pathspec exists"
 	) |
 	perl -e '
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index ff26535..d5743ee 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -868,4 +868,16 @@ test_expect_success 'submodule deinit fails when submodule has a .git directory
 	test -n "$(git config --get-regexp "submodule\.example\.")"
 '
 
+test_expect_success 'submodule with strange name works "å äö"' '
+	mkdir "å äö" &&
+	(
+		cd "å äö" &&
+		git init &&
+		touch sub
+		git add sub
+		git commit -m "init sub"
+	)
+	git submodule add "/å äö" &&
+	test -n "$(git submodule | grep "å äö")"
+'
 test_done

-- 
1.8.3.1.381.g2ab719e.dirty
Junio C Hamano· Jun 14, 2013, 17:23 UTC · re: Fredrik Gustafsson · lore

Re: [[PATCH v3] 1/2] [submodule] handle multibyte characters in name

Fredrik Gustafsson <iveqy@iveqy.com> writes:
> ... The
> correct approach to solve the problem for all pathnames may be to use
> "ls-files -z" and tell the Perl script that reads its output to read NUL
> separated records by using $/ = "\0".

I've tentatively queued the attached without 2/2; the scriptlet is small enough not to matter in an eventual rewrite, so it shouldn't make a difference either way.

-- >8 --
From: Fredrik Gustafsson <iveqy@iveqy.com>
Subject: [PATCH] handle multibyte characters in name

Many "git submodule" operations do not work on a submodule at a path whose name is not in ASCII.

This is because "git ls-files" is used to find which paths are bound to submodules to the current working tree, and the output is C-quoted by default for non ASCII pathnames.

Tell "git ls-files" to not C-quote its output, which is easier than unwrapping C-quote ourselves.

Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-submodule.sh           |  3 ++-
 t/t7400-submodule-basic.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
Show changes to 2 files +14 −1

git-submodule.sh, t/t7400-submodule-basic.sh

diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..48bdf84 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -113,7 +113,7 @@ resolve_relative_url ()
 module_list()
 {
 	(
-		git ls-files --error-unmatch --stage -- "$@" ||
+		git ls-files -z --error-unmatch --stage -- "$@" ||
 		echo "unmatched pathspec exists"
 	) |
 	perl -e '
@@ -121,6 +121,7 @@ module_list()
 	my ($null_sha1) = ("0" x 40);
 	my @out = ();
 	my $unmatched = 0;
+	$/ = "\0";
 	while (<STDIN>) {
 		if (/^unmatched pathspec/) {
 			$unmatched = 1;
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index ff26535..d5743ee 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -868,4 +868,16 @@ test_expect_success 'submodule deinit fails when submodule has a .git directory
 	test -n "$(git config --get-regexp "submodule\.example\.")"
 '
 
+test_expect_success 'submodule with strange name works "å äö"' '
+	mkdir "å äö" &&
+	(
+		cd "å äö" &&
+		git init &&
+		touch sub
+		git add sub
+		git commit -m "init sub"
+	)
+	git submodule add "/å äö" &&
+	test -n "$(git submodule | grep "å äö")"
+'
 test_done

-- 
1.8.3.1-538-gb4d04a7
Fredrik Gustafsson· Jun 14, 2013, 18:27 UTC · re: Junio C Hamano · lore

Re: [[PATCH v3] 1/2] [submodule] handle multibyte characters in name

On Fri, Jun 14, 2013 at 10:23:52AM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Fredrik Gustafsson <iveqy@iveqy.com> writes:
> 
> > ... The
> > correct approach to solve the problem for all pathnames may be to use
> > "ls-files -z" and tell the Perl script that reads its output to read NUL
> > separated records by using $/ = "\0".
> 
> I've tentatively queued the attached without 2/2; the scriptlet is
> small enough not to matter in an eventual rewrite, so it shouldn't
> make a difference either way.

Sorry, I didn't knew enough perl to understand that that was a suggestion rather than a hint to a future developer.

Now when I see how you meant it's looks like the best solution to me. To me it looks like we now should be able to handle the multiline case here. However, git submodule add doesn't handle newline yet, so it really doesn't matter for now.

Thanks for the help!
-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com
Junio C Hamano· Jun 14, 2013, 18:33 UTC · re: Fredrik Gustafsson · lore

Re: [[PATCH v3] 1/2] [submodule] handle multibyte characters in name

Fredrik Gustafsson <iveqy@iveqy.com> writes:
Show 14 quoted lines
> On Fri, Jun 14, 2013 at 10:23:52AM -0700, Junio C Hamano wrote:
>> Fredrik Gustafsson <iveqy@iveqy.com> writes:
>> 
>> > ... The
>> > correct approach to solve the problem for all pathnames may be to use
>> > "ls-files -z" and tell the Perl script that reads its output to read NUL
>> > separated records by using $/ = "\0".
>> 
>> I've tentatively queued the attached without 2/2; the scriptlet is
>> small enough not to matter in an eventual rewrite, so it shouldn't
>> make a difference either way.
>
> Sorry, I didn't knew enough perl to understand that that was a
> suggestion rather than a hint to a future developer.

Heh, no need to be sorry. It was a hint, and I just made you a future developer ;-)

Fredrik Gustafsson· Jun 14, 2013, 15:56 UTC · re: Fredrik Gustafsson · lore

[[PATCH v3] 2/2] [submodule] Replace perl-code with sh

This will prevent a fork and makes the code similair to the rest of the file.

In the long term git-submodule.sh needs to use something else than sh to handle newline in filenames (and therefore needs to use a language that accepts \0 in strings). However I don't think that keeping that small perl-part will ease any rewrite.

Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
---
 git-submodule.sh | 51 ++++++++++++++++++++++-----------------------------
 1 file changed, 22 insertions(+), 29 deletions(-)
Show changes to git-submodule.sh +22 −29
diff --git a/git-submodule.sh b/git-submodule.sh
index bad051e..be96934 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -112,38 +112,31 @@ resolve_relative_url ()
 #
 module_list()
 {
+	null_sha1=0000000000000000000000000000000000000000
+	unmerged=
 	(
 		git -c core.quotepath=false ls-files --error-unmatch --stage -- "$@" ||
-		echo "unmatched pathspec exists"
+		echo "#unmatched"
 	) |
-	perl -e '
-	my %unmerged = ();
-	my ($null_sha1) = ("0" x 40);
-	my @out = ();
-	my $unmatched = 0;
-	while (<STDIN>) {
-		if (/^unmatched pathspec/) {
-			$unmatched = 1;
-			next;
-		}
-		chomp;
-		my ($mode, $sha1, $stage, $path) =
-			/^([0-7]+) ([0-9a-f]{40}) ([0-3])\t(.*)$/;
-		next unless $mode eq "160000";
-		if ($stage ne "0") {
-			if (!$unmerged{$path}++) {
-				push @out, "$mode $null_sha1 U\t$path\n";
-			}
-			next;
-		}
-		push @out, "$_\n";
-	}
-	if ($unmatched) {
-		print "#unmatched\n";
-	} else {
-		print for (@out);
-	}
-	'
+	while read mode sha1 stage path
+	do
+		if test $mode = "#unmatched"
+		then
+			echo "#unmatched"
+		elif test $mode = "160000"
+		then
+			if test $stage != "0"
+			then
+				if test "$unmerged" != "$path"
+				then
+					echo "$mode $null_sha1 U $path"
+				fi
+				unmerged="$path"
+			else
+				echo "$mode $sha1 $stage $path"
+			fi
+		fi
+	done
 }
 
 die_if_unmatched ()

-- 
1.8.3.1.381.g2ab719e.dirty

← back to recent threads