darcs

Patch 1968 fix file path encoding in convert export command

Title fix file path encoding in convert export command
Superseder Nosy List bf, ganesh
Related Issues
Status rejected Assigned To ganesh
Milestone

Created on 2020-01-27.20:45:39 by bf, last changed 2020-07-05.07:13:27 by bf.

Files
File name Status Uploaded Type Edit Remove
convert_export.txt ganesh, 2020-07-04.15:21:45 text/plain
convert_export.txt ganesh, 2020-07-04.21:48:57 text/plain
fix-file-path-encoding-in-convert-export-command.dpatch dead bf, 2020-01-27.20:45:39 application/x-darcs-patch
fix-file-path-encoding-in-convert-export-command.dpatch bf, 2020-01-27.21:43:56 application/x-darcs-patch
fix_tests_convert_export.dpatch bf, 2020-07-04.21:23:11 text/plain
patch-preview.txt dead bf, 2020-01-27.20:45:39 text/x-darcs-patch
patch-preview.txt bf, 2020-01-27.21:43:56 text/x-darcs-patch
unnamed bf, 2020-01-27.20:45:39 text/plain
unnamed bf, 2020-01-27.21:43:56 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg21749 (view) Author: bf Date: 2020-01-27.20:45:39
1 patch for repository http://darcs.net/screened:

patch ad6b597f3cf2e6d2627dfb6446390b044ff09b65
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jan 27 20:23:54 CET 2020
  * fix file path encoding in convert export command
  
  This failed when the locale encoding is not UTF8, but file paths contain
  non-ASCII characters (such as tested in convert_export.sh). The reason is
  that we used anchorPath and then quoted the resulting String, which means we
  decodeLocale. Instead we have to use Darcs.Util.Path.flatten and do the
  quoting and escaping on the raw bytes. The test script was slightly extended
  to check that the quoting and escaping works as expected by git.
Attachments
msg21754 (view) Author: bf Date: 2020-01-27.21:43:56
Amended the patch to fix a mistake I made with the escaping. This was found
by git_quoted_filenames test. Also re-added escaping of '\r'; even if the
git docu doesn't explicitly require it I guess it can't hurt. Second patch
contains a few useful changes for diagnosis when the test fails.

2 patches for repository http://darcs.net/screened:

patch fe2dd87f789dfa0f6f4bbbc65cc9953a68c1e552
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jan 27 20:23:54 CET 2020
  * fix file path encoding in convert export command
  
  This failed when the locale encoding is not UTF8, but file paths contain
  non-ASCII characters (such as tested in convert_export.sh). The reason is
  that we used anchorPath and then quoted the resulting String, which means we
  decodeLocale. Instead we have to use Darcs.Util.Path.flatten and do the
  quoting and escaping on the raw bytes. The test script was slightly extended
  to check that the quoting and escaping works as expected by git.

patch e3ede20b474ab8fb0227dc5dbe1c860829a06a2e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jan 27 22:49:13 CET 2020
  * some small improvements for tests/git_quoted_filenames.sh
Attachments
msg21825 (view) Author: ganesh Date: 2020-02-15.20:32:28
Looks reasonable. I will check the test runs ok on Windows before pushing 
:-)
msg21830 (view) Author: ganesh Date: 2020-02-15.22:01:06
Perhaps unsurprisingly, convert_export fails on Windows. 
(git_quoted_filenames was already skipped on Windows)

I'll look more closely.
msg22128 (view) Author: ganesh Date: 2020-07-04.15:21:45
Sorry for failing to follow up at all so far. I've attached the 
failing output for now.
Attachments
msg22130 (view) Author: bf Date: 2020-07-04.20:12:15
> I've attached the failing output for now.

The error is obviously unrelated to darcs (and thus to the patch):

| echo 'Liebe Grüße' > e/'"Liebe\ Grüße'
| + echo 'Liebe Grüße'
| test: line 41: e/"Liebe\ Grüße: No such file or directory

This cannot have succeeded before the patch. Something must have changed
in either your cygwin's bash or Windows that makes the above command fail.
msg22131 (view) Author: ganesh Date: 2020-07-04.20:17:35
I've just rerun it in the same shell, with and without

  * fix file path encoding in convert export command

and it definitely succeeds without it and fails with it.
msg22133 (view) Author: bf Date: 2020-07-04.21:23:12
I had forgotten that part of the patch was a change to the test, which I
wanted to make a bit more challenging. I had not expected the
redirection to a file with a strange name to fail on Windows. A fix that
rolls back the changes to the test script is attached.
Attachments
msg22134 (view) Author: ganesh Date: 2020-07-04.21:48:57
I get a different error now, attached.
Attachments
msg22135 (view) Author: bf Date: 2020-07-05.07:12:47
I should have known better. On Windows, specifically NTFS, file names
are stored using UTF-16. This is a two-byte encoding that is not ASCII
compatible. It is thus not valid to operate on the raw byte strings in
this way on Windows. Since the particular problem this patch was
supposed to fix is an anomaly anyway and the solution works only on
*nix, I decided that this patch is bad. I will obliterate it and mark
the bundle as rejected.
History
Date User Action Args
2020-01-27 20:45:39bfcreate
2020-01-27 21:43:56bfsetfiles: + patch-preview.txt, fix-file-path-encoding-in-convert-export-command.dpatch, unnamed
messages: + msg21754
2020-01-29 08:11:41bfsetstatus: needs-screening -> needs-review
2020-02-15 20:32:28ganeshsetstatus: needs-review -> accepted-pending-tests
messages: + msg21825
2020-02-15 22:01:06ganeshsetstatus: accepted-pending-tests -> review-in-progress
assignedto: ganesh
messages: + msg21830
nosy: + ganesh
2020-07-04 15:21:46ganeshsetfiles: + convert_export.txt
messages: + msg22128
2020-07-04 20:12:16bfsetmessages: + msg22130
2020-07-04 20:17:35ganeshsetmessages: + msg22131
2020-07-04 21:23:12bfsetfiles: + fix_tests_convert_export.dpatch
messages: + msg22133
2020-07-04 21:48:58ganeshsetfiles: + convert_export.txt
messages: + msg22134
2020-07-05 07:12:48bfsetmessages: + msg22135
2020-07-05 07:13:27bfsetstatus: review-in-progress -> rejected