Re: One more issue in tests on Windows with Python 3

2020-05-26 Thread Yasuhito FUTATSUKI 
On 2020/05/26 15:23, Johan Corveleyn wrote:
> On Tue, May 26, 2020 at 4:14 AM Yasuhito FUTATSUKI  
> wrote:
>>
>> On 2020/05/26 5:52, Yasuhito FUTATSUKI wrote:
>>> On 2020/05/26 5:23, Johan Corveleyn wrote:
 On Thu, May 14, 2020 at 3:55 AM Yasuhito FUTATSUKI  
 wrote:
>>>
 Hmm, I still get this test failure, with trunk@today, when running
 update_tests.py with ra_serf, with Python 3.8.2 on Windows.

 [[[
 C:\research\svn\dev\trunk>python win-tests.py -c --log-level=DEBUG -p
 --httpd-dir=C:\Apache2.4.43 --httpd-port=1234 --httpd-no-log -t update
 ...
 FAIL:  update_tests.py 76: test filename with backslashes inside
 ]]]
>>
 Any ideas? Is there something wrong with the fix of r1877712? Or is
 there some other code path that runs into the same issue that wasn't
 addressed by r1877712?
>>>
>>> Thank you for the report.
>>>
>>> I found there still exist that uses raw 'const char *' value, in
>>> entries_dump() and tree_dump_dir(). So I'll fix them.
>>
>> I've made a patch address it. Could you please test the attached patch?
> 
> Yes, perfect. This fixes the update test. I will also apply this to my
> 1.14.0 while testing it for signing off on the release :-). Together
> with other applied patches, and backported fixes from trunk that
> should yield a fully successful test suite with Python 3 on Windows.

Thank you! Committed in r1878141.

I had waited for the detailed report of failure on Windows with Python 2,
reported in another thread "Re: SVN 1.14 release: please fix your buildbot",
but I had enough. So I'll commit other patches address the issues on
testing on Windows with Python 3 which I submitted, to go ahead.

Thanks,
-- 
Yasuhito FUTATSUKI /

Re: One more issue in tests on Windows with Python 3

2020-05-26 Thread Johan Corveleyn 
On Tue, May 26, 2020 at 4:14 AM Yasuhito FUTATSUKI  wrote:
>
> On 2020/05/26 5:52, Yasuhito FUTATSUKI wrote:
> > On 2020/05/26 5:23, Johan Corveleyn wrote:
> >> On Thu, May 14, 2020 at 3:55 AM Yasuhito FUTATSUKI  
> >> wrote:
> >
> >> Hmm, I still get this test failure, with trunk@today, when running
> >> update_tests.py with ra_serf, with Python 3.8.2 on Windows.
> >>
> >> [[[
> >> C:\research\svn\dev\trunk>python win-tests.py -c --log-level=DEBUG -p
> >> --httpd-dir=C:\Apache2.4.43 --httpd-port=1234 --httpd-no-log -t update
> >> ...
> >> FAIL:  update_tests.py 76: test filename with backslashes inside
> >> ]]]
>
> >> Any ideas? Is there something wrong with the fix of r1877712? Or is
> >> there some other code path that runs into the same issue that wasn't
> >> addressed by r1877712?
> >
> > Thank you for the report.
> >
> > I found there still exist that uses raw 'const char *' value, in
> > entries_dump() and tree_dump_dir(). So I'll fix them.
>
> I've made a patch address it. Could you please test the attached patch?

Yes, perfect. This fixes the update test. I will also apply this to my
1.14.0 while testing it for signing off on the release :-). Together
with other applied patches, and backported fixes from trunk that
should yield a fully successful test suite with Python 3 on Windows.

Thanks,
-- 
Johan

Re: One more issue in tests on Windows with Python 3

2020-05-25 Thread Yasuhito FUTATSUKI 
On 2020/05/26 5:52, Yasuhito FUTATSUKI wrote:
> On 2020/05/26 5:23, Johan Corveleyn wrote:
>> On Thu, May 14, 2020 at 3:55 AM Yasuhito FUTATSUKI  
>> wrote:
> 
>> Hmm, I still get this test failure, with trunk@today, when running
>> update_tests.py with ra_serf, with Python 3.8.2 on Windows.
>>
>> [[[
>> C:\research\svn\dev\trunk>python win-tests.py -c --log-level=DEBUG -p
>> --httpd-dir=C:\Apache2.4.43 --httpd-port=1234 --httpd-no-log -t update
>> ...
>> FAIL:  update_tests.py 76: test filename with backslashes inside
>> ]]]

>> Any ideas? Is there something wrong with the fix of r1877712? Or is
>> there some other code path that runs into the same issue that wasn't
>> addressed by r1877712?
> 
> Thank you for the report.
> 
> I found there still exist that uses raw 'const char *' value, in
> entries_dump() and tree_dump_dir(). So I'll fix them.

I've made a patch address it. Could you please test the attached patch?

Thanks,
-- 
Yasuhito FUTATSUKI /
Follow-up to r187712: entries-dump: Use escape string-typed value for two 
more char * type values.

* subversion/tests/cmdline/entries-dump.c
  (print_prefix): Add new Python function _to_str and then use it in
  Entry.set_strval.
  (print_as_bytes): New function.
  (str_value):
  - Quote human-readable value when print it.
  - Use print_as_bytes to print the value of "value" 
  (entries_dump):
  - Use iterpool in the loop to print each entry.
  - Use an escaped string for the key value of the "entries" dict object.
  (tree_dump_dir): Use an escaped string for the key value of the "dirs"
  dict object.

Reported by: jcorvel

Index: subversion/tests/cmdline/entries-dump.c
===
--- subversion/tests/cmdline/entries-dump.c (revision 1877963)
+++ subversion/tests/cmdline/entries-dump.c (working copy)
@@ -45,7 +45,14 @@
 static void
 print_prefix(void)
 {
-  puts("class Entry(object):\n"
+  puts("if b'' == '':\n"
+   "  def _to_str(s):\n"
+   "return s\n"
+   "else:\n"
+   "  def _to_str(s):\n"
+   "return s.decode('utf-8', 'surrogateescape')\n"
+   "\n"
+   "class Entry(object):\n"
"  \"\"\"An Entry object represents one node's entry in a pre-1.6"
" .svn/entries file.\n\n"
"Similar to #svn_wc_entry_t, but not all fields are populated.\n\n"
@@ -56,11 +63,22 @@
"  self.__setattr__(name, val)\n"
"  else:\n"
"def set_strval(self, name, val):\n"
-   "  self.__setattr__(name, val.decode('utf-8', "
-   "'surrogateescape'))\n");
+   "  global _to_str\n"
+   "  self.__setattr__(name, _to_str(val))\n");
 }
 
 static void
+print_as_bytes(const char *val)
+{
+  printf("b'");
+  while(*val)
+{
+  printf("\\x%02x", (unsigned int)(unsigned char)*val++);
+}
+  printf("'");
+}
+
+static void
 str_value(const char *name, const char *value, apr_pool_t *pool)
 {
   if (value == NULL)
@@ -72,19 +90,15 @@
 
   /* Print the human-readable value. */
   assert(NULL == strchr(escaped_value->data, '\n'));
-  printf("# e.%s = %s\n", name, escaped_value->data);
+  printf("# e.%s = '%s'\n", name, escaped_value->data);
 
   /* Print the machine-readable value. */
-  printf("e.set_strval('%s', b'", name);
-  while(*value)
-{
-  printf("\\x%02x", (unsigned int)(unsigned char)*value++);
-}  
-  printf("')\n");
+  printf("e.set_strval('%s', ", name);
+  print_as_bytes(value);
+  printf(")\n");
 }
 }
 
-
 static void
 int_value(const char *name, long int value)
 {
@@ -111,6 +125,7 @@
   svn_error_t *err;
   svn_wc_context_t *wc_ctx = NULL;
   const char *dir_abspath;
+  apr_pool_t *iterpool = svn_pool_create(pool); 
 
   SVN_ERR(svn_dirent_get_absolute(_abspath, dir_path, pool));
 
@@ -159,17 +174,18 @@
 
   for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
 {
+  svn_stringbuf_t *escaped_key;
   const char *key = apr_hash_this_key(hi);
   const svn_wc_entry_t *entry = apr_hash_this_val(hi);
 
+  svn_pool_clear(iterpool); 
   SVN_ERR_ASSERT(strcmp(key, entry->name) == 0);
-
   printf("e = Entry()\n");
-  str_value("name", entry->name, pool);
+  str_value("name", entry->name, iterpool);
   int_value("revision", entry->revision);
-  str_value("url", entry->url, pool);
-  str_value("repos", entry->repos, pool);
-  str_value("uuid", entry->uuid, pool);
+  str_value("url", entry->url, iterpool);
+  str_value("repos", entry->repos, iterpool);
+  str_value("uuid", entry->uuid, iterpool);
   int_value("kind", entry->kind);
   int_value("schedule", entry->schedule);
   bool_value("copied", entry->copied);
@@ -176,27 +192,27 @@
   bool_value("deleted", entry->deleted);
   bool_value("absent", entry->absent);
   bool_value("incomplete", entry->incomplete);
-  str_value("copyfrom_url", entry->copyfrom_url, pool);
+

Re: One more issue in tests on Windows with Python 3

2020-05-25 Thread Yasuhito FUTATSUKI 
On 2020/05/26 5:23, Johan Corveleyn wrote:
> On Thu, May 14, 2020 at 3:55 AM Yasuhito FUTATSUKI  
> wrote:

> Hmm, I still get this test failure, with trunk@today, when running
> update_tests.py with ra_serf, with Python 3.8.2 on Windows.
> 
> [[[
> C:\research\svn\dev\trunk>python win-tests.py -c --log-level=DEBUG -p
> --httpd-dir=C:\Apache2.4.43 --httpd-port=1234 --httpd-no-log -t update
> ...
> FAIL:  update_tests.py 76: test filename with backslashes inside
> ]]]
> 
> dav-fails.log contains:
> 
> [[[
> I: CMD: entries-dump.exe --tree-dump
> svn-test-work\working_copies\update_tests-76
> W: CWD: 
> R:\test_release-p--httpd-dirC__Apache2.4.43--httpd-port1234--httpd-no-log-tupdate\subversion\tests\cmdline
> Traceback (most recent call last):
>   File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\main.py",
> line 1927, in run
> rc = self.pred.run(sandbox)
>   File 
> "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\testcase.py",
> line 258, in run
> return self._delegate.run(sandbox)
>   File 
> "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\testcase.py",
> line 178, in run
> result = self.func(sandbox)
>   File "C:\research\svn\dev\trunk\subversion\tests\cmdline\update_tests.py",
> line 6382, in windows_update_backslash
> svntest.actions.run_and_verify_status(wc_dir, expected_status)
>   File 
> "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\actions.py",
> line 1605, in run_and_verify_status
> actual_entries = wc.State.from_entries(wc_dir_name)
>   File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\wc.py",
> line 744, in from_entries
> dump_data = svntest.main.run_entriesdump_tree(base)
>   File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\main.py",
> line 929, in run_entriesdump_tree
> exec(''.join(filter_dbg(stdout_lines)))
>   File "", line 280
> SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes
> in position 10-11: truncated \u escape
> FAIL:  update_tests.py 76: test filename with backslashes inside
> ]]]
> 
> See entire dav-fails.log in attachment.
> 
> Any ideas? Is there something wrong with the fix of r1877712? Or is
> there some other code path that runs into the same issue that wasn't
> addressed by r1877712?

Thank you for the report.

I found there still exist that uses raw 'const char *' value, in
entries_dump() and tree_dump_dir(). So I'll fix them.

Cheers,
-- 
Yasuhito FUTATSUKI /

Re: One more issue in tests on Windows with Python 3

2020-05-25 Thread Johan Corveleyn 
On Thu, May 14, 2020 at 3:55 AM Yasuhito FUTATSUKI  wrote:
>
> On 2020/05/13 9:57, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Wed, 13 May 2020 07:11 +0900:
> >> On 2020/05/10 1:24, Daniel Shahaf wrote:
> >>> Yasuhito FUTATSUKI wrote on Fri, 08 May 2020 20:55 +0900:
>  On 2020/05/08 2:46, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Thu, 07 May 2020 20:46 +0900:
> >> I think it is need to escape characters in char *value when we print
> ^some (not all)
> >> them as Python's str value. The patch below may work for this purpose,
> >> but I want someone to write more nice code :)
> >
> > How about simply adding the human-readable value in a comment? —
> 
>  It's very nice. One of the reason I don't like my code is just
>  readability of the value of "value".
> >>>
> >>> Sure, in general it's nice for protocols and serialization formats to
> >>> be texty, in order for them to be human-readable and -writable.  On
> >>> this instance, however, generating Python string literals that are both
> >>> correct and human-readable seems to me like it'd be an effort spent for
> >>> little gain.  (I think there's a good chance that no one will _ever_
> >>> run entries-dump by hand again once it properly supports Python 3.)
> >>>
> >>> One easy way to make the output nicer is to name the lambda function.
> >>
> >> Yes, it's also one of the reasons that it uses lambda function.
> >> I use it only to reduce the occurence of 'value' in
> >>
> >> e.name = value if isinstance(value, str) else value.decode()
> >>
> >> without using temporary named object, in the first patch.
> >>
> >> Then I updated the patch. Not to use lambda function, I added
> >> a method to set str attribute from bytes object to "Entry" class,
> >> and move its definition to the output of entries-dump to show
> >> what it does.
> >
> > Thanks.  Feel free to commit whichever variant you prefer.  They're all
> > functionally identical to one another, so whoever writes the code gets
> > to choose.
> >
> > Review:
>
> Thank you for the review. I've commited in 1877712, and I'm waiting
> for the results of buildbots.

Hmm, I still get this test failure, with trunk@today, when running
update_tests.py with ra_serf, with Python 3.8.2 on Windows.

[[[
C:\research\svn\dev\trunk>python win-tests.py -c --log-level=DEBUG -p
--httpd-dir=C:\Apache2.4.43 --httpd-port=1234 --httpd-no-log -t update
...
FAIL:  update_tests.py 76: test filename with backslashes inside
]]]

dav-fails.log contains:

[[[
I: CMD: entries-dump.exe --tree-dump
svn-test-work\working_copies\update_tests-76
W: CWD: 
R:\test_release-p--httpd-dirC__Apache2.4.43--httpd-port1234--httpd-no-log-tupdate\subversion\tests\cmdline
Traceback (most recent call last):
  File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\main.py",
line 1927, in run
rc = self.pred.run(sandbox)
  File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\testcase.py",
line 258, in run
return self._delegate.run(sandbox)
  File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\testcase.py",
line 178, in run
result = self.func(sandbox)
  File "C:\research\svn\dev\trunk\subversion\tests\cmdline\update_tests.py",
line 6382, in windows_update_backslash
svntest.actions.run_and_verify_status(wc_dir, expected_status)
  File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\actions.py",
line 1605, in run_and_verify_status
actual_entries = wc.State.from_entries(wc_dir_name)
  File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\wc.py",
line 744, in from_entries
dump_data = svntest.main.run_entriesdump_tree(base)
  File "C:\research\svn\dev\trunk\subversion\tests\cmdline\svntest\main.py",
line 929, in run_entriesdump_tree
exec(''.join(filter_dbg(stdout_lines)))
  File "", line 280
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes
in position 10-11: truncated \u escape
FAIL:  update_tests.py 76: test filename with backslashes inside
]]]

See entire dav-fails.log in attachment.

Any ideas? Is there something wrong with the fix of r1877712? Or is
there some other code path that runs into the same issue that wasn't
addressed by r1877712?

-- 
Johan


dav-fails.log
Description: Binary data

Re: One more issue in tests on Windows with Python 3

2020-05-13 Thread Yasuhito FUTATSUKI 
On 2020/05/13 9:57, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Wed, 13 May 2020 07:11 +0900:
>> On 2020/05/10 1:24, Daniel Shahaf wrote:
>>> Yasuhito FUTATSUKI wrote on Fri, 08 May 2020 20:55 +0900:  
 On 2020/05/08 2:46, Daniel Shahaf wrote:  
> Yasuhito FUTATSUKI wrote on Thu, 07 May 2020 20:46 +0900:
>> I think it is need to escape characters in char *value when we print
^some (not all)  
>> them as Python's str value. The patch below may work for this purpose,
>> but I want someone to write more nice code :)
>
> How about simply adding the human-readable value in a comment? —

 It's very nice. One of the reason I don't like my code is just
 readability of the value of "value".  
>>>
>>> Sure, in general it's nice for protocols and serialization formats to
>>> be texty, in order for them to be human-readable and -writable.  On
>>> this instance, however, generating Python string literals that are both
>>> correct and human-readable seems to me like it'd be an effort spent for
>>> little gain.  (I think there's a good chance that no one will _ever_
>>> run entries-dump by hand again once it properly supports Python 3.)
>>>
>>> One easy way to make the output nicer is to name the lambda function.  
>>
>> Yes, it's also one of the reasons that it uses lambda function.
>> I use it only to reduce the occurence of 'value' in
>>
>> e.name = value if isinstance(value, str) else value.decode()
>>
>> without using temporary named object, in the first patch.
>>
>> Then I updated the patch. Not to use lambda function, I added
>> a method to set str attribute from bytes object to "Entry" class,
>> and move its definition to the output of entries-dump to show
>> what it does.
> 
> Thanks.  Feel free to commit whichever variant you prefer.  They're all
> functionally identical to one another, so whoever writes the code gets
> to choose.
> 
> Review:

Thank you for the review. I've commited in 1877712, and I'm waiting
for the results of buildbots.

Cheers,
-- 
Yasuhito FUTATSUKI /

Re: One more issue in tests on Windows with Python 3

2020-05-12 Thread Daniel Shahaf 
Yasuhito FUTATSUKI wrote on Wed, 13 May 2020 07:11 +0900:
> On 2020/05/10 1:24, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Fri, 08 May 2020 20:55 +0900:  
> >> On 2020/05/08 2:46, Daniel Shahaf wrote:  
> >>> Yasuhito FUTATSUKI wrote on Thu, 07 May 2020 20:46 +0900:
>  I think it is need to escape characters in char *value when we print
> >>^some (not all)  
>  them as Python's str value. The patch below may work for this purpose,
>  but I want someone to write more nice code :)
> >>>
> >>> How about simply adding the human-readable value in a comment? —
> >>
> >> It's very nice. One of the reason I don't like my code is just
> >> readability of the value of "value".  
> > 
> > Sure, in general it's nice for protocols and serialization formats to
> > be texty, in order for them to be human-readable and -writable.  On
> > this instance, however, generating Python string literals that are both
> > correct and human-readable seems to me like it'd be an effort spent for
> > little gain.  (I think there's a good chance that no one will _ever_
> > run entries-dump by hand again once it properly supports Python 3.)
> > 
> > One easy way to make the output nicer is to name the lambda function.  
> 
> Yes, it's also one of the reasons that it uses lambda function.
> I use it only to reduce the occurence of 'value' in
> 
> e.name = value if isinstance(value, str) else value.decode()
> 
> without using temporary named object, in the first patch.
> 
> Then I updated the patch. Not to use lambda function, I added
> a method to set str attribute from bytes object to "Entry" class,
> and move its definition to the output of entries-dump to show
> what it does.

Thanks.  Feel free to commit whichever variant you prefer.  They're all
functionally identical to one another, so whoever writes the code gets
to choose.

Review:

> +++ subversion/tests/cmdline/entries-dump.c   (working copy)
> @@ -41,12 +43,41 @@
> +print_prefix()

Should this read "(void)" for C89 compatibility?  In trunk, empty parentheses
in a function definition occur only in platform-specific code and in the
definition of svn_swig_pl_get_current_pool(), if I'm grepping
correctly.  On the other hand, we have 127 instances of "(void)" in a
function definition.

>  {
> +  puts("class Entry(object):\n"
> +   "  \"Object represents pre-1.6 svn_wc_* entries "
> +   "used by entries-dump helper\"\n"

A couple of tweaks/additions to suggest:

"""An Entry object represents one node's entry in a pre-1.6 .svn/entries file.

Similar to #svn_wc_entry_t, but not all fields are populated.

Entry objects are generated by the 'entries-dump' test helper tool."""

> +   "  if b'' == '':\n"
> +   "def set_strval(self, name, val):\n"
> +   "  self.__setattr__(name, val)\n"
> +   "  else:\n"
> +   "def set_strval(self, name, val):\n"
> +   "  self.__setattr__(name, val.decode('utf-8', 
> 'surrogateescape'))");

Suggest to append a \n to the last line, even though puts() adds its own
trailing newline.  Adding our own would make a specific type of bug impossible:
it won't be possible for anyone to extend the string literal and forget to add
the trailing newline on the last incumbent line.

Cheers,

Daniel

Re: One more issue in tests on Windows with Python 3

2020-05-12 Thread Yasuhito FUTATSUKI 
On 2020/05/10 1:24, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Fri, 08 May 2020 20:55 +0900:
>> On 2020/05/08 2:46, Daniel Shahaf wrote:
>>> Yasuhito FUTATSUKI wrote on Thu, 07 May 2020 20:46 +0900:  
 I think it is need to escape characters in char *value when we print  
>>^some (not all)
 them as Python's str value. The patch below may work for this purpose,
 but I want someone to write more nice code :)  
>>>
>>> How about simply adding the human-readable value in a comment? —  
>>
>> It's very nice. One of the reason I don't like my code is just
>> readability of the value of "value".
> 
> Sure, in general it's nice for protocols and serialization formats to
> be texty, in order for them to be human-readable and -writable.  On
> this instance, however, generating Python string literals that are both
> correct and human-readable seems to me like it'd be an effort spent for
> little gain.  (I think there's a good chance that no one will _ever_
> run entries-dump by hand again once it properly supports Python 3.)
> 
> One easy way to make the output nicer is to name the lambda function.

Yes, it's also one of the reasons that it uses lambda function.
I use it only to reduce the occurence of 'value' in

e.name = value if isinstance(value, str) else value.decode()

without using temporary named object, in the first patch.

Then I updated the patch. Not to use lambda function, I added
a method to set str attribute from bytes object to "Entry" class,
and move its definition to the output of entries-dump to show
what it does.

Cheers,
-- 
Yasuhito FUTATSUKI /
Index: subversion/tests/cmdline/entries-dump.c
===
--- subversion/tests/cmdline/entries-dump.c (revision 1877407)
+++ subversion/tests/cmdline/entries-dump.c (working copy)
@@ -23,6 +23,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -34,6 +35,7 @@
 #include "svn_pools.h"
 #include "svn_wc.h"
 #include "svn_dirent_uri.h"
+#include "svn_xml.h"
 
 #include "private/svn_wc_private.h"
 
@@ -41,12 +43,41 @@
 #include "../../libsvn_wc/lock.h"
 
 static void
-str_value(const char *name, const char *value)
+print_prefix()
 {
+  puts("class Entry(object):\n"
+   "  \"Object represents pre-1.6 svn_wc_* entries "
+   "used by entries-dump helper\"\n"
+   "  if b'' == '':\n"
+   "def set_strval(self, name, val):\n"
+   "  self.__setattr__(name, val)\n"
+   "  else:\n"
+   "def set_strval(self, name, val):\n"
+   "  self.__setattr__(name, val.decode('utf-8', 'surrogateescape'))");
+}
+
+static void
+str_value(const char *name, const char *value, apr_pool_t *pool)
+{
   if (value == NULL)
 printf("e.%s = None\n", name);
   else
-printf("e.%s = '%s'\n", name, value);
+{
+  svn_stringbuf_t *escaped_value = NULL;
+  svn_xml_escape_attr_cstring(_value, value, pool);
+
+  /* Print the human-readable value. */
+  assert(NULL == strchr(escaped_value->data, '\n'));
+  printf("# e.%s = %s\n", name, escaped_value->data);
+
+  /* Print the machine-readable value. */
+  printf("e.set_strval('%s', b'", name);
+  while(*value)
+{
+  printf("\\x%02x", (unsigned int)(unsigned char)*value++);
+}  
+  printf("')\n");
+}
 }
 
 
@@ -130,11 +161,11 @@
   SVN_ERR_ASSERT(strcmp(key, entry->name) == 0);
 
   printf("e = Entry()\n");
-  str_value("name", entry->name);
+  str_value("name", entry->name, pool);
   int_value("revision", entry->revision);
-  str_value("url", entry->url);
-  str_value("repos", entry->repos);
-  str_value("uuid", entry->uuid);
+  str_value("url", entry->url, pool);
+  str_value("repos", entry->repos, pool);
+  str_value("uuid", entry->uuid, pool);
   int_value("kind", entry->kind);
   int_value("schedule", entry->schedule);
   bool_value("copied", entry->copied);
@@ -141,27 +172,27 @@
   bool_value("deleted", entry->deleted);
   bool_value("absent", entry->absent);
   bool_value("incomplete", entry->incomplete);
-  str_value("copyfrom_url", entry->copyfrom_url);
+  str_value("copyfrom_url", entry->copyfrom_url, pool);
   int_value("copyfrom_rev", entry->copyfrom_rev);
-  str_value("conflict_old", entry->conflict_old);
-  str_value("conflict_new", entry->conflict_new);
-  str_value("conflict_wrk", entry->conflict_wrk);
-  str_value("prejfile", entry->prejfile);
+  str_value("conflict_old", entry->conflict_old, pool);
+  str_value("conflict_new", entry->conflict_new, pool);
+  str_value("conflict_wrk", entry->conflict_wrk, pool);
+  str_value("prejfile", entry->prejfile, pool);
   /* skip: text_time */
   /* skip: prop_time */
   /* skip: checksum */
   int_value("cmt_rev", entry->cmt_rev);
   /* skip: cmt_date */
-  str_value("cmt_author", entry->cmt_author);
-

Re: One more issue in tests on Windows with Python 3

2020-05-09 Thread Daniel Shahaf 
Yasuhito FUTATSUKI wrote on Fri, 08 May 2020 20:55 +0900:
> On 2020/05/08 2:46, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Thu, 07 May 2020 20:46 +0900:  
> >> I think it is need to escape characters in char *value when we print  
>^some (not all)
> >> them as Python's str value. The patch below may work for this purpose,
> >> but I want someone to write more nice code :)  
> > 
> > How about simply adding the human-readable value in a comment? —  
> 
> It's very nice. One of the reason I don't like my code is just
> readability of the value of "value".

Sure, in general it's nice for protocols and serialization formats to
be texty, in order for them to be human-readable and -writable.  On
this instance, however, generating Python string literals that are both
correct and human-readable seems to me like it'd be an effort spent for
little gain.  (I think there's a good chance that no one will _ever_
run entries-dump by hand again once it properly supports Python 3.)

One easy way to make the output nicer is to name the lambda function.

> (It seems that this patch just presents a concept but isn't a actual
> code, though).

Precisely.  Sorry, I should've made that clear.

> I tweaked a condition to distinct py2 and py3, then make a patch.

+1

Thanks,

Daniel

Re: One more issue in tests on Windows with Python 3

2020-05-08 Thread Yasuhito FUTATSUKI 
On 2020/05/08 2:46, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Thu, 07 May 2020 20:46 +0900:
>> I think it is need to escape characters in char *value when we print
   ^some (not all)
>> them as Python's str value. The patch below may work for this purpose,
>> but I want someone to write more nice code :)
> 
> How about simply adding the human-readable value in a comment? —

It's very nice. One of the reason I don't like my code is just
readability of the value of "value". (It seems that this patch just
presents a concept but isn't a actual code, though).

> [[[



> +  printf("\\x%02x", (unsigned int)(unsigned char)*value++);



> ]]]
> 
> Also, I propose to change the cast as above, because I think the
> previous one wouldn't DTRT on platforms where 'char' is signed.

Ah, sure. My code was unsafe. Thank you.

I tweaked a condition to distinct py2 and py3, then make a patch.

Cheers,
-- 
Yasuhito FUTATSUKI /
Index: subversion/tests/cmdline/entries-dump.c
===
--- subversion/tests/cmdline/entries-dump.c (revision 1877407)
+++ subversion/tests/cmdline/entries-dump.c (working copy)
@@ -23,6 +23,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -34,6 +35,7 @@
 #include "svn_pools.h"
 #include "svn_wc.h"
 #include "svn_dirent_uri.h"
+#include "svn_xml.h"
 
 #include "private/svn_wc_private.h"
 
@@ -41,12 +43,28 @@
 #include "../../libsvn_wc/lock.h"
 
 static void
-str_value(const char *name, const char *value)
+str_value(const char *name, const char *value, apr_pool_t *pool)
 {
   if (value == NULL)
 printf("e.%s = None\n", name);
   else
-printf("e.%s = '%s'\n", name, value);
+{
+  svn_stringbuf_t *escaped_value = NULL;
+  svn_xml_escape_attr_cstring(_value, value, pool);
+
+  /* Print the human-readable value. */
+  assert(NULL == strchr(escaped_value->data, '\n'));
+  printf("# e.%s = %s\n", name, escaped_value->data);
+
+  /* Print the machine-readable value. */
+  printf("e.%s = (lambda x: x if b'' == '' else "
+ "x.decode('utf-8', 'surrogateescape'))(b'", name);
+  while(*value)
+{
+  printf("\\x%02x", (unsigned int)(unsigned char)*value++);
+}  
+  printf("')\n");
+}
 }
 
 
@@ -130,11 +148,11 @@
   SVN_ERR_ASSERT(strcmp(key, entry->name) == 0);
 
   printf("e = Entry()\n");
-  str_value("name", entry->name);
+  str_value("name", entry->name, pool);
   int_value("revision", entry->revision);
-  str_value("url", entry->url);
-  str_value("repos", entry->repos);
-  str_value("uuid", entry->uuid);
+  str_value("url", entry->url, pool);
+  str_value("repos", entry->repos, pool);
+  str_value("uuid", entry->uuid, pool);
   int_value("kind", entry->kind);
   int_value("schedule", entry->schedule);
   bool_value("copied", entry->copied);
@@ -141,27 +159,27 @@
   bool_value("deleted", entry->deleted);
   bool_value("absent", entry->absent);
   bool_value("incomplete", entry->incomplete);
-  str_value("copyfrom_url", entry->copyfrom_url);
+  str_value("copyfrom_url", entry->copyfrom_url, pool);
   int_value("copyfrom_rev", entry->copyfrom_rev);
-  str_value("conflict_old", entry->conflict_old);
-  str_value("conflict_new", entry->conflict_new);
-  str_value("conflict_wrk", entry->conflict_wrk);
-  str_value("prejfile", entry->prejfile);
+  str_value("conflict_old", entry->conflict_old, pool);
+  str_value("conflict_new", entry->conflict_new, pool);
+  str_value("conflict_wrk", entry->conflict_wrk, pool);
+  str_value("prejfile", entry->prejfile, pool);
   /* skip: text_time */
   /* skip: prop_time */
   /* skip: checksum */
   int_value("cmt_rev", entry->cmt_rev);
   /* skip: cmt_date */
-  str_value("cmt_author", entry->cmt_author);
-  str_value("lock_token", entry->lock_token);
-  str_value("lock_owner", entry->lock_owner);
-  str_value("lock_comment", entry->lock_comment);
+  str_value("cmt_author", entry->cmt_author, pool);
+  str_value("lock_token", entry->lock_token, pool);
+  str_value("lock_owner", entry->lock_owner, pool);
+  str_value("lock_comment", entry->lock_comment, pool);
   /* skip: lock_creation_date */
   /* skip: has_props */
   /* skip: has_prop_mods */
   /* skip: cachable_props */
   /* skip: present_props */
-  str_value("changelist", entry->changelist);
+  str_value("changelist", entry->changelist, pool);
   /* skip: working_size */
   /* skip: keep_local */
   int_value("depth", entry->depth);

Re: One more issue in tests on Windows with Python 3

2020-05-07 Thread Daniel Shahaf 
Yasuhito FUTATSUKI wrote on Thu, 07 May 2020 20:46 +0900:
> I think it is need to escape characters in char *value when we print
> them as Python's str value. The patch below may work for this purpose,
> but I want someone to write more nice code :)

How about simply adding the human-readable value in a comment? —

[[[
Index: subversion/tests/cmdline/entries-dump.c
===
--- subversion/tests/cmdline/entries-dump.c (revision 1877310)
+++ subversion/tests/cmdline/entries-dump.c (working copy)
@@ -46,7 +46,23 @@ str_value(const char *name, const char *value)
   if (value == NULL)
 printf("e.%s = None\n", name);
   else
-printf("e.%s = '%s'\n", name, value);
+{
+  svn_stringbuf_t *escaped_value;
+  SVN_ERR(svn_xml_escape_attr_cstring(_value, value, pool));
+
+  /* Print the human-readable value. */
+  assert(NULL == strchr(escaped_value->data, '\n'));
+  printf("# e.%s = %s\n", name, escaped_value->data);
+
+  /* Print the machine-readable value. */
+  printf("e.%s = (lambda x: x if isinstance(x, str) else "
+ "x.decode('utf-8', 'surrogateescape'))(b'", name);
+  while(*value)
+{
+  printf("\\x%02x", (unsigned int)(unsigned char)*value++);
+}  
+  printf("')\n");
+}
 }
 
 
]]]

Also, I propose to change the cast as above, because I think the
previous one wouldn't DTRT on platforms where 'char' is signed.

Cheers,

Daniel

One more issue in tests on Windows with Python 3

2020-05-07 Thread Yasuhito FUTATSUKI 
While I'm browsing faillog on svn-windows-ra buildbot, I found
another failure not to be reported here, on Windows with Python 3

>From 
>https://ci.apache.org/builders/svn-windows-ra/builds/2935/steps/Test%20fsfs%2Bserf/logs/faillog:
[[[
W: CWD: E:\svn-ra\tests\subversion\tests\cmdline
Traceback (most recent call last):
  File "D:\ra\svn-ra\build\subversion\tests\cmdline\svntest\main.py", line 
1931, in run
rc = self.pred.run(sandbox)
  File "D:\ra\svn-ra\build\subversion\tests\cmdline\svntest\testcase.py", line 
258, in run
return self._delegate.run(sandbox)
  File "D:\ra\svn-ra\build\subversion\tests\cmdline\svntest\testcase.py", line 
178, in run
result = self.func(sandbox)
  File "D:\ra\svn-ra\build\subversion\tests\cmdline\update_tests.py", line 
6393, in windows_update_backslash
svntest.actions.run_and_verify_status(wc_dir, expected_status)
  File "D:\ra\svn-ra\build\subversion\tests\cmdline\svntest\actions.py", line 
1605, in run_and_verify_status
actual_entries = wc.State.from_entries(wc_dir_name)
  File "D:\ra\svn-ra\build\subversion\tests\cmdline\svntest\wc.py", line 735, 
in from_entries
dump_data = svntest.main.run_entriesdump_tree(base)
  File "D:\ra\svn-ra\build\subversion\tests\cmdline\svntest\main.py", line 933, 
in run_entriesdump_tree
exec(''.join(filter_dbg(stdout_lines)))
  File "", line 201
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in 
position 10-11: truncated \u escape
FAIL:  update_tests.py 76: test filename with backslashes inside
]]]

It seems this is caused by evaluation of "e.name = 'completely\unusable\dir'"
line generated by subversion/tests/cmdline/entries-dump. This is acceptable
by Python 2.7, but not acceptable by Python 3.7.

e.g.
[[[
$ python2.7 -c "print(repr('completely\unusable\dir'))"
'completely\\unusable\\dir'
$ python3.7 -c "print(repr('completely\unusable\dir'))"
  File "", line 1
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in 
position 10-11: truncated \u escape
]]]

I think it is need to escape characters in char *value when we print
them as Python's str value. The patch below may work for this purpose,
but I want someone to write more nice code :)
[[[
Index: subversion/tests/cmdline/entries-dump.c
===
--- subversion/tests/cmdline/entries-dump.c (revision 1877407)
+++ subversion/tests/cmdline/entries-dump.c (working copy)
@@ -46,7 +46,15 @@
   if (value == NULL)
 printf("e.%s = None\n", name);
   else
-printf("e.%s = '%s'\n", name, value);
+{
+  printf("e.%s = (lambda x: x if isinstance(x, str) else "
+ "x.decode('utf-8', 'surrogateescape'))(b'", name);
+  while(*value)
+{
+  printf("\\x%02x", (int)*value++);
+}  
+  printf("')\n");
+}
 }


]]]

Cheers,
-- 
Yasuhito FUTATSUKI /