You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by James Abbatiello <ab...@gmail.com> on 2009/07/26 21:11:27 UTC

Re: [PATCH] Change test suite to read file properties in XML format [was: Re: cvs2svn test suite breakage caused by svn changes]

On Sun, Jul 26, 2009 at 12:02 PM, Michael Haggerty<mh...@alum.mit.edu> wrote:
> The only difference I would expect compared to the old code is that the
> old code handled end-of-lines unusually, which I presume was a
> side-effect of having to parse the text output of "svn proplist".  I
> think that what the new code does is the "right thing", but *if* there
> are problems they are likely to appear under Windows, which I don't have
> available for testing.

On Windows the old code produces:
>>> svntest.tree.get_props(['svntest'])
{'svntest': {'svn:ignore': '*.pyc\n*.o\n*~\n.*~\n\n'}}

And the new version produces:
>>> svntest.tree.get_props(['svntest'])
{u'svntest': {u'svn:ignore': u'*.pyc\r\n*.o\r\n*~\r\n.*~\r\n\r\n'}}

So it looks like some line-ending manipulation is still needed unless
the consumers are prepared to handle both forms.

-- 
James Abbatiello

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2375715


Re: [PATCH v2] Change test suite to read file properties in XML format

Posted by Julian Foad <ju...@btopenworld.com>.
On Mon, 2009-07-27 at 20:21 +0200, Michael Haggerty wrote:
> James Abbatiello wrote:
> > On Mon, Jul 27, 2009 at 11:15 AM, Michael Haggerty<mh...@alum.mit.edu> wrote:
> >> [...]
> >> But even though SVN outputs '\r\n' on Windows, I still don't understand
> >> how the '\r\n' characters get through in the test suite.
> > 
> > This had me stumped for a while too.
> > C:>svn --version
> > svn, version 1.6.3 (r38063)
> >    compiled Jun 18 2009, 12:57:17
> > ...
> > 
> > C:>svn proplist --verbose --xml svntest
> > <?xml version="1.0"?>
> > <properties>
> > <target
> >    path="svntest">
> > <property
> >    name="svn:ignore">*.pyc&#13;
> > *.o&#13;
> > *~&#13;
> > .*~&#13;
> > &#13;
> > </property>
> > </target>
> > </properties>
> > 
> > The '\r\n' sequences are being folded to '\n'.  But then the XML
> > parser turns '&#13\n' into '\r\n' in the final output.
> 
> Yuck.  I wonder whether the output contains '&#13;\n' or '&#13;\r\n'
> (i.e., does the output contain one or two extraneous CR characters?
> 
> In any case, I'm defeated.  Attached is a patch like the previous one,
> except that it also smashes all EOL combinations into '\n'.  Feedback is
> welcome, especially from Windows users.
> 
> Michael
> 
> [[[
> Read svn properties in XML rather than text format in test suite.
> 
> This makes the routine shorter and more robust to strange property
> values and to format changes in the text output format.

Please state the main goal of making this part of the test suite
compatible with the output of pre-1.6 svn, and the cvs2svn project's
interest in this. And this change doesn't really make it shorter if you
ignore the blank lines.

Thanks.

- Julian

> * subversion/tests/cmdline/svntest/tree.py
>   (get_props): Read svn properties via "svn proplist -v" using the
>   "--xml" option.
> ]]]

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2376142

Re: [PATCH v3] Change test suite to read file properties in XML format

Posted by James Abbatiello <ab...@gmail.com>.
On Tue, Jul 28, 2009 at 4:47 AM, Michael Haggerty<mh...@alum.mit.edu> wrote:
> This time I hand-tested against the output that you listed in your
> previous email, and it seems to work.  Third time's the charm?

Success!  It works for me.

-- 
James Abbatiello

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2376398


Re: [PATCH v3] Change test suite to read file properties in XML format

Posted by Julian Foad <ju...@btopenworld.com>.
Michael Haggerty wrote:
> [[[
> Read svn properties in XML rather than text format in test suite.
> 
> This makes the routine more robust to strange property values.  It also
> makes it immune to changes in the text output format, as happened
> between svn 1.5 and 1.6.  (This is useful for the cvs2svn project,
> which uses the svntest infrastructure but doesn't want to care what
> version of svn the user has installed.)
> 
> * subversion/tests/cmdline/svntest/tree.py
>   (get_props): Read svn properties via "svn proplist -v" using the
>   "--xml" option.
> ]]]

Nice log message. Thanks.

I have confirmed this version (patch v3) works in a trunk build (r38481)
on my Linux system. +1 to commit.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2376259

[PATCH v3] Change test suite to read file properties in XML format

Posted by Michael Haggerty <mh...@alum.mit.edu>.
Thanks, everybody, for the feedback.

James Abbatiello wrote:
> Sorry, it still doesn't work.
>>>> svntest.tree.get_props(["svntest"])
> {u'svntest': {u'svn:ignore': u'*.pyc\r\n*.o\r\n*~\r\n.*~\r\n\r\n'}}
> 
> You do the folding at the top of the function.  As you noted,
> wait_on_pipe() already does basically the same thing so that by the
> time the strings get to you they end with "&#13;\n".  And rstrip isn't
> going to match against encoded XML entities.  You're either going to
> have to match against "&#13;\n" at the end of the line (ick) or wait
> until after the XML parser has had a shot at it and adjust the line
> endings then.

Yes, of course you are right again.

This time I hand-tested against the output that you listed in your
previous email, and it seems to work.  Third time's the charm?

Julian Foad wrote:
> Please state the main goal of making this part of the test suite
> compatible with the output of pre-1.6 svn, and the cvs2svn project's
> interest in this. And this change doesn't really make it shorter if you
> ignore the blank lines.

Suggestion incorporated.

Hyrum K. Wright wrote:
>> I don't think there is anything precluding the use of "svn proplist
>> > --xml" for svntest's property-checking functions.
> 
> Julian Foad wrote
> Nor do I, just as long as we make sure to still test the "standard"  
> output somewhere in the testsuite.

I just intentionally broke the code for "svn proplist -v" (without
--xml) and several test cases in prop_test.py broke.  So there is at
least some level of testing aside from tree.get_props().

David Glasser wrote:
> The Python versions we support ship with an XML parser, right?

The xml.dom.minidom parser (which I'm using) was added in Python 2.0.

Michael

[[[
Read svn properties in XML rather than text format in test suite.

This makes the routine more robust to strange property values.  It also
makes it immune to changes in the text output format, as happened
between svn 1.5 and 1.6.  (This is useful for the cvs2svn project,
which uses the svntest infrastructure but doesn't want to care what
version of svn the user has installed.)

* subversion/tests/cmdline/svntest/tree.py
  (get_props): Read svn properties via "svn proplist -v" using the
  "--xml" option.
]]]

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2376241

Re: [PATCH v2] Change test suite to read file properties in XML format

Posted by James Abbatiello <ab...@gmail.com>.
On Mon, Jul 27, 2009 at 2:21 PM, Michael Haggerty<mh...@alum.mit.edu> wrote:
> James Abbatiello wrote:
>> The '\r\n' sequences are being folded to '\n'.  But then the XML
>> parser turns '&#13\n' into '\r\n' in the final output.
>
> Yuck.  I wonder whether the output contains '&#13;\n' or '&#13;\r\n'
> (i.e., does the output contain one or two extraneous CR characters?

>>> stdin, stdout, stderr, kid = svntest.main.open_pipe(["svn", "proplist", "--verbose", "--xml", "svntest"])
>>> stdout.read()
'<?xml version="1.0"?>\r\n<properties>\r\n<target\r\n
path="svntest">\r\n<property\r\n
name="svn:ignore">*.pyc&#13;\r\n*.o&#13;\r\n*~&#13;\r\n.*~&#13;\r\n&#13;\r\n</property>\r\n</target>\r\n</properties>\r\n'


> In any case, I'm defeated.  Attached is a patch like the previous one,
> except that it also smashes all EOL combinations into '\n'.  Feedback is
> welcome, especially from Windows users.

Sorry, it still doesn't work.
>>> svntest.tree.get_props(["svntest"])
{u'svntest': {u'svn:ignore': u'*.pyc\r\n*.o\r\n*~\r\n.*~\r\n\r\n'}}

You do the folding at the top of the function.  As you noted,
wait_on_pipe() already does basically the same thing so that by the
time the strings get to you they end with "&#13;\n".  And rstrip isn't
going to match against encoded XML entities.  You're either going to
have to match against "&#13;\n" at the end of the line (ick) or wait
until after the XML parser has had a shot at it and adjust the line
endings then.

-- 
James Abbatiello

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2376110


[PATCH v2] Change test suite to read file properties in XML format

Posted by Michael Haggerty <mh...@alum.mit.edu>.
James Abbatiello wrote:
> On Mon, Jul 27, 2009 at 11:15 AM, Michael Haggerty<mh...@alum.mit.edu> wrote:
>> [...]
>> But even though SVN outputs '\r\n' on Windows, I still don't understand
>> how the '\r\n' characters get through in the test suite.
> 
> This had me stumped for a while too.
> C:>svn --version
> svn, version 1.6.3 (r38063)
>    compiled Jun 18 2009, 12:57:17
> ...
> 
> C:>svn proplist --verbose --xml svntest
> <?xml version="1.0"?>
> <properties>
> <target
>    path="svntest">
> <property
>    name="svn:ignore">*.pyc&#13;
> *.o&#13;
> *~&#13;
> .*~&#13;
> &#13;
> </property>
> </target>
> </properties>
> 
> The '\r\n' sequences are being folded to '\n'.  But then the XML
> parser turns '&#13\n' into '\r\n' in the final output.

Yuck.  I wonder whether the output contains '&#13;\n' or '&#13;\r\n'
(i.e., does the output contain one or two extraneous CR characters?

In any case, I'm defeated.  Attached is a patch like the previous one,
except that it also smashes all EOL combinations into '\n'.  Feedback is
welcome, especially from Windows users.

Michael

[[[
Read svn properties in XML rather than text format in test suite.

This makes the routine shorter and more robust to strange property
values and to format changes in the text output format.

* subversion/tests/cmdline/svntest/tree.py
  (get_props): Read svn properties via "svn proplist -v" using the
  "--xml" option.
]]]

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2376090

Re: [PATCH] Change test suite to read file properties in XML format [was: Re: cvs2svn test suite breakage caused by svn changes]

Posted by James Abbatiello <ab...@gmail.com>.
On Mon, Jul 27, 2009 at 11:15 AM, Michael Haggerty<mh...@alum.mit.edu> wrote:
> James, thanks for testing this.  It would be trivial to add the
> EOL-fixing code to my patch, but first I want to understand it because I
> fear there is a deeper problem.
>
> After digging through more Subversion code I found that the handling of
> property values is strange (and not very consistent).  Even in the case
> of XML output, where I had expected the data to come out 1:1, SVN munges
> properties that start with "svn:" while they are being output.  I hadn't
> expected that.
>
> But even though SVN outputs '\r\n' on Windows, I still don't understand
> how the '\r\n' characters get through in the test suite.

This had me stumped for a while too.
C:>svn --version
svn, version 1.6.3 (r38063)
   compiled Jun 18 2009, 12:57:17
...

C:>svn proplist --verbose --xml svntest
<?xml version="1.0"?>
<properties>
<target
   path="svntest">
<property
   name="svn:ignore">*.pyc&#13;
*.o&#13;
*~&#13;
.*~&#13;
&#13;
</property>
</target>
</properties>

The '\r\n' sequences are being folded to '\n'.  But then the XML
parser turns '&#13\n' into '\r\n' in the final output.


> Perhaps there is a cygwin issue or something.  James, what does
>
>    python -c 'import sys; print sys.platform'
>
> print on your computer?

It prints win32.  I'm running windows python not cygwin python.

-- 
James Abbatiello

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2376062


Re: [PATCH] Change test suite to read file properties in XML format [was: Re: cvs2svn test suite breakage caused by svn changes]

Posted by Michael Haggerty <mh...@alum.mit.edu>.
James Abbatiello wrote:
> On Sun, Jul 26, 2009 at 12:02 PM, Michael Haggerty<mh...@alum.mit.edu> wrote:
>> The only difference I would expect compared to the old code is that the
>> old code handled end-of-lines unusually, which I presume was a
>> side-effect of having to parse the text output of "svn proplist".  I
>> think that what the new code does is the "right thing", but *if* there
>> are problems they are likely to appear under Windows, which I don't have
>> available for testing.
> 
> On Windows the old code produces:
>>>> svntest.tree.get_props(['svntest'])
> {'svntest': {'svn:ignore': '*.pyc\n*.o\n*~\n.*~\n\n'}}
> 
> And the new version produces:
>>>> svntest.tree.get_props(['svntest'])
> {u'svntest': {u'svn:ignore': u'*.pyc\r\n*.o\r\n*~\r\n.*~\r\n\r\n'}}
> 
> So it looks like some line-ending manipulation is still needed unless
> the consumers are prepared to handle both forms.

James, thanks for testing this.  It would be trivial to add the
EOL-fixing code to my patch, but first I want to understand it because I
fear there is a deeper problem.

After digging through more Subversion code I found that the handling of
property values is strange (and not very consistent).  Even in the case
of XML output, where I had expected the data to come out 1:1, SVN munges
properties that start with "svn:" while they are being output.  I hadn't
expected that.

But even though SVN outputs '\r\n' on Windows, I still don't understand
how the '\r\n' characters get through in the test suite.  The calling
sequence in the test suite goes like this (before my patch):

    tree.get_props(...):
      main.run_svn(1, ...):
        main.run_command(..., binary_mode=0):
          main.run_command_stdin(...):
            main.spawn_process(...):
              main.open_pipe()
              main.wait_on_pipe():
                if windows and not binary_mode:
                  stdout = stdout.replace('\r\n', '\n')
                  stderr = stderr.replace('\r\n', '\n')
                stdout_lines = stdout.splitlines(True)
                stderr_lines = stderr.splitlines(True)
            remove 'DBG:' lines from stdout

main.run_svn() explicitly sets binary_mode=0 when it calls
main.run_command(), and the "windows" variable is set by the following
code in main.py:

# Windows specifics
if sys.platform == 'win32':
  windows = True
  # ...
else:
  windows = False
  # ...

So, assuming that your python interpreter has sys.platform=='win32', the
end-of-lines should have all been normalized to '\n' in main.wait_on_pipe().

Perhaps there is a cygwin issue or something.  James, what does

    python -c 'import sys; print sys.platform'

print on your computer?