All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] enable "svn.pathnameencoding" on dcommit
@ 2016-02-08 15:19 Kazutoshi Satoda
  2016-02-08 15:20 ` [PATCH 1/2] git-svn: " Kazutoshi Satoda
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-02-08 15:19 UTC (permalink / raw)
  To: git; +Cc: normalperson, alex.crezoff

These are small fixes to problems I encountered using git-svn with
svn.pathnameencoding configuration (cp932 in my case). The problems
happen only when sending changes on non-ASCII paths.

I'm sorry not coming with test scripts, but I couldn't figure out how to
write tests to reproduce problems happen only with non-UTF-8 paths while
the tests seems to be run on UTF-8 locale.

I just ran existing tests with these fixes and there was no breakage
(with UTF-8 locale).

I think sending these fixes as-is is better than not sending because of
my lack of ability to write such tests. I also expect that these fixes
can be applied as-is because there were some small fixes without tests
in the history of file perl/Git/SVN/Editor.pm.

The first one is an updated fix of an very old but not applied patch.
http://comments.gmane.org/gmane.comp.version-control.git/164166#o2

The second one can be seen as a separate fix, but it takes effect only
after the first one.

Kazutoshi SATODA (2):
  git-svn: enable "svn.pathnameencoding" on dcommit
  git-svn: apply "svn.pathnameencoding" before URL encoding

 perl/Git/SVN/Editor.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.7.0

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] git-svn: enable "svn.pathnameencoding" on dcommit
  2016-02-08 15:19 [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Kazutoshi Satoda
@ 2016-02-08 15:20 ` Kazutoshi Satoda
  2016-02-15  0:30   ` Eric Wong
  2016-02-08 15:21 ` [PATCH 2/2] git-svn: apply "svn.pathnameencoding" before URL encoding Kazutoshi Satoda
  2016-02-08 22:58 ` [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Eric Wong
  2 siblings, 1 reply; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-02-08 15:20 UTC (permalink / raw)
  To: git; +Cc: normalperson, alex.crezoff

Without the initialization of $self->{pathnameencoding}, conversion in
repo_path() is always skipped as $self->{pathnameencoding} is undefined
even if "svn.pathnameencoding" is configured.

The lack of conversion results in mysterious failure of dcommit (e.g.
"Malformed XML") which happen only when a commit involves a change on
non-ASCII path.

Signed-off-by: Kazutoshi SATODA <k_satoda@f2.dion.ne.jp>
---
 perl/Git/SVN/Editor.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index c50176e..d9d9bdf 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -41,6 +41,7 @@ sub new {
 	                       "$self->{svn_path}/" : '';
 	$self->{config} = $opts->{config};
 	$self->{mergeinfo} = $opts->{mergeinfo};
+	$self->{pathnameencoding} = Git::config('svn.pathnameencoding');
 	return $self;
 }
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/2] git-svn: apply "svn.pathnameencoding" before URL encoding
  2016-02-08 15:19 [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Kazutoshi Satoda
  2016-02-08 15:20 ` [PATCH 1/2] git-svn: " Kazutoshi Satoda
@ 2016-02-08 15:21 ` Kazutoshi Satoda
  2016-02-15  0:33   ` Eric Wong
  2016-02-08 22:58 ` [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Eric Wong
  2 siblings, 1 reply; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-02-08 15:21 UTC (permalink / raw)
  To: git; +Cc: normalperson, alex.crezoff

The conversion from "svn.pathnameencoding" to UTF-8 should be applied
first, and then URL encoding should be applied on the resulting UTF-8
path. The reversed order of these transforms (used before this fix)
makes non-UTF-8 URL which causes error from Subversion such as
"Filesystem has no item: '...' path not found" when sending a rename (or
a copy) from non-ASCII path.

Signed-off-by: Kazutoshi SATODA <k_satoda@f2.dion.ne.jp>
---
 perl/Git/SVN/Editor.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index d9d9bdf..4c4199a 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -144,11 +144,12 @@ sub repo_path {
 
 sub url_path {
 	my ($self, $path) = @_;
+	$path = $self->repo_path($path);
 	if ($self->{url} =~ m#^https?://#) {
 		# characters are taken from subversion/libsvn_subr/path.c
 		$path =~ s#([^~a-zA-Z0-9_./!$&'()*+,-])#sprintf("%%%02X",ord($1))#eg;
 	}
-	$self->{url} . '/' . $self->repo_path($path);
+	$self->{url} . '/' . $path;
 }
 
 sub rmdirs {
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 0/2] enable "svn.pathnameencoding" on dcommit
  2016-02-08 15:19 [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Kazutoshi Satoda
  2016-02-08 15:20 ` [PATCH 1/2] git-svn: " Kazutoshi Satoda
  2016-02-08 15:21 ` [PATCH 2/2] git-svn: apply "svn.pathnameencoding" before URL encoding Kazutoshi Satoda
@ 2016-02-08 22:58 ` Eric Wong
  2016-02-15  0:52   ` [PULL] svn pathnameencoding for git svn dcommit Eric Wong
  2016-02-27 18:28   ` [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS tboegi
  2 siblings, 2 replies; 25+ messages in thread
From: Eric Wong @ 2016-02-08 22:58 UTC (permalink / raw)
  To: Kazutoshi Satoda; +Cc: git, alex.crezoff

Kazutoshi Satoda <k_satoda@f2.dion.ne.jp> wrote:
> I'm sorry not coming with test scripts, but I couldn't figure out how to
> write tests to reproduce problems happen only with non-UTF-8 paths while
> the tests seems to be run on UTF-8 locale.

Thank you.  I will try to work on some tests throughout the week
(help appreciated!) but will be willing to push these up to Junio
regardless.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] git-svn: enable "svn.pathnameencoding" on dcommit
  2016-02-08 15:20 ` [PATCH 1/2] git-svn: " Kazutoshi Satoda
@ 2016-02-15  0:30   ` Eric Wong
  0 siblings, 0 replies; 25+ messages in thread
From: Eric Wong @ 2016-02-15  0:30 UTC (permalink / raw)
  To: Kazutoshi Satoda; +Cc: git, alex.crezoff

Kazutoshi Satoda <k_satoda@f2.dion.ne.jp> wrote:
> Signed-off-by: Kazutoshi SATODA <k_satoda@f2.dion.ne.jp>

Signed-off-by: Eric Wong <normalperson@yhbt.net>

Thanks, it took me some time to figure out the test
case which I will amend:

diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
index 6a48e40..82222fd 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -77,11 +77,21 @@ test_expect_success 'make a commit to test rebase' '
 	'
 
 test_expect_success 'git svn rebase works inside a fresh-cloned repository' '
-	cd test-rebase &&
+	(
+		cd test-rebase &&
 		git svn rebase &&
 		test -e test-rebase-main &&
 		test -e test-rebase
-	'
+	)'
+
+test_expect_success 'svn.pathnameencoding=cp932 new file on dcommit' '
+	neq=$(printf "\201\202") &&
+	git config svn.pathnameencoding cp932 &&
+	echo neq >"$neq" &&
+	git add "$neq" &&
+	git commit -m "neq" &&
+	git svn dcommit
+'
 
 stop_httpd
 
-- 
EW

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] git-svn: apply "svn.pathnameencoding" before URL encoding
  2016-02-08 15:21 ` [PATCH 2/2] git-svn: apply "svn.pathnameencoding" before URL encoding Kazutoshi Satoda
@ 2016-02-15  0:33   ` Eric Wong
  0 siblings, 0 replies; 25+ messages in thread
From: Eric Wong @ 2016-02-15  0:33 UTC (permalink / raw)
  To: Kazutoshi Satoda; +Cc: git, alex.crezoff

Kazutoshi Satoda <k_satoda@f2.dion.ne.jp> wrote:
> The conversion from "svn.pathnameencoding" to UTF-8 should be applied
> first, and then URL encoding should be applied on the resulting UTF-8
> path. The reversed order of these transforms (used before this fix)
> makes non-UTF-8 URL which causes error from Subversion such as
> "Filesystem has no item: '...' path not found" when sending a rename (or
> a copy) from non-ASCII path.
> 
> Signed-off-by: Kazutoshi SATODA <k_satoda@f2.dion.ne.jp>

Thanks, running full SVN tests now.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

> --- a/perl/Git/SVN/Editor.pm
> +++ b/perl/Git/SVN/Editor.pm
> @@ -144,11 +144,12 @@ sub repo_path {
>  
>  sub url_path {
>  	my ($self, $path) = @_;
> +	$path = $self->repo_path($path);
>  	if ($self->{url} =~ m#^https?://#) {
>  		# characters are taken from subversion/libsvn_subr/path.c
>  		$path =~ s#([^~a-zA-Z0-9_./!$&'()*+,-])#sprintf("%%%02X",ord($1))#eg;
>  	}
> -	$self->{url} . '/' . $self->repo_path($path);
> +	$self->{url} . '/' . $path;
>  }

This is trickier to test, as it requires an https?:// URL.
It always succeeds with the default file:// URL.

diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
index 82222fd..9828f05 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -93,6 +93,18 @@ test_expect_success 'svn.pathnameencoding=cp932 new file on dcommit' '
 	git svn dcommit
 '
 
+test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
+	inf=$(printf "\201\207") &&
+	git config svn.pathnameencoding cp932 &&
+	echo inf >"$inf" &&
+	git add "$inf" &&
+	git commit -m "inf" &&
+	git svn dcommit &&
+	git mv "$inf" inf &&
+	git commit -m "inf rename" &&
+	git svn dcommit
+'
+
 stop_httpd
 
 test_done
-- 
EW

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PULL] svn pathnameencoding for git svn dcommit
  2016-02-08 22:58 ` [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Eric Wong
@ 2016-02-15  0:52   ` Eric Wong
  2016-02-15 21:32     ` Junio C Hamano
  2016-02-16  3:29     ` Kazutoshi Satoda
  2016-02-27 18:28   ` [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS tboegi
  1 sibling, 2 replies; 25+ messages in thread
From: Eric Wong @ 2016-02-15  0:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, alex.crezoff, Kazutoshi Satoda

I've amended tests to both commits, but the URL encoding one
requires an HTTP server to test effectively.

I couldn't find a test prereq for httpd, but perhaps it's good
to test by default regardless in case a future SVN changes
file:// behavior.  I've only tested this with SVN 1.6.17 under
Debian wheezy.

The following changes since commit 6faf27b4ff26804a07363078b238b5cfd3dfa976:

  Merge branch 'tb/conversion' into next (2016-02-12 10:20:20 -0800)

are available in the git repository at:

  git://bogomips.org/git-svn.git ks/svn-pathnameencoding

for you to fetch changes up to dfee0cf8123e7f63268f05a02731ce82db136188:

  git-svn: apply "svn.pathnameencoding" before URL encoding (2016-02-15 00:31:21 +0000)

----------------------------------------------------------------
Kazutoshi Satoda (2):
      git-svn: enable "svn.pathnameencoding" on dcommit
      git-svn: apply "svn.pathnameencoding" before URL encoding

 perl/Git/SVN/Editor.pm                   |  4 +++-
 t/t9115-git-svn-dcommit-funky-renames.sh | 26 ++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PULL] svn pathnameencoding for git svn dcommit
  2016-02-15  0:52   ` [PULL] svn pathnameencoding for git svn dcommit Eric Wong
@ 2016-02-15 21:32     ` Junio C Hamano
  2016-02-16  3:29     ` Kazutoshi Satoda
  1 sibling, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2016-02-15 21:32 UTC (permalink / raw)
  To: Eric Wong; +Cc: git, alex.crezoff, Kazutoshi Satoda

Eric Wong <normalperson@yhbt.net> writes:

> I've amended tests to both commits, but the URL encoding one
> requires an HTTP server to test effectively.
>
> I couldn't find a test prereq for httpd, but perhaps it's good
> to test by default regardless in case a future SVN changes
> file:// behavior.  I've only tested this with SVN 1.6.17 under
> Debian wheezy.
>
> The following changes since commit 6faf27b4ff26804a07363078b238b5cfd3dfa976:
>
>   Merge branch 'tb/conversion' into next (2016-02-12 10:20:20 -0800)
>
> are available in the git repository at:
>
>   git://bogomips.org/git-svn.git ks/svn-pathnameencoding
>
> for you to fetch changes up to dfee0cf8123e7f63268f05a02731ce82db136188:
>
>   git-svn: apply "svn.pathnameencoding" before URL encoding (2016-02-15 00:31:21 +0000)

Sorry, Eric, I cannot pull this.  You made your branch on top of
'next', there are many commits that are not ready.

>
> ----------------------------------------------------------------
> Kazutoshi Satoda (2):
>       git-svn: enable "svn.pathnameencoding" on dcommit
>       git-svn: apply "svn.pathnameencoding" before URL encoding
>
>  perl/Git/SVN/Editor.pm                   |  4 +++-
>  t/t9115-git-svn-dcommit-funky-renames.sh | 26 ++++++++++++++++++++++++--
>  2 files changed, 27 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PULL] svn pathnameencoding for git svn dcommit
  2016-02-15  0:52   ` [PULL] svn pathnameencoding for git svn dcommit Eric Wong
  2016-02-15 21:32     ` Junio C Hamano
@ 2016-02-16  3:29     ` Kazutoshi Satoda
  2016-02-16  6:33       ` Eric Wong
  1 sibling, 1 reply; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-02-16  3:29 UTC (permalink / raw)
  To: Eric Wong, Junio C Hamano; +Cc: git, alex.crezoff

On 2016/02/15 9:52 +0900, Eric Wong wrote:
> I've amended tests to both commits, but the URL encoding one
> requires an HTTP server to test effectively.

Thank you for the tests. But, on my environment, both of them failed
unexpectedly. (Windows 7 SP1, x86_64 Cygwin, LANG=ja_JP.UTF-8)

For now, I don't have enough time to investigate this further. Let me
just share the result.

> $ ./t9115-git-svn-dcommit-funky-renames.sh -x --run='11-12'
(snip)
> expecting success:
>         neq=$(printf "\201\202") &&
>         git config svn.pathnameencoding cp932 &&
>         echo neq >"$neq" &&
>         git add "$neq" &&
>         git commit -m "neq" &&
>         git svn dcommit
>
> +++ printf '\201\202'
> ++ neq=$'\201\202'
> ++ git config svn.pathnameencoding cp932
> ++ echo neq
> ++ git add $'\201\202'
> ++ git commit -m neq
> On branch master
>
> Initial commit
>
> Untracked files:
>         svnrepo/
>         "\357\202\201\357\202\202"
>
> nothing added to commit but untracked files present
> error: last command exited with $?=1
> not ok 11 - svn.pathnameencoding=cp932 new file on dcommit
> #
> #               neq=$(printf "\201\202") &&
> #               git config svn.pathnameencoding cp932 &&
> #               echo neq >"$neq" &&
> #               git add "$neq" &&
> #               git commit -m "neq" &&
> #               git svn dcommit
> #
>
> expecting success:
>         inf=$(printf "\201\207") &&
>         git config svn.pathnameencoding cp932 &&
>         echo inf >"$inf" &&
>         git add "$inf" &&
>         git commit -m "inf" &&
>         git svn dcommit &&
>         git mv "$inf" inf &&
>         git commit -m "inf rename" &&
>         git svn dcommit
>
> +++ printf '\201\207'
> ++ inf=$'\201\207'
> ++ git config svn.pathnameencoding cp932
> ++ echo inf
> ++ git add $'\201\207'
> ++ git commit -m inf
> On branch master
>
> Initial commit
>
> Untracked files:
>         svnrepo/
>         "\357\202\201\357\202\202"
>         "\357\202\201\357\202\207"
>
> nothing added to commit but untracked files present
> error: last command exited with $?=1
> not ok 12 - svn.pathnameencoding=cp932 rename on dcommit
> #
> #               inf=$(printf "\201\207") &&
> #               git config svn.pathnameencoding cp932 &&
> #               echo inf >"$inf" &&
> #               git add "$inf" &&
> #               git commit -m "inf" &&
> #               git svn dcommit &&
> #               git mv "$inf" inf &&
> #               git commit -m "inf rename" &&
> #               git svn dcommit
> #

Strangely, after the test failure, "git status" in the trash directory
reports different status.
> $ cd 'trash directory.t9115-git-svn-dcommit-funky-renames'
> $ git status
> On branch master
>
> Initial commit
>
> Untracked files:
>   (use "git add <file>..." to include in what will be committed)
>
>         svnrepo/
>         "\201\202"
>         "\201\207"
>
> nothing added to commit but untracked files present (use "git add" to track)

I can manually add and commit them.
> $ neq=$(printf "\201\202")
> $ git add "$neq"
> $ git commit -m "neq"
> [master (root-commit) 3fd464f] neq
>  1 file changed, 1 insertion(+)
>  create mode 100644 "\201\202"

I can't see how "\357\202\201\357\202\202" came as output in the test.

-- 
k_satoda

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PULL] svn pathnameencoding for git svn dcommit
  2016-02-16  3:29     ` Kazutoshi Satoda
@ 2016-02-16  6:33       ` Eric Wong
  2016-02-16 16:19         ` Kazutoshi Satoda
  0 siblings, 1 reply; 25+ messages in thread
From: Eric Wong @ 2016-02-16  6:33 UTC (permalink / raw)
  To: Kazutoshi Satoda, Junio C Hamano; +Cc: git, alex.crezoff

Junio, sorry about basing on next, I somehow
thought I was going to need to depend on something there.
Updated pull below if Kazutoshi can run the test effectively.

Kazutoshi Satoda <k_satoda@f2.dion.ne.jp> wrote:
> Thank you for the tests. But, on my environment, both of them failed
> unexpectedly. (Windows 7 SP1, x86_64 Cygwin, LANG=ja_JP.UTF-8)
> 
> For now, I don't have enough time to investigate this further. Let me
> just share the result.

<snip>

> > Untracked files:
> >         svnrepo/
> >         "\357\202\201\357\202\202"
> >         "\357\202\201\357\202\207"
> >

<snip>

> I can't see how "\357\202\201\357\202\202" came as output in the test.

I wonder if it's a shell or printf portability problem.  I've
repushed the branch against master and implemented the weird
character use inside Perl scripts.

Can you give it a try?

The following changes since commit 494398473714dcbedb38b1ac79b531c7384b3bc4:

  Sixth batch for the 2.8 cycle (2016-02-10 14:24:14 -0800)

are available in the git repository at:

  git://bogomips.org/git-svn.git ks/svn-pathnameencoding

for you to fetch changes up to 7d7ea44ea5a1a38ee36402e6709e64c05650ba46:

  git-svn: apply "svn.pathnameencoding" before URL encoding (2016-02-16 06:25:01 +0000)

----------------------------------------------------------------
Kazutoshi Satoda (2):
      git-svn: enable "svn.pathnameencoding" on dcommit
      git-svn: apply "svn.pathnameencoding" before URL encoding

 perl/Git/SVN/Editor.pm                   |  4 +++-
 t/t9115-git-svn-dcommit-funky-renames.sh | 16 ++++++++++++++--
 t/t9115/inf.perl                         | 12 ++++++++++++
 t/t9115/neq.perl                         |  5 +++++
 4 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 t/t9115/inf.perl
 create mode 100644 t/t9115/neq.perl

 t/t9115-git-svn-dcommit-funky-renames.sh | 16 +++-------------
 t/t9115/inf.perl                         | 12 ++++++++++++
 t/t9115/neq.perl                         |  5 +++++
 3 files changed, 20 insertions(+), 13 deletions(-)

Interdiff of test changes:

diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
index 9828f05..94698fe 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -84,25 +84,15 @@ test_expect_success 'git svn rebase works inside a fresh-cloned repository' '
 		test -e test-rebase
 	)'
 
-test_expect_success 'svn.pathnameencoding=cp932 new file on dcommit' '
-	neq=$(printf "\201\202") &&
+test_expect_success PERL 'svn.pathnameencoding=cp932 new file on dcommit' '
 	git config svn.pathnameencoding cp932 &&
-	echo neq >"$neq" &&
-	git add "$neq" &&
+	"$PERL_PATH" -w "$TEST_DIRECTORY"/t9115/neq.perl &&
 	git commit -m "neq" &&
 	git svn dcommit
 '
 
 test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
-	inf=$(printf "\201\207") &&
-	git config svn.pathnameencoding cp932 &&
-	echo inf >"$inf" &&
-	git add "$inf" &&
-	git commit -m "inf" &&
-	git svn dcommit &&
-	git mv "$inf" inf &&
-	git commit -m "inf rename" &&
-	git svn dcommit
+	"$PERL_PATH" -w "$TEST_DIRECTORY"/t9115/inf.perl
 '
 
 stop_httpd
diff --git a/t/t9115/inf.perl b/t/t9115/inf.perl
new file mode 100644
index 0000000..0adcfc7
--- /dev/null
+++ b/t/t9115/inf.perl
@@ -0,0 +1,12 @@
+use strict;
+my $path = "\201\207";
+open my $fh, '>', $path or die $!;
+close $fh or die $!;
+sub run { system(@_) == 0 or die join(' ', @_) . ": $?" }
+run(qw(git config svn.pathnameencoding cp932));
+run(qw(git add), $path);
+run(qw(git commit -m inf));
+run(qw(git svn dcommit));
+run(qw(git mv), $path, 'inf');
+run(qw(git commit -m inf-rename));
+run(qw(git svn dcommit));
diff --git a/t/t9115/neq.perl b/t/t9115/neq.perl
new file mode 100644
index 0000000..91b1061
--- /dev/null
+++ b/t/t9115/neq.perl
@@ -0,0 +1,5 @@
+use strict;
+my $path = "\201\202";
+open my $fh, '>', $path or die $!;
+close $fh or die $!;
+exec(qw(git add), $path);

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PULL] svn pathnameencoding for git svn dcommit
  2016-02-16  6:33       ` Eric Wong
@ 2016-02-16 16:19         ` Kazutoshi Satoda
  2016-02-20 23:37           ` Eric Wong
  0 siblings, 1 reply; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-02-16 16:19 UTC (permalink / raw)
  To: Eric Wong, Junio C Hamano; +Cc: git, alex.crezoff

[-- Attachment #1: Type: text/plain, Size: 3300 bytes --]

On 2016/02/16 15:33 +0900, Eric Wong wrote:
> Kazutoshi Satoda <k_satoda@f2.dion.ne.jp> wrote:
>> Thank you for the tests. But, on my environment, both of them failed
>> unexpectedly. (Windows 7 SP1, x86_64 Cygwin, LANG=ja_JP.UTF-8)
...
>> > Untracked files:
>> >         svnrepo/
>> >         "\357\202\201\357\202\202"
>> >         "\357\202\201\357\202\207"
...
>> I can't see how "\357\202\201\357\202\202" came as output in the test.
> 
> I wonder if it's a shell or printf portability problem.  I've
> repushed the branch against master and implemented the weird
> character use inside Perl scripts.
> 
> Can you give it a try?

(Shouldn't the branch be based on maint, as these are bugfixes?)

Thank you. I tried it but got similar problem:
> $ ./t9115-git-svn-dcommit-funky-renames.sh -x --run='11-12'
(snip)
> expecting success:
>         git config svn.pathnameencoding cp932 &&
>         "$PERL_PATH" -w "$TEST_DIRECTORY"/t9115/neq.perl &&
>         git commit -m "neq" &&
>         git svn dcommit
>
> ++ git config svn.pathnameencoding cp932
> ++ /usr/bin/perl -w /home/k_satoda/project/git/t/t9115/neq.perl
> ++ git commit -m neq
> On branch master
>
> Initial commit
>
> Untracked files:
>         svnrepo/
>         "\357\202\201\357\202\202"
>
> nothing added to commit but untracked files present
> error: last command exited with $?=1
> not ok 11 - svn.pathnameencoding=cp932 new file on dcommit
> #
> #               git config svn.pathnameencoding cp932 &&
> #               "$PERL_PATH" -w "$TEST_DIRECTORY"/t9115/neq.perl &&
> #               git commit -m "neq" &&
> #               git svn dcommit
> #
>
> expecting success:
>         "$PERL_PATH" -w "$TEST_DIRECTORY"/t9115/inf.perl
>
> ++ /usr/bin/perl -w /home/k_satoda/project/git/t/t9115/inf.perl
> On branch master
>
> Initial commit
>
> Untracked files:
>         svnrepo/
>         "\357\202\201\357\202\202"
>         "\357\202\201\357\202\207"
>
> nothing added to commit but untracked files present
> git commit -m inf: 256 at /home/k_satoda/project/git/t/t9115/inf.perl line 5.
> error: last command exited with $?=1
> not ok 12 - svn.pathnameencoding=cp932 rename on dcommit
> #
> #               "$PERL_PATH" -w "$TEST_DIRECTORY"/t9115/inf.perl
> #


I found how "\357\202\201\357\202\202" (U+F081 U+F082 in UTF-8) could
come.
https://cygwin.com/cygwin-ug-net/using-specialnames.html#pathnames-specialchars
> Some characters are disallowed in filenames on Windows filesystems. ...
...
> ... All of the above characters, except for the backslash, are converted
> to special UNICODE characters in the range 0xf000 to 0xf0ff (the
> "Private use area") when creating or accessing files.
"U+F081 U+F082" seems the result of conversion from "0x8182" (neq in
cp932) as treating each of 2 bytes as disallowed characters.

And I also noticed that LANG and LC_ALL is set to "C" in test-lib.sh.

Setting LC_ALL=C.UTF-8 in the test 11-12 made them pass on Cygwin.
Same change made the previous version also pass. Please find the patch
in the attached output of git format-patch.

Could you please test with this on non-Cygwin environment?

If it made no harm, please tell me what should I do to proceed this patch.
Will you (Eric) please make further integration? Shall I make another
series (v2) of patches?

-- 
k_satoda

[-- Attachment #2: 0001-Add-LC_ALL-C.UTF-8-in-t9115-git-svn-dcommit-funky-re.patch --]
[-- Type: text/plain, Size: 1808 bytes --]

>From 7b827f2d65aaa859030ba5b08055020f2bda1f0f Mon Sep 17 00:00:00 2001
From: Kazutoshi SATODA <k_satoda@f2.dion.ne.jp>
Date: Wed, 17 Feb 2016 00:29:24 +0900
Subject: [PATCH] Add LC_ALL=C.UTF-8 in t9115-git-svn-dcommit-funky-renames.sh

This makes the test 11-12 pass on Cygwin.
---
 t/t9115-git-svn-dcommit-funky-renames.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
index 9828f05..59d086b 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -84,7 +84,16 @@ test_expect_success 'git svn rebase works inside a fresh-cloned repository' '
 		test -e test-rebase
 	)'
 
+# Without this, LC_ALL=C as set in test-lib.sh, and Cygwin converts
+# non-ASCII characters in filenames unexpectedly, and causes errors.
+# https://cygwin.com/cygwin-ug-net/using-specialnames.html#pathnames-specialchars
+# > Some characters are disallowed in filenames on Windows filesystems. ...
+# ...
+# > ... All of the above characters, except for the backslash, are converted
+# > to special UNICODE characters in the range 0xf000 to 0xf0ff (the
+# > "Private use area") when creating or accessing files.
 test_expect_success 'svn.pathnameencoding=cp932 new file on dcommit' '
+	export LC_ALL=C.UTF-8 &&
 	neq=$(printf "\201\202") &&
 	git config svn.pathnameencoding cp932 &&
 	echo neq >"$neq" &&
@@ -93,7 +102,9 @@ test_expect_success 'svn.pathnameencoding=cp932 new file on dcommit' '
 	git svn dcommit
 '
 
+# See the comment on the above test for setting of LC_ALL.
 test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
+	export LC_ALL=C.UTF-8 &&
 	inf=$(printf "\201\207") &&
 	git config svn.pathnameencoding cp932 &&
 	echo inf >"$inf" &&
-- 
2.7.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PULL] svn pathnameencoding for git svn dcommit
  2016-02-16 16:19         ` Kazutoshi Satoda
@ 2016-02-20 23:37           ` Eric Wong
  2016-02-21 13:12             ` Kazutoshi Satoda
  0 siblings, 1 reply; 25+ messages in thread
From: Eric Wong @ 2016-02-20 23:37 UTC (permalink / raw)
  To: Kazutoshi Satoda; +Cc: Junio C Hamano, git, alex.crezoff

Kazutoshi Satoda <k_satoda@f2.dion.ne.jp> wrote:
> (Shouldn't the branch be based on maint, as these are bugfixes?)

I'm not sure if it being previously left-out/untested feature
would qualify for maint.  Maybe it does, but I suppose I'll let
Junio decide.

> Thank you. I tried it but got similar problem:

<snip>

> I found how "\357\202\201\357\202\202" (U+F081 U+F082 in UTF-8) could
> come.
> https://cygwin.com/cygwin-ug-net/using-specialnames.html#pathnames-specialchars
> > Some characters are disallowed in filenames on Windows filesystems. ...
> ...
> > ... All of the above characters, except for the backslash, are converted
> > to special UNICODE characters in the range 0xf000 to 0xf0ff (the
> > "Private use area") when creating or accessing files.
> "U+F081 U+F082" seems the result of conversion from "0x8182" (neq in
> cp932) as treating each of 2 bytes as disallowed characters.
> 
> And I also noticed that LANG and LC_ALL is set to "C" in test-lib.sh.
> 
> Setting LC_ALL=C.UTF-8 in the test 11-12 made them pass on Cygwin.
> Same change made the previous version also pass. Please find the patch
> in the attached output of git format-patch.

Thanks.  However, I also wonder what happens on machines without
"C.UTF-8" support (are there still any?).

> Could you please test with this on non-Cygwin environment?

Works for me, at least.  I've squashed your changes into the two
patches already queued up.  I needed to split the
"export LC_ALL=C.UTF-8" statement into
"LC_ALL=C.UTF-8 && export LC_ALL" for portability.

> If it made no harm, please tell me what should I do to proceed this patch.
> Will you (Eric) please make further integration? Shall I make another
> series (v2) of patches?

I've pushed out a new branch with your LC_ALL changes squashed
in.  However I'm unsure if there's any new portability problems
with LC_ALL=C.UTF-8...

Junio or anyone else: thoughts?

The following changes since commit 0233b800c838ddda41db318ee396320b3c21a560:

  Merge branch 'maint' (2016-02-17 10:14:39 -0800)

are available in the git repository at:

  git://bogomips.org/git-svn.git ks/svn-pathnameencoding-3

for you to fetch changes up to 980c083276ba06a9400c5b1b2558c3626bcff969:

  git-svn: apply "svn.pathnameencoding" before URL encoding (2016-02-20 23:30:16 +0000)

----------------------------------------------------------------
Kazutoshi Satoda (2):
      git-svn: enable "svn.pathnameencoding" on dcommit
      git-svn: apply "svn.pathnameencoding" before URL encoding

 perl/Git/SVN/Editor.pm                   |  4 +++-
 t/t9115-git-svn-dcommit-funky-renames.sh | 39 ++++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PULL] svn pathnameencoding for git svn dcommit
  2016-02-20 23:37           ` Eric Wong
@ 2016-02-21 13:12             ` Kazutoshi Satoda
  0 siblings, 0 replies; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-02-21 13:12 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git, alex.crezoff

On 2016/02/21 8:37 +0900, Eric Wong wrote:
> Kazutoshi Satoda <k_satoda@f2.dion.ne.jp> wrote:
...
>> Setting LC_ALL=C.UTF-8 in the test 11-12 made them pass on Cygwin.
>> Same change made the previous version also pass. Please find the patch
>> in the attached output of git format-patch.
> 
> Thanks.  However, I also wonder what happens on machines without
> "C.UTF-8" support (are there still any?).
> 
>> Could you please test with this on non-Cygwin environment?
> 
> Works for me, at least.  I've squashed your changes into the two
> patches already queued up.  I needed to split the
> "export LC_ALL=C.UTF-8" statement into
> "LC_ALL=C.UTF-8 && export LC_ALL" for portability.

Thank you.

>> If it made no harm, please tell me what should I do to proceed this patch.
>> Will you (Eric) please make further integration? Shall I make another
>> series (v2) of patches?
> 
> I've pushed out a new branch with your LC_ALL changes squashed
> in.  However I'm unsure if there's any new portability problems
> with LC_ALL=C.UTF-8...
> 
> Junio or anyone else: thoughts?

The test passed on my environment.

I've searched use of LC_ALL values other than "C".
It seems be the best to move the variable a_utf8_locale in t9129 to
lib-git-svn.sh and use it also in t9115.


t/Makefile:83
> 	$(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8

Here, "en_US.UTF-8" is hard-coded. I think this is at least more
problematic than hard-coding "C.UTF-8". Beside hard-coding, does this
take effect while test-lib.sh does LC_ALL=C ?

t/t9129-git-svn-i18n-commitencoding.sh:17
> a_utf8_locale=$(locale -a | sed -n '/\.[uU][tT][fF]-*8$/{
> 	p
> 	q
> }')
>
> if test -n "$a_utf8_locale"
> then
> 	test_set_prereq UTF8
> else
> 	say "# UTF-8 locale not available, some tests are skipped"
> fi

Here, a UTF-8 locale is took from "locale -a", and the test is skipped
if not found. This gives "a_utf8_locale=C.utf8" on my Cygwin
environment. There was a record that says the difference of ".utf8" and
".UTF-8" caused a failure.
https://git.kernel.org/cgit/git/git.git/commit/?id=2de03ebe0635c93e182c3367140f999e79bdadcd

t/lib-gettext.sh:17
> 	# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
> 	is_IS_locale=$(locale -a 2>/dev/null |
> 		sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
> 		p
> 		q
> 	}')
...
> 	if test -n "$is_IS_locale" &&
> 		test $GIT_INTERNAL_GETTEXT_SH_SCHEME != "fallthrough"
> 	then
> 		# Some of the tests need the reference Icelandic locale
> 		test_set_prereq GETTEXT_LOCALE
...
> 	else
> 		say "# lib-gettext: No is_IS UTF-8 locale available"
> 	fi
(the same logic is used for is_IS.ISO8859-1.)

Here, a UTF-8 locale with hard coded "is_IS" is took from "locale -a",
and the test is skipped if not found. This gives
"is_IS_locale=is_IS.utf8.utf8" on my Cygwin environment.

-- 
k_satoda

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-02-08 22:58 ` [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Eric Wong
  2016-02-15  0:52   ` [PULL] svn pathnameencoding for git svn dcommit Eric Wong
@ 2016-02-27 18:28   ` tboegi
  2016-02-28  4:59     ` Eric Wong
  1 sibling, 1 reply; 25+ messages in thread
From: tboegi @ 2016-02-27 18:28 UTC (permalink / raw)
  To: git; +Cc: normalperson, k_satoda, Torsten Bögershausen

From: Torsten Bögershausen <tboegi@web.de>

HFS+ under Mac OS X doesn't allow file names like "\201\207",
so skip the tests using cp932.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t9115-git-svn-dcommit-funky-renames.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
index 0990f8d..805a9eb 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -93,7 +93,7 @@ test_expect_success 'git svn rebase works inside a fresh-cloned repository' '
 # > to special UNICODE characters in the range 0xf000 to 0xf0ff (the
 # > "Private use area") when creating or accessing files.
 prepare_a_utf8_locale
-test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
+test_expect_success !UTF8_NFD_TO_NFC,UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
 	LC_ALL=$a_utf8_locale &&
 	export LC_ALL &&
 	neq=$(printf "\201\202") &&
@@ -105,10 +105,10 @@ test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
 '
 
 # See the comment on the above test for setting of LC_ALL.
-test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
+test_expect_success !UTF8_NFD_TO_NFC,UTF8 'svn.pathnameencoding=cp932 rename on dcommit' '
 	LC_ALL=$a_utf8_locale &&
 	export LC_ALL &&
-	inf=$(printf "\201\207") &&
+	inf=$(printf "\201\207"o) &&
 	git config svn.pathnameencoding cp932 &&
 	echo inf >"$inf" &&
 	git add "$inf" &&
-- 
2.7.0.303.g2c4f448.dirty

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-02-27 18:28   ` [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS tboegi
@ 2016-02-28  4:59     ` Eric Wong
  2016-02-28 17:52       ` Torsten Bögershausen
  0 siblings, 1 reply; 25+ messages in thread
From: Eric Wong @ 2016-02-28  4:59 UTC (permalink / raw)
  To: tboegi; +Cc: git, k_satoda

tboegi@web.de wrote:
> --- a/t/t9115-git-svn-dcommit-funky-renames.sh
> +++ b/t/t9115-git-svn-dcommit-funky-renames.sh
> @@ -93,7 +93,7 @@ test_expect_success 'git svn rebase works inside a fresh-cloned repository' '
>  # > to special UNICODE characters in the range 0xf000 to 0xf0ff (the
>  # > "Private use area") when creating or accessing files.
>  prepare_a_utf8_locale
> -test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
> +test_expect_success !UTF8_NFD_TO_NFC,UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '

Please keep lines wrapped at 80 cols or less.
(I need big fonts)

> @@ -105,10 +105,10 @@ test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
>  '
>  
>  # See the comment on the above test for setting of LC_ALL.
> -test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
> +test_expect_success !UTF8_NFD_TO_NFC,UTF8 'svn.pathnameencoding=cp932 rename on dcommit' '
>  	LC_ALL=$a_utf8_locale &&
>  	export LC_ALL &&
> -	inf=$(printf "\201\207") &&
> +	inf=$(printf "\201\207"o) &&

Why the extra 'o'?

>  	git config svn.pathnameencoding cp932 &&
>  	echo inf >"$inf" &&
>  	git add "$inf" &&
> -- 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-02-28  4:59     ` Eric Wong
@ 2016-02-28 17:52       ` Torsten Bögershausen
  2016-03-15  1:59         ` Eric Wong
  0 siblings, 1 reply; 25+ messages in thread
From: Torsten Bögershausen @ 2016-02-28 17:52 UTC (permalink / raw)
  To: Eric Wong, tboegi; +Cc: git, k_satoda

On 28.02.16 05:59, Eric Wong wrote:
> tboegi@web.de wrote:
> Please keep lines wrapped at 80 cols or less.
> (I need big fonts)
OK
> 
>> @@ -105,10 +105,10 @@ test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
>>  '
> Why the extra 'o'?
That shouldn't be there, sorry.
Will send V2...

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-02-28 17:52       ` Torsten Bögershausen
@ 2016-03-15  1:59         ` Eric Wong
  2016-03-15  5:23           ` Torsten Bögershausen
  0 siblings, 1 reply; 25+ messages in thread
From: Eric Wong @ 2016-03-15  1:59 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git, k_satoda

Torsten Bögershausen <tboegi@web.de> wrote:
> On 28.02.16 05:59, Eric Wong wrote:
> > tboegi@web.de wrote:
> > Please keep lines wrapped at 80 cols or less.
> > (I need big fonts)
> OK
> > 
> >> @@ -105,10 +105,10 @@ test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
> >>  '
> > Why the extra 'o'?
> That shouldn't be there, sorry.
> Will send V2...

I just edited locally and pushed those out to Junio:

http://mid.gmane.org/20160315015726.GA25295@dcvr.yhbt.net

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-15  1:59         ` Eric Wong
@ 2016-03-15  5:23           ` Torsten Bögershausen
  2016-03-15  7:09             ` Eric Wong
  0 siblings, 1 reply; 25+ messages in thread
From: Torsten Bögershausen @ 2016-03-15  5:23 UTC (permalink / raw)
  To: Eric Wong, Torsten Bögershausen; +Cc: git, k_satoda

On 03/15/2016 02:59 AM, Eric Wong wrote:
> []
> I just edited locally and pushed those out to Junio:
>
> http://mid.gmane.org/20160315015726.GA25295@dcvr.yhbt.net
>

The new TC 11/12 don't pass under cygwin.

Do we need cp932 ?

If not, we may use the paych from here:
https://github.com/tboegi/git/commit/379c01bf52464f8a50065b11af516127e9144045

Date:   Tue Mar 15 05:03:18 2016 +0100

     t9115: Use funcky file names that work under unicode FS

     Don't use funky file names, that can not be created under
     HFS or NTFS.

diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh 
b/t/t9115-git-svn-dcommit-funky-renames.sh
index 0990f8d..d022f0d 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -93,11 +93,11 @@ test_expect_success 'git svn rebase works inside a 
fresh-cloned repository' '
  # > to special UNICODE characters in the range 0xf000 to 0xf0ff (the
  # > "Private use area") when creating or accessing files.
  prepare_a_utf8_locale
-test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
+test_expect_success UTF8 'svn.pathnameencoding=ISO8859-1 new file on dcommit' '
         LC_ALL=$a_utf8_locale &&
         export LC_ALL &&
-       neq=$(printf "\201\202") &&
-       git config svn.pathnameencoding cp932 &&
+       neq=$(printf "\303\244") &&
+       git config svn.pathnameencoding ISO8859-1 &&
         echo neq >"$neq" &&
         git add "$neq" &&
         git commit -m "neq" &&
@@ -105,11 +105,11 @@ test_expect_success UTF8 'svn.pathnameencoding=cp932 new 
file on dcommit' '
  '

  # See the comment on the above test for setting of LC_ALL.
-test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
+test_expect_success 'svn.pathnameencoding=ISO8859-1 rename on dcommit' '
         LC_ALL=$a_utf8_locale &&
         export LC_ALL &&
-       inf=$(printf "\201\207") &&
-       git config svn.pathnameencoding cp932 &&
+       inf=$(printf "\303\226") &&
+       git config svn.pathnameencoding ISO8859-1 &&
         echo inf >"$inf" &&
         git add "$inf" &&
         git commit -m "inf" &&

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-15  5:23           ` Torsten Bögershausen
@ 2016-03-15  7:09             ` Eric Wong
  2016-03-16 17:37               ` Kazutoshi Satoda
  0 siblings, 1 reply; 25+ messages in thread
From: Eric Wong @ 2016-03-15  7:09 UTC (permalink / raw)
  To: Kazutoshi Satoda; +Cc: git, Torsten Bögershausen

Torsten Bögershausen <tboegi@web.de> wrote:
> On 03/15/2016 02:59 AM, Eric Wong wrote:
> >[]
> >I just edited locally and pushed those out to Junio:
> >
> >http://mid.gmane.org/20160315015726.GA25295@dcvr.yhbt.net
> >
> 
> The new TC 11/12 don't pass under cygwin.
> 
> Do we need cp932 ?

Not sure, both CP932 and ISO8859-1 work fine for me on
GNU/Linux.   Anyways I'm fine skipping this patch for 2.8
while we hash it out, too.

Kazutoshi: can you answer?  Thanks.

> If not, we may use the paych from here:
> https://github.com/tboegi/git/commit/379c01bf52464f8a50065b11af516127e9144045
> 
> Date:   Tue Mar 15 05:03:18 2016 +0100
> 
>     t9115: Use funcky file names that work under unicode FS
> 
>     Don't use funky file names, that can not be created under
>     HFS or NTFS.
> 
> diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh
> b/t/t9115-git-svn-dcommit-funky-renames.sh
> index 0990f8d..d022f0d 100755
> --- a/t/t9115-git-svn-dcommit-funky-renames.sh
> +++ b/t/t9115-git-svn-dcommit-funky-renames.sh
> @@ -93,11 +93,11 @@ test_expect_success 'git svn rebase works inside
> a fresh-cloned repository' '
>  # > to special UNICODE characters in the range 0xf000 to 0xf0ff (the
>  # > "Private use area") when creating or accessing files.
>  prepare_a_utf8_locale
> -test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
> +test_expect_success UTF8 'svn.pathnameencoding=ISO8859-1 new file on dcommit' '
>         LC_ALL=$a_utf8_locale &&
>         export LC_ALL &&
> -       neq=$(printf "\201\202") &&
> -       git config svn.pathnameencoding cp932 &&
> +       neq=$(printf "\303\244") &&
> +       git config svn.pathnameencoding ISO8859-1 &&
>         echo neq >"$neq" &&
>         git add "$neq" &&
>         git commit -m "neq" &&
> @@ -105,11 +105,11 @@ test_expect_success UTF8
> 'svn.pathnameencoding=cp932 new file on dcommit' '
>  '
> 
>  # See the comment on the above test for setting of LC_ALL.
> -test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
> +test_expect_success 'svn.pathnameencoding=ISO8859-1 rename on dcommit' '
>         LC_ALL=$a_utf8_locale &&
>         export LC_ALL &&
> -       inf=$(printf "\201\207") &&
> -       git config svn.pathnameencoding cp932 &&
> +       inf=$(printf "\303\226") &&
> +       git config svn.pathnameencoding ISO8859-1 &&
>         echo inf >"$inf" &&
>         git add "$inf" &&
>         git commit -m "inf" &&
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-15  7:09             ` Eric Wong
@ 2016-03-16 17:37               ` Kazutoshi Satoda
  2016-03-17  5:16                 ` Torsten Bögershausen
  0 siblings, 1 reply; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-03-16 17:37 UTC (permalink / raw)
  To: Eric Wong, Torsten Bögershausen; +Cc: git

On 2016/03/15 16:09 +0900, Eric Wong wrote:
> Torsten Bögershausen <tboegi@web.de> wrote:
>> On 03/15/2016 02:59 AM, Eric Wong wrote:
>> >[]
>> >I just edited locally and pushed those out to Junio:
>> >
>> >http://mid.gmane.org/20160315015726.GA25295@dcvr.yhbt.net
>> >
>> 
>> The new TC 11/12 don't pass under cygwin.
>> 
>> Do we need cp932 ?
> 
> Not sure, both CP932 and ISO8859-1 work fine for me on
> GNU/Linux.   Anyways I'm fine skipping this patch for 2.8
> while we hash it out, too.
> 
> Kazutoshi: can you answer?  Thanks.

I tried the patch. The test works (pass with my fixes, and fails
without fixes) with ISO8859-1 for me on Cygnus. The change sounds
good.


>> If not, we may use the paych from here:
>> https://github.com/tboegi/git/commit/379c01bf52464f8a50065b11af516127e9144045
>> 
>> Date:   Tue Mar 15 05:03:18 2016 +0100
>> 
>>     t9115: Use funcky file names that work under unicode FS

"funcky" looks a typo.

>>     Don't use funky file names, that can not be created under
>>     HFS or NTFS.

The file can be created on my Cygnus environment, which is under FONTS.
So it looks a bit inaccurate.

I think a quote from the actual error message may be useful. It will
likely tell what was wrong, accurately. And also, someone may search for
that message.

>> -       neq=$(printf "\201\202") &&
>> -       git config svn.pathnameencoding cp932 &&
>> +       neq=$(printf "\303\244") &&
>> +       git config svn.pathnameencoding ISO8859-1 &&

The variable name "new" was for "NOT EQUAL TO" (0x8182 in cp932 = U+2260).
http://unicode.org/Public/MAPPINGS/VENDORS/MICSFT/WINDOWS/CP932.TXT
Then it should be changed, too. A more abstract one may be appropriate.

>> -       inf=$(printf "\201\207") &&
>> -       git config svn.pathnameencoding cp932 &&
>> +       inf=$(printf "\303\226") &&
>> +       git config svn.pathnameencoding ISO8859-1 &&

Ditto. (0x8187 in cp932 = U+221E, INFINITY)

-- 
k_satoda

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-16 17:37               ` Kazutoshi Satoda
@ 2016-03-17  5:16                 ` Torsten Bögershausen
  2016-03-17  5:35                   ` Torsten Bögershausen
  2016-03-18  2:14                   ` Kazutoshi Satoda
  0 siblings, 2 replies; 25+ messages in thread
From: Torsten Bögershausen @ 2016-03-17  5:16 UTC (permalink / raw)
  To: Kazutoshi Satoda, Eric Wong, Torsten Bögershausen; +Cc: git

On 2016-03-16 18.37, Kazutoshi Satoda wrote:
> "funcky" looks a typo.
> 
>>>     Don't use funky file names, that can not be created under
>>>     HFS or NTFS.
> 
> The file can be created on my Cygnus environment, which is under FONTS.
> So it looks a bit inaccurate.
> 
> I think a quote from the actual error message may be useful. It will
> likely tell what was wrong, accurately. And also, someone may search for
> that message.
> 
>>> -       neq=$(printf "\201\202") &&
>>> -       git config svn.pathnameencoding cp932 &&
>>> +       neq=$(printf "\303\244") &&
>>> +       git config svn.pathnameencoding ISO8859-1 &&
> 
> The variable name "new" was for "NOT EQUAL TO" (0x8182 in cp932 = U+2260).
> http://unicode.org/Public/MAPPINGS/VENDORS/MICSFT/WINDOWS/CP932.TXT
> Then it should be changed, too. A more abstract one may be appropriate.
> 
>>> -       inf=$(printf "\201\207") &&
>>> -       git config svn.pathnameencoding cp932 &&
>>> +       inf=$(printf "\303\226") &&
>>> +       git config svn.pathnameencoding ISO8859-1 &&
> 
> Ditto. (0x8187 in cp932 = U+221E, INFINITY)
> 

Agreed with all your comments, thanks for that.
A better version is here:
<https://github.com/tboegi/git/commit/7ea2fa1ffaeb1c05669a837d7fed9c60b8a0d3cb>


commit 7ea2fa1ffaeb1c05669a837d7fed9c60b8a0d3cb
Author: Torsten Bögershausen <tboegi@web.de>
Date:   Thu Mar 17 06:08:14 2016 +0100

    t9115: Use prereq for funky file name

    Some file systems like HFS don't allow file names outside unicode.
    Add a precondition FS_CP932 and use it in t9115#11 and #12

diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh
b/t/t9115-git-svn-dcommit-funky-renames.sh
index 0990f8d..5a6525c 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -93,7 +93,7 @@ test_expect_success 'git svn rebase works inside a
fresh-cloned repository' '
 # > to special UNICODE characters in the range 0xf000 to 0xf0ff (the
 # > "Private use area") when creating or accessing files.
 prepare_a_utf8_locale
-test_expect_success UTF8 'svn.pathnameencoding=cp932 new file on dcommit' '
+test_expect_success UTF8,FS_CP932 'svn.pathnameencoding=cp932 new file on
dcommit' '
 	LC_ALL=$a_utf8_locale &&
 	export LC_ALL &&
 	neq=$(printf "\201\202") &&
@@ -105,7 +105,7 @@ test_expect_success UTF8 'svn.pathnameencoding=cp932 new
file on dcommit' '
 '

 # See the comment on the above test for setting of LC_ALL.
-test_expect_success 'svn.pathnameencoding=cp932 rename on dcommit' '
+test_expect_success UTF8,FS_CP932 'svn.pathnameencoding=cp932 rename on dcommit' '
 	LC_ALL=$a_utf8_locale &&
 	export LC_ALL &&
 	inf=$(printf "\201\207") &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0b47eb6..7bb1262 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1037,6 +1037,18 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
 	esac
 '

+test_lazy_prereq FS_CP932 '
+	# check whether FS allows filenames from cp932
+	neq=$(printf "\201\202")
+	>"$neq" &&
+	case "$(echo *)" in
+	"neq")
+		true ;;
+	*)
+		false ;;
+	esac
+'
+
 test_lazy_prereq AUTOIDENT '
 	sane_unset GIT_AUTHOR_NAME &&
 	sane_unset GIT_AUTHOR_EMAIL &&

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-17  5:16                 ` Torsten Bögershausen
@ 2016-03-17  5:35                   ` Torsten Bögershausen
  2016-03-18  2:15                     ` Kazutoshi Satoda
  2016-03-18  2:14                   ` Kazutoshi Satoda
  1 sibling, 1 reply; 25+ messages in thread
From: Torsten Bögershausen @ 2016-03-17  5:35 UTC (permalink / raw)
  To: Torsten Bögershausen, Kazutoshi Satoda, Eric Wong; +Cc: git

On 2016-03-17 06.16, Torsten Bögershausen wrote:

Oh Boy,
typo in the last patch and t9115#11,12 where skipped on Mac & Linux :-(

The "case" should look like this:

> +	"$neq")
> +		true ;;
> +	*)
> +		false ;;
> +	esac

And the pathch is here:
<https://github.com/tboegi/git/commit/866dfc192a0d4428aebfc7242f5134899b6dafd4>

Kazutoshi, could you re-test on your environment ?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-17  5:16                 ` Torsten Bögershausen
  2016-03-17  5:35                   ` Torsten Bögershausen
@ 2016-03-18  2:14                   ` Kazutoshi Satoda
  1 sibling, 0 replies; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-03-18  2:14 UTC (permalink / raw)
  To: Torsten Bögershausen, Eric Wong; +Cc: git

On 2016/03/17 14:16 +0900, Torsten Bögershausen wrote:
> On 2016-03-16 18.37, Kazutoshi Satoda wrote:
>> "funcky" looks a typo.
>> 
>>>>     Don't use funky file names, that can not be created under
>>>>     HFS or NTFS.
>> 
>> The file can be created on my Cygnus environment, which is under FONTS.
>> So it looks a bit inaccurate.

Uh, ... I noticed that I wrongly instruct the spell checker...

The above was meant to be:

  The file can be created on my Cygwin environment, which is under NTFS.
  So it looks a bit inaccurate.

>>>> -       neq=$(printf "\201\202") &&
>>>> -       git config svn.pathnameencoding cp932 &&
>>>> +       neq=$(printf "\303\244") &&
>>>> +       git config svn.pathnameencoding ISO8859-1 &&
>> 
>> The variable name "new" was for "NOT EQUAL TO" (0x8182 in cp932 = U+2260).
>> http://unicode.org/Public/MAPPINGS/VENDORS/MICSFT/WINDOWS/CP932.TXT

Here "new" was meant to be "neq", of course.


Sorry for the confusion.

-- 
k_satoda

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-17  5:35                   ` Torsten Bögershausen
@ 2016-03-18  2:15                     ` Kazutoshi Satoda
  2016-03-19  6:59                       ` Torsten Bögershausen
  0 siblings, 1 reply; 25+ messages in thread
From: Kazutoshi Satoda @ 2016-03-18  2:15 UTC (permalink / raw)
  To: Torsten Bögershausen, Eric Wong; +Cc: git

On 2016/03/17 14:35 +0900, Torsten Bögershausen wrote:
> On 2016-03-17 06.16, Torsten Bögershausen wrote:
...
> And the pathch is here:
> <https://github.com/tboegi/git/commit/866dfc192a0d4428aebfc7242f5134899b6dafd4>
> 
> Kazutoshi, could you re-test on your environment ?

The test 11,12 was skipped on my environment, too:
> $ ./t9115-git-svn-dcommit-funky-renames.sh
> ok 1 - load repository with strange names
> ...
> ok 10 - git svn rebase works inside a fresh-cloned repository
> ok 11 # skip svn.pathnameencoding=cp932 new file on dcommit (missing FS_CP932 of UTF8,FS_CP932)
> ok 12 # skip svn.pathnameencoding=cp932 rename on dcommit (missing FS_CP932 of UTF8,FS_CP932)
> # passed all 12 test(s)
> 1..12

That's because the check for FS_CP932 runs under LC_ALL=C environment
which affects how filenames are treated. See the comment on #11 for more
details.

Setting LC_ALL before the check works for me.

 test_lazy_prereq FS_CP932 '
        # check whether FS allows filenames from cp932
+       prepare_a_utf8_locale
+       LC_ALL=$a_utf8_locale &&
+       export LC_ALL &&
        neq=$(printf "\201\202")

But it looks a bit strange to do "prepare_a_utf8_locale" here because it
is done just before #11 now, though I think it does no harm.

I also don't understand the way of the check: Why don't you just touch
and test it, like this:
	touch "$neq" && test -e "$neq"
?

-- 
k_satoda

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS
  2016-03-18  2:15                     ` Kazutoshi Satoda
@ 2016-03-19  6:59                       ` Torsten Bögershausen
  0 siblings, 0 replies; 25+ messages in thread
From: Torsten Bögershausen @ 2016-03-19  6:59 UTC (permalink / raw)
  To: Kazutoshi Satoda, Torsten Bögershausen, Eric Wong; +Cc: git

On 2016-03-18 03.15, Kazutoshi Satoda wrote:
> On 2016/03/17 14:35 +0900, Torsten Bögershausen wrote:
>> On 2016-03-17 06.16, Torsten Bögershausen wrote:
> ...
>> And the pathch is here:
>> <https://github.com/tboegi/git/commit/866dfc192a0d4428aebfc7242f5134899b6dafd4>
>>
>> Kazutoshi, could you re-test on your environment ?
> 
> The test 11,12 was skipped on my environment, too:
>> $ ./t9115-git-svn-dcommit-funky-renames.sh
>> ok 1 - load repository with strange names
>> ...
>> ok 10 - git svn rebase works inside a fresh-cloned repository
>> ok 11 # skip svn.pathnameencoding=cp932 new file on dcommit (missing FS_CP932 of UTF8,FS_CP932)
>> ok 12 # skip svn.pathnameencoding=cp932 rename on dcommit (missing FS_CP932 of UTF8,FS_CP932)
>> # passed all 12 test(s)
>> 1..12
> 
> That's because the check for FS_CP932 runs under LC_ALL=C environment
> which affects how filenames are treated. See the comment on #11 for more
> details.
> 
> Setting LC_ALL before the check works for me.
> 
>  test_lazy_prereq FS_CP932 '
>         # check whether FS allows filenames from cp932
> +       prepare_a_utf8_locale
> +       LC_ALL=$a_utf8_locale &&
> +       export LC_ALL &&
>         neq=$(printf "\201\202")
> 
> But it looks a bit strange to do "prepare_a_utf8_locale" here because it
> is done just before #11 now, though I think it does no harm.
> 
> I also don't understand the way of the check: Why don't you just touch
> and test it, like this:
> 	touch "$neq" && test -e "$neq"
> ?
> 
Thanks, I will re-send a new version in a couple of days or so.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2016-03-19  6:59 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-08 15:19 [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Kazutoshi Satoda
2016-02-08 15:20 ` [PATCH 1/2] git-svn: " Kazutoshi Satoda
2016-02-15  0:30   ` Eric Wong
2016-02-08 15:21 ` [PATCH 2/2] git-svn: apply "svn.pathnameencoding" before URL encoding Kazutoshi Satoda
2016-02-15  0:33   ` Eric Wong
2016-02-08 22:58 ` [PATCH 0/2] enable "svn.pathnameencoding" on dcommit Eric Wong
2016-02-15  0:52   ` [PULL] svn pathnameencoding for git svn dcommit Eric Wong
2016-02-15 21:32     ` Junio C Hamano
2016-02-16  3:29     ` Kazutoshi Satoda
2016-02-16  6:33       ` Eric Wong
2016-02-16 16:19         ` Kazutoshi Satoda
2016-02-20 23:37           ` Eric Wong
2016-02-21 13:12             ` Kazutoshi Satoda
2016-02-27 18:28   ` [PATCH 1/1] t9115: Skip pathnameencoding=cp932 under HFS tboegi
2016-02-28  4:59     ` Eric Wong
2016-02-28 17:52       ` Torsten Bögershausen
2016-03-15  1:59         ` Eric Wong
2016-03-15  5:23           ` Torsten Bögershausen
2016-03-15  7:09             ` Eric Wong
2016-03-16 17:37               ` Kazutoshi Satoda
2016-03-17  5:16                 ` Torsten Bögershausen
2016-03-17  5:35                   ` Torsten Bögershausen
2016-03-18  2:15                     ` Kazutoshi Satoda
2016-03-19  6:59                       ` Torsten Bögershausen
2016-03-18  2:14                   ` Kazutoshi Satoda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.