You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alan Wood <Al...@clear.net.nz> on 2011/04/18 13:04:30 UTC
[PATCH] to help benchmark.py run on windows
Hi devs,
I have just been looking at running the benchmarks and have got to the stage where I can
run it on windows.
This attached patch fixes three issues with the script:
1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
2) make sure no \ end up is repos url file://
3) at error handler to rmtree to handle the windows read-only files.
I have removed the "which svn" command, not sure how to do this so it is only removed on
windows as I'm not really a python person.
I hope this is useful.
I haven't provided a log message as I don't really think it can be applied without a bit of
editing.
Cheers
Alan Wood
Re: [PATCH] to help benchmark.py run on windows
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Apr 19, 2011 at 17:37, Alan Wood <Al...@clear.net.nz> wrote:
> On 19 Apr 2011 at 13:07, Greg Stein wrote:
>> >
>> > On Windows, the path returned by mkdtemp() is something like
>> >
>> > C:\users\billga~1\appdata\local\temp\tmpfoobar
>> >
>> > with no leading slash, so an extra slash makes the URL valid.
>> >
>> > The directory path could even have spaces in it, if the user wishes.
>> > For a geeky script like this, we don't have to be paranoid.
>>
>> I reviewed that portion of Alan's patch and omitted, for the reasons
>> Neels stated, but I also think the following is valid:
>>
>> file://C:/users/blah/blah/repos
>
> Not valid: the code goes off looking for a network machine called 'C:' and comes back some
> time later with an error.
> IIRC the text between the 2nd and 3rd slash is a machine name.
Alrighty.
>> Thus, I left out the introduction of a slash. Are you sure there is
>> supposed to be a third slash in there? My impression is that the
>> "third slash" is a result of the leading slash of an absolute path in
>> Unix. But for Windows, you start with the drive letter (tho you could
>> get a slash if you use a remote path).
>
> I suppose mkdtemp could come back with '\\servername\temp\blah\'. That would make a real
> mess. That may happen is the current drive was invalid, but so much else would fail that I
> can't really get worried about it.
hehe... yeah.
I just found this:
http://en.wikipedia.org/wiki/File_URI_scheme#Examples
While Wikipedia is not authoritative, I see no reason to distrust it.
I'll go ahead and apply your original suggestion. Thanks!
Cheers,
-g
Re: [PATCH] to help benchmark.py run on windows
Posted by Alan Wood <Al...@clear.net.nz>.
On 19 Apr 2011 at 13:07, Greg Stein wrote:
> >
> > On Windows, the path returned by mkdtemp() is something like
> >
> > C:\users\billga~1\appdata\local\temp\tmpfoobar
> >
> > with no leading slash, so an extra slash makes the URL valid.
> >
> > The directory path could even have spaces in it, if the user wishes.
> > For a geeky script like this, we don't have to be paranoid.
>
> I reviewed that portion of Alan's patch and omitted, for the reasons
> Neels stated, but I also think the following is valid:
>
> file://C:/users/blah/blah/repos
Not valid: the code goes off looking for a network machine called 'C:' and comes back some
time later with an error.
IIRC the text between the 2nd and 3rd slash is a machine name.
>
> Thus, I left out the introduction of a slash. Are you sure there is
> supposed to be a third slash in there? My impression is that the
> "third slash" is a result of the leading slash of an absolute path in
> Unix. But for Windows, you start with the drive letter (tho you could
> get a slash if you use a remote path).
I suppose mkdtemp could come back with '\\servername\temp\blah\'. That would make a real
mess. That may happen is the current drive was invalid, but so much else would fail that I
can't really get worried about it.
> Bert? Any insight here?
>
> Cheers,
> -g
Alan Wood
Napier
New Zealand
Phone +64 6 835 4505
Re: [PATCH] to help benchmark.py run on windows
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Apr 19, 2011 at 11:07, Stephen Butler <sb...@elego.de> wrote:
>
> On Apr 19, 2011, at 15:32 , Neels Hofmeyr wrote:
>
>> On Mon, 2011-04-18 at 19:47 -0400, Greg Stein wrote:
>>> Applied in r1094816.
>>>
>>> On Mon, Apr 18, 2011 at 18:44, Greg Stein <gs...@gmail.com> wrote:
>>>> On Mon, Apr 18, 2011 at 07:04, Alan Wood <Al...@clear.net.nz> wrote:
>>>>> Hi devs,
>>>>> I have just been looking at running the benchmarks and have got to the stage where I can
>>>>> run it on windows.
>>>>>
>>>>> This attached patch fixes three issues with the script:
>>>>> 1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
>>
>> This particular change is not necessary -- code extract with
>> annotations:
>>
>> base = tempfile.mkdtemp() # base == '/tmp/dir123'
>> repos = j(base, 'repos') # repos == '/tmp/dir123/repos'
>> file_url = 'file://%s' % repos # file_url == 'file://' + '/tmp/...'
>>
>> With your change, file_url becomes file:////tmp/..., which is still
>> valid, but nonsense :) (BTW, the script would not have worked if there
>> had been only two slashes.)
>
> On Windows, the path returned by mkdtemp() is something like
>
> C:\users\billga~1\appdata\local\temp\tmpfoobar
>
> with no leading slash, so an extra slash makes the URL valid.
>
> The directory path could even have spaces in it, if the user wishes.
> For a geeky script like this, we don't have to be paranoid.
I reviewed that portion of Alan's patch and omitted, for the reasons
Neels stated, but I also think the following is valid:
file://C:/users/blah/blah/repos
Thus, I left out the introduction of a slash. Are you sure there is
supposed to be a third slash in there? My impression is that the
"third slash" is a result of the leading slash of an absolute path in
Unix. But for Windows, you start with the drive letter (tho you could
get a slash if you use a remote path).
Bert? Any insight here?
Cheers,
-g
Re: [PATCH] to help benchmark.py run on windows
Posted by Neels Hofmeyr <ne...@elego.de>.
On Tue, 2011-04-19 at 17:07 +0200, Stephen Butler wrote:
> On Apr 19, 2011, at 15:32 , Neels Hofmeyr wrote:
>
> >>>>
> >>>> This attached patch fixes three issues with the script:
> >>>> 1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
> >
> > This particular change is not necessary -- code extract with
> > annotations:
> >
> > base = tempfile.mkdtemp() # base == '/tmp/dir123'
> > repos = j(base, 'repos') # repos == '/tmp/dir123/repos'
> > file_url = 'file://%s' % repos # file_url == 'file://' + '/tmp/...'
> >
> > With your change, file_url becomes file:////tmp/..., which is still
> > valid, but nonsense :) (BTW, the script would not have worked if there
> > had been only two slashes.)
>
> On Windows, the path returned by mkdtemp() is something like
>
> C:\users\billga~1\appdata\local\temp\tmpfoobar
>
> with no leading slash, so an extra slash makes the URL valid.
gotcha!
Thanks, Steve, I wasn't aware of that. Sorry, Alan, I am hopelessly
unix. Let me paraphrase. Hopefully unix.
So we need a special treatment. Like
file_url = 'file://'
if not base.startswith('/'):
file_url.append('/')
...or leave the four slashes, that's also fine, actually.
~Neels
Re: [PATCH] to help benchmark.py run on windows
Posted by Stephen Butler <sb...@elego.de>.
On Apr 19, 2011, at 15:32 , Neels Hofmeyr wrote:
> On Mon, 2011-04-18 at 19:47 -0400, Greg Stein wrote:
>> Applied in r1094816.
>>
>> On Mon, Apr 18, 2011 at 18:44, Greg Stein <gs...@gmail.com> wrote:
>>> On Mon, Apr 18, 2011 at 07:04, Alan Wood <Al...@clear.net.nz> wrote:
>>>> Hi devs,
>>>> I have just been looking at running the benchmarks and have got to the stage where I can
>>>> run it on windows.
>>>>
>>>> This attached patch fixes three issues with the script:
>>>> 1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
>
> This particular change is not necessary -- code extract with
> annotations:
>
> base = tempfile.mkdtemp() # base == '/tmp/dir123'
> repos = j(base, 'repos') # repos == '/tmp/dir123/repos'
> file_url = 'file://%s' % repos # file_url == 'file://' + '/tmp/...'
>
> With your change, file_url becomes file:////tmp/..., which is still
> valid, but nonsense :) (BTW, the script would not have worked if there
> had been only two slashes.)
On Windows, the path returned by mkdtemp() is something like
C:\users\billga~1\appdata\local\temp\tmpfoobar
with no leading slash, so an extra slash makes the URL valid.
The directory path could even have spaces in it, if the user wishes.
For a geeky script like this, we don't have to be paranoid.
Steve
>
> Thanks for your patch, Alan!
>
> ~Neels
>
>>>> 2) make sure no \ end up is repos url file://
>>>> 3) at error handler to rmtree to handle the windows read-only files.
>>>>
>>>> I have removed the "which svn" command, not sure how to do this so it is only removed on
>>>> windows as I'm not really a python person.
>>>> I hope this is useful.
>>>> I haven't provided a log message as I don't really think it can be applied without a bit of
>>>> editing.
>>>
>>> Yes, it will certainly be useful. Thanks!
>>>
>>> I'll fold your changes into the scripts. It is handy because I don't
>>> have a Windows machine for testing. Please go ahead and continue
>>> sending your patches to the list!
>>>
>>> (or if you'd like, we can offer you direct commit access to the
>>> suite1/ directory)
>>>
>>> Cheers,
>>> -g
>>>
>
>
--
Stephen Butler | Senior Consultant
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
tel: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Re: [PATCH] to help benchmark.py run on windows
Posted by Neels Hofmeyr <ne...@elego.de>.
On Mon, 2011-04-18 at 19:47 -0400, Greg Stein wrote:
> Applied in r1094816.
>
> On Mon, Apr 18, 2011 at 18:44, Greg Stein <gs...@gmail.com> wrote:
> > On Mon, Apr 18, 2011 at 07:04, Alan Wood <Al...@clear.net.nz> wrote:
> >> Hi devs,
> >> I have just been looking at running the benchmarks and have got to the stage where I can
> >> run it on windows.
> >>
> >> This attached patch fixes three issues with the script:
> >> 1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
This particular change is not necessary -- code extract with
annotations:
base = tempfile.mkdtemp() # base == '/tmp/dir123'
repos = j(base, 'repos') # repos == '/tmp/dir123/repos'
file_url = 'file://%s' % repos # file_url == 'file://' + '/tmp/...'
With your change, file_url becomes file:////tmp/..., which is still
valid, but nonsense :) (BTW, the script would not have worked if there
had been only two slashes.)
Thanks for your patch, Alan!
~Neels
> >> 2) make sure no \ end up is repos url file://
> >> 3) at error handler to rmtree to handle the windows read-only files.
> >>
> >> I have removed the "which svn" command, not sure how to do this so it is only removed on
> >> windows as I'm not really a python person.
> >> I hope this is useful.
> >> I haven't provided a log message as I don't really think it can be applied without a bit of
> >> editing.
> >
> > Yes, it will certainly be useful. Thanks!
> >
> > I'll fold your changes into the scripts. It is handy because I don't
> > have a Windows machine for testing. Please go ahead and continue
> > sending your patches to the list!
> >
> > (or if you'd like, we can offer you direct commit access to the
> > suite1/ directory)
> >
> > Cheers,
> > -g
> >
Re: [PATCH] to help benchmark.py run on windows
Posted by Greg Stein <gs...@gmail.com>.
Applied in r1094816.
On Mon, Apr 18, 2011 at 18:44, Greg Stein <gs...@gmail.com> wrote:
> On Mon, Apr 18, 2011 at 07:04, Alan Wood <Al...@clear.net.nz> wrote:
>> Hi devs,
>> I have just been looking at running the benchmarks and have got to the stage where I can
>> run it on windows.
>>
>> This attached patch fixes three issues with the script:
>> 1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
>> 2) make sure no \ end up is repos url file://
>> 3) at error handler to rmtree to handle the windows read-only files.
>>
>> I have removed the "which svn" command, not sure how to do this so it is only removed on
>> windows as I'm not really a python person.
>> I hope this is useful.
>> I haven't provided a log message as I don't really think it can be applied without a bit of
>> editing.
>
> Yes, it will certainly be useful. Thanks!
>
> I'll fold your changes into the scripts. It is handy because I don't
> have a Windows machine for testing. Please go ahead and continue
> sending your patches to the list!
>
> (or if you'd like, we can offer you direct commit access to the
> suite1/ directory)
>
> Cheers,
> -g
>
Re: [PATCH] to help benchmark.py run on windows
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Apr 18, 2011 at 07:04, Alan Wood <Al...@clear.net.nz> wrote:
> Hi devs,
> I have just been looking at running the benchmarks and have got to the stage where I can
> run it on windows.
>
> This attached patch fixes three issues with the script:
> 1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
> 2) make sure no \ end up is repos url file://
> 3) at error handler to rmtree to handle the windows read-only files.
>
> I have removed the "which svn" command, not sure how to do this so it is only removed on
> windows as I'm not really a python person.
> I hope this is useful.
> I haven't provided a log message as I don't really think it can be applied without a bit of
> editing.
Yes, it will certainly be useful. Thanks!
I'll fold your changes into the scripts. It is handy because I don't
have a Windows machine for testing. Please go ahead and continue
sending your patches to the list!
(or if you'd like, we can offer you direct commit access to the
suite1/ directory)
Cheers,
-g
Re: [PATCH] to help benchmark.py run on windows
Posted by Gavin Baumanis <ga...@thespidernet.com>.
Hi Alan,
Thanks for your patch.
Not too sure if you have been following the dev list discussions or not; but just in case you haven't been;
http://svn.haxx.se/dev/archive-2011-04/0243.shtml
Which would seem that Greg has been working on the same thing, making changes to the script to allow it to be portable across to a windows environment.
THat's not to say we're ignoring yours, it will get reviewed as soon as someone has time.
It was more an update for you in case you hadn't seen it already.
Gavin "Beau" Baumanis
On 18/04/2011, at 9:04 PM, Alan Wood wrote:
> Hi devs,
> I have just been looking at running the benchmarks and have got to the stage where I can
> run it on windows.
>
> This attached patch fixes three issues with the script:
> 1) use of file:// when I'm sure that file:/// is correct from previous discussions on this list
> 2) make sure no \ end up is repos url file://
> 3) at error handler to rmtree to handle the windows read-only files.
>
> I have removed the "which svn" command, not sure how to do this so it is only removed on
> windows as I'm not really a python person.
> I hope this is useful.
> I haven't provided a log message as I don't really think it can be applied without a bit of
> editing.
>
> Cheers
> Alan Wood
> <bench.patch>