threads / patch / 35165

patchremote-hg: unquote C-style paths when exporting

Subject: [PATCH] remote-hg: unquote C-style paths when exporting

## tl;dr

7 messages between Oct 18, 2013 and Oct 23, 2013. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Antoine Pelisse· Oct 18, 2013, 17:03 UTC · lore

git-fast-import documentation says that paths can be C-style quoted. Unfortunately, the current remote-hg helper doesn't unquote quoted path and pass them as-is to Mercurial when the commit is created.

This result in the following situation:
- clone a mercurial repository with git
- Add a file with space: `mkdir dir/foo\ bar`
- Commit that new file, and push the change to mercurial
- The mercurial repository as now a new directory named '"dir', which
contains a file named 'foo bar"'

Use python ast.literal_eval to unquote the string if it starts with ". It has been tested with quotes, spaces, and utf-8 encoded file-names.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 3 +++
 1 file changed, 3 insertions(+)
Show changes to contrib/remote-helpers/git-remote-hg +3 −0
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 92d994e..0141949 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -14,6 +14,7 @@
 
 from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util
 
+import ast
 import re
 import sys
 import os
@@ -742,6 +743,8 @@ def parse_commit(parser):
             f = { 'deleted' : True }
         else:
             die('Unknown file command: %s' % line)
+        if path.startswith('"'):
+            path = ast.literal_eval(path)
         files[path] = f
 
     # only export the commits if we are on an internal proxy repo
-- 
1.8.4.1.507.g9768648.dirty
Junio C Hamano· Oct 22, 2013, 19:13 UTC · re: Antoine Pelisse · lore

Re: [PATCH] remote-hg: unquote C-style paths when exporting

Antoine Pelisse <apelisse@gmail.com> writes:
Show 17 quoted lines
> git-fast-import documentation says that paths can be C-style quoted.
> Unfortunately, the current remote-hg helper doesn't unquote quoted
> path and pass them as-is to Mercurial when the commit is created.
>
> This result in the following situation:
>
> - clone a mercurial repository with git
> - Add a file with space: `mkdir dir/foo\ bar`
> - Commit that new file, and push the change to mercurial
> - The mercurial repository as now a new directory named '"dir', which
> contains a file named 'foo bar"'
>
> Use python ast.literal_eval to unquote the string if it starts with ".
> It has been tested with quotes, spaces, and utf-8 encoded file-names.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---

A path you read in fast-import input indeed needs to be unquoted when it begins with a dq, and I _think_ by using ast.literal_eval(), you probably can correctly unquote any valid C-quoted string.

But it bothers me somewhat that what the patch does seems to be overly broad. Doesn't ast.literal_eval() take a lot more than just strings?

    ast.literal_eval(node_or_string)
        Safely evaluate an expression node or a Unicode or Latin-1
        encoded string containing a Python expression. The string or
        node provided may only consist of the following Python literal
        structures: strings, numbers, tuples, lists, dicts, booleans,
        and None.

Also doesn't Python's double-quoted string have a lot more magic than C-quoted string, e.g.

	$ python -i
        >>> import ast
        >>> not_cq_path = '"abc" "def"'
        >>> ast.literal_eval(not_cq_path)
	'abcdef'
Show 24 quoted lines
>  contrib/remote-helpers/git-remote-hg | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index 92d994e..0141949 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -14,6 +14,7 @@
>  
>  from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util
>  
> +import ast
>  import re
>  import sys
>  import os
> @@ -742,6 +743,8 @@ def parse_commit(parser):
>              f = { 'deleted' : True }
>          else:
>              die('Unknown file command: %s' % line)
> +        if path.startswith('"'):
> +            path = ast.literal_eval(path)
>          files[path] = f
>  
>      # only export the commits if we are on an internal proxy repo
Antoine Pelisse· Oct 22, 2013, 20:49 UTC · re: Junio C Hamano · lore

Re: [PATCH] remote-hg: unquote C-style paths when exporting

On Tue, Oct 22, 2013 at 9:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> Antoine Pelisse <apelisse@gmail.com> writes:
>
>> git-fast-import documentation says that paths can be C-style quoted.
>> Unfortunately, the current remote-hg helper doesn't unquote quoted
>> path and pass them as-is to Mercurial when the commit is created.
>>
>> This result in the following situation:
>>
>> - clone a mercurial repository with git
>> - Add a file with space: `mkdir dir/foo\ bar`
Note to myself, mkdir doesn't create a "file"
Show 17 quoted lines
>> - Commit that new file, and push the change to mercurial
>> - The mercurial repository as now a new directory named '"dir', which
>> contains a file named 'foo bar"'
>>
>> Use python ast.literal_eval to unquote the string if it starts with ".
>> It has been tested with quotes, spaces, and utf-8 encoded file-names.
>>
>> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
>> ---
>
> A path you read in fast-import input indeed needs to be unquoted
> when it begins with a dq, and I _think_ by using ast.literal_eval(),
> you probably can correctly unquote any valid C-quoted string.
>
> But it bothers me somewhat that what the patch does seems to be
> overly broad.  Doesn't ast.literal_eval() take a lot more than just
> strings?
Good point
Show 7 quoted lines
>     ast.literal_eval(node_or_string)
>
>         Safely evaluate an expression node or a Unicode or Latin-1
>         encoded string containing a Python expression. The string or
>         node provided may only consist of the following Python literal
>         structures: strings, numbers, tuples, lists, dicts, booleans,
>         and None.

Fortunately, I don't believe any of the other type can start with a dq. So currently, I don't believe we can end-up with anything else but a string. We could certainly check that this is always true though.

Show 8 quoted lines
> Also doesn't Python's double-quoted string have a lot more magic
> than C-quoted string, e.g.
>
>         $ python -i
>         >>> import ast
>         >>> not_cq_path = '"abc" "def"'
>         >>> ast.literal_eval(not_cq_path)
>         'abcdef'

It is true that I have expected "valid output" from git-fast-export. And I don't have in mind any easy solution to detect that the output is broken, yet still accepted as a valid string by python. We could obviously write a unquote_c_style() equivalent in python if needed.

Thanks,
Felipe Contreras· Oct 23, 2013, 00:45 UTC · re: Antoine Pelisse · lore

Re: [PATCH] remote-hg: unquote C-style paths when exporting

On Tue, Oct 22, 2013 at 3:49 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> It is true that I have expected "valid output" from git-fast-export.
> And I don't have in mind any easy solution to detect that the output
> is broken, yet still accepted as a valid string by python. We could
> obviously write a unquote_c_style() equivalent in python if needed.
Something like this?
def c_style_unescape(string):
    if string[0] == string[-1] == '"':
        return string.decode('string-escape')[1:-1]
    return string
It's in git-remote-bzr.py.
-- 
Felipe Contreras
Antoine Pelisse· Oct 23, 2013, 08:38 UTC · re: Felipe Contreras · lore

Re: [PATCH] remote-hg: unquote C-style paths when exporting

On Wed, Oct 23, 2013 at 2:45 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 15 quoted lines
> On Tue, Oct 22, 2013 at 3:49 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>
>> It is true that I have expected "valid output" from git-fast-export.
>> And I don't have in mind any easy solution to detect that the output
>> is broken, yet still accepted as a valid string by python. We could
>> obviously write a unquote_c_style() equivalent in python if needed.
>
> Something like this?
>
> def c_style_unescape(string):
>     if string[0] == string[-1] == '"':
>         return string.decode('string-escape')[1:-1]
>     return string
>
> It's in git-remote-bzr.py.
Yeah, that's certainly better,
Thanks,
Junio C Hamano· Oct 23, 2013, 15:44 UTC · re: Antoine Pelisse · lore

Re* [PATCH] remote-hg: unquote C-style paths when exporting

Antoine Pelisse <apelisse@gmail.com> writes:
Show 10 quoted lines
>> def c_style_unescape(string):
>>     if string[0] == string[-1] == '"':
>>         return string.decode('string-escape')[1:-1]
>>     return string
>>
>> It's in git-remote-bzr.py.
>
> Yeah, that's certainly better,
>
> Thanks,
OK, so an amended one will look like this?
-- >8 --
From: Antoine Pelisse <apelisse@gmail.com>
Subject: remote-hg: unquote C-style paths when exporting

git-fast-import documentation says that paths can be C-style quoted. Unfortunately, the current remote-hg helper doesn't unquote quoted path and pass them as-is to Mercurial when the commit is created.

This result in the following situation:
- clone a mercurial repository with git
- Add a file with space: `mkdir dir/foo\ bar`
- Commit that new file, and push the change to mercurial
- The mercurial repository as now a new directory named '"dir', which
contains a file named 'foo bar"'

Use python ast.literal_eval to unquote the string if it starts with ". It has been tested with quotes, spaces, and utf-8 encoded file-names.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/remote-helpers/git-remote-hg | 6 ++++++
 1 file changed, 6 insertions(+)
Show changes to contrib/remote-helpers/git-remote-hg +6 −0
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 0194c67..85abbed 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -678,6 +678,11 @@ def get_merge_files(repo, p1, p2, files):
             f = { 'ctx' : repo[p1][e] }
             files[e] = f
 
+def c_style_unescape(string):
+    if string[0] == string[-1] == '"':
+        return string.decode('string-escape')[1:-1]
+    return string
+
 def parse_commit(parser):
     global marks, blob_marks, parsed_refs
     global mode
@@ -720,6 +725,7 @@ def parse_commit(parser):
             f = { 'deleted' : True }
         else:
             die('Unknown file command: %s' % line)
+        path = c_style_unescape(path).decode('utf-8')
         files[path] = f
 
     # only export the commits if we are on an internal proxy repo
Antoine Pelisse· Oct 23, 2013, 15:53 UTC · re: Junio C Hamano · lore

Re: Re* [PATCH] remote-hg: unquote C-style paths when exporting

On Wed, Oct 23, 2013 at 5:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> Antoine Pelisse <apelisse@gmail.com> writes:
>
>>> def c_style_unescape(string):
>>>     if string[0] == string[-1] == '"':
>>>         return string.decode('string-escape')[1:-1]
>>>     return string
>>>
>>> It's in git-remote-bzr.py.
>>
>> Yeah, that's certainly better,
>>
>> Thanks,
>
> OK, so an amended one will look like this?
The commit message needs to be updated as well.
Show 9 quoted lines
> -- >8 --
> From: Antoine Pelisse <apelisse@gmail.com>
> Subject: remote-hg: unquote C-style paths when exporting
>
> git-fast-import documentation says that paths can be C-style quoted.
> Unfortunately, the current remote-hg helper doesn't unquote quoted
> path and pass them as-is to Mercurial when the commit is created.
>
> This result in the following situation:
s/result/&s/
> - clone a mercurial repository with git
> - Add a file with space: `mkdir dir/foo\ bar`
- Add a file with space in a directory: `>dir/foo\ bar`
> - Commit that new file, and push the change to mercurial
> - The mercurial repository as now a new directory named '"dir', which
> contains a file named 'foo bar"'
I'm so ashamed I'd rather not report this one: s/as/has/
> Use python ast.literal_eval to unquote the string if it starts with ".

Use python str.decode('string-escape') to unquote the string if it starts and ends with ".

Show 32 quoted lines
> It has been tested with quotes, spaces, and utf-8 encoded file-names.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  contrib/remote-helpers/git-remote-hg | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index 0194c67..85abbed 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -678,6 +678,11 @@ def get_merge_files(repo, p1, p2, files):
>              f = { 'ctx' : repo[p1][e] }
>              files[e] = f
>
> +def c_style_unescape(string):
> +    if string[0] == string[-1] == '"':
> +        return string.decode('string-escape')[1:-1]
> +    return string
> +
>  def parse_commit(parser):
>      global marks, blob_marks, parsed_refs
>      global mode
> @@ -720,6 +725,7 @@ def parse_commit(parser):
>              f = { 'deleted' : True }
>          else:
>              die('Unknown file command: %s' % line)
> +        path = c_style_unescape(path).decode('utf-8')
>          files[path] = f
>
>      # only export the commits if we are on an internal proxy repo
That is consistent with git-remote-bzr,
Thanks

← back to recent threads