You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2011/04/19 00:44:48 UTC
Re: [PATCH] to help benchmark.py run on windows
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 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
>