Discussion of the Zero Install software distribution system

Dave Abrahams | 30 Jun 2012 11:32

Re: [PATCH 2/2] Don't try to delete a directory tree from within that tree

Cleanup of some temp directory triggers it. Search for chdir followed by this call in the next 80 lines. If
you mean to fix the caller without applying this patch, that'd be a mistake IMO. This is a kind of
portability bug that you'll keep making otherwise, because it's so easy to do and is silent on Linux. 

Sent from my Q-42 Space Modulator

On Jun 30, 2012, at 4:48 AM, Thomas Leonard <talex5 <at> gmail.com> wrote:

> On 29 June 2012 18:51, Dave Abrahams <dave <at> boostpro.com> wrote:
>> ---
>>  zeroinstall/support/__init__.py |    4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/zeroinstall/support/__init__.py b/zeroinstall/support/__init__.py
>> index efe3b8a..c652362 100644
>> --- a/zeroinstall/support/__init__.py
>> +++ b/zeroinstall/support/__init__.py
>>  <at>  <at>  -76,6 +76,10  <at>  <at>  def ro_rmtree(root):
>>        import shutil
>>        import platform
>>        if platform.system() == 'Windows':
>> +               def true_path(x):
>> +                        return os.path.normcase(os.path.normpath(os.path.realpath(x)));
>> +               if true_path(os.getcwd()).startswith(true_path(root)):
>> +                        os.chdir(os.path.join(root,os.path.pardir))
>>                for main, dirs, files in os.walk(root):
>>                        for i in files + dirs:
>>                                os.chmod(os.path.join(main, i), 0o700)
> 
> What triggers this? Probably we should fix the caller.

(Continue reading)

headers

Thomas Leonard | 30 Jun 2012 12:48

Re: [PATCH 2/2] Don't try to delete a directory tree from within that tree

I don't see any calls to chdir in 0install:

0install$ git grep chdir
tests/testdistro.py:            archdir =
os.path.join(os.path.dirname(__file__), 'arch')
tests/testdistro.py:            arch = distro.ArchDistribution(archdir)

A library really shouldn't be changing the current directory, anyway.
Which program triggers it?

On 30 June 2012 10:32, Dave Abrahams <dave <at> boostpro.com> wrote:
> Cleanup of some temp directory triggers it. Search for chdir followed by this call in the next 80 lines. If
you mean to fix the caller without applying this patch, that'd be a mistake IMO. This is a kind of
portability bug that you'll keep making otherwise, because it's so easy to do and is silent on Linux.
>
> Sent from my Q-42 Space Modulator
>
> On Jun 30, 2012, at 4:48 AM, Thomas Leonard <talex5 <at> gmail.com> wrote:
>
>> On 29 June 2012 18:51, Dave Abrahams <dave <at> boostpro.com> wrote:
>>> ---
>>>  zeroinstall/support/__init__.py |    4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/zeroinstall/support/__init__.py b/zeroinstall/support/__init__.py
>>> index efe3b8a..c652362 100644
>>> --- a/zeroinstall/support/__init__.py
>>> +++ b/zeroinstall/support/__init__.py
>>>  <at>  <at>  -76,6 +76,10  <at>  <at>  def ro_rmtree(root):
>>>        import shutil

(Continue reading)

headers

Dave Abrahams | 30 Jun 2012 12:57

Re: [PATCH 2/2] Don't try to delete a directory tree from within that tree


on Sat Jun 30 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:

> I don't see any calls to chdir in 0install:
>
> 0install$ git grep chdir
> tests/testdistro.py:            archdir =
> os.path.join(os.path.dirname(__file__), 'arch')
> tests/testdistro.py:            arch = distro.ArchDistribution(archdir)
>
> A library really shouldn't be changing the current directory, anyway.
> Which program triggers it?

0compile; it's in autocompile.py

>
> On 30 June 2012 10:32, Dave Abrahams <dave <at> boostpro.com> wrote:
>> Cleanup of some temp directory triggers it. Search for chdir
>> followed by this call in the next 80 lines. If you mean to fix the
>> caller without applying this patch, that'd be a mistake IMO. This is
>> a kind of portability bug that you'll keep making otherwise, because
>> it's so easy to do and is silent on Linux.
>>
>> Sent from my Q-42 Space Modulator
>>
>> On Jun 30, 2012, at 4:48 AM, Thomas Leonard <talex5 <at> gmail.com> wrote:
>>
>>> On 29 June 2012 18:51, Dave Abrahams <dave <at> boostpro.com> wrote:
>>>> ---
>>>>  zeroinstall/support/__init__.py |    4 ++++

(Continue reading)

headers

Thomas Leonard | 12 Jul 2012 19:40

Re: [PATCH 2/2] Don't try to delete a directory tree from within that tree

On 30 June 2012 11:57, Dave Abrahams <dave <at> boostpro.com> wrote:
>
> on Sat Jun 30 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:
>
>> I don't see any calls to chdir in 0install:
>>
>> 0install$ git grep chdir
>> tests/testdistro.py:            archdir =
>> os.path.join(os.path.dirname(__file__), 'arch')
>> tests/testdistro.py:            arch = distro.ArchDistribution(archdir)
>>
>> A library really shouldn't be changing the current directory, anyway.
>> Which program triggers it?
>
> 0compile; it's in autocompile.py

OK, fixed. I also updated ro_rmtree to issue a warning if you try to
delete the current directory, so hopefully we'll spot these problems
in future.

--

-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware

(Continue reading)

headers

Dave Abrahams | 13 Jul 2012 21:56

Re: [PATCH 2/2] Don't try to delete a directory tree from within that tree


on Thu Jul 12 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:

> On 30 June 2012 11:57, Dave Abrahams <dave <at> boostpro.com> wrote:
>>
>> on Sat Jun 30 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:
>>
>>> I don't see any calls to chdir in 0install:
>>>
>>> 0install$ git grep chdir
>>> tests/testdistro.py:            archdir =
>>> os.path.join(os.path.dirname(__file__), 'arch')
>>> tests/testdistro.py:            arch = distro.ArchDistribution(archdir)
>>>
>>> A library really shouldn't be changing the current directory, anyway.
>>> Which program triggers it?
>>
>> 0compile; it's in autocompile.py
>
> OK, fixed. I also updated ro_rmtree to issue a warning if you try to
> delete the current directory, 

Or any of its ancestors, I presume.

> so hopefully we'll spot these problems in future.

Why only a warning?  Do you really want to allow code into the codebase
that's going to fail on Windows?

--

--

(Continue reading)

headers

Thomas Leonard | 14 Jul 2012 07:23

Re: [PATCH 2/2] Don't try to delete a directory tree from within that tree

On Jul 13, 2012 8:56 PM, "Dave Abrahams" <dave <at> boostpro.com> wrote:
>
>
> on Thu Jul 12 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:
>
> > On 30 June 2012 11:57, Dave Abrahams <dave <at> boostpro.com> wrote:
> >>
> >> on Sat Jun 30 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:
> >>
> >>> I don't see any calls to chdir in 0install:
> >>>
> >>> 0install$ git grep chdir
> >>> tests/testdistro.py: archdir =
> >>> os.path.join(os.path.dirname(__file__), 'arch')
> >>> tests/testdistro.py: arch = distro.ArchDistribution(archdir)
> >>>
> >>> A library really shouldn't be changing the current directory, anyway.
> >>> Which program triggers it?
> >>
> >> 0compile; it's in autocompile.py
> >
> > OK, fixed. I also updated ro_rmtree to issue a warning if you try to
> > delete the current directory,
>
> Or any of its ancestors, I presume.
>
> > so hopefully we'll spot these problems in future.
>
> Why only a warning? Do you really want to allow code into the codebase
> that's going to fail on Windows?

Making it an error would break existing software (including the released versions of 0compile).

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

_______________________________________________
Zero-install-devel mailing list
Zero-install-devel <at> lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/zero-install-devel

headers

Dave Abrahams | 15 Jul 2012 04:38

Re: [PATCH 2/2] Don't try to delete a directory tree from within that tree


on Sat Jul 14 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:

> On Jul 13, 2012 8:56 PM, "Dave Abrahams" <dave <at> boostpro.com> wrote:
>>
>>
>> on Thu Jul 12 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:
>>
>> > On 30 June 2012 11:57, Dave Abrahams <dave <at> boostpro.com> wrote:
>> >>
>> >> on Sat Jun 30 2012, Thomas Leonard <talex5-AT-gmail.com> wrote:
>> >>
>> >>> I don't see any calls to chdir in 0install:
>> >>>
>> >>> 0install$ git grep chdir
>> >>> tests/testdistro.py: archdir =
>> >>> os.path.join(os.path.dirname(__file__), 'arch')
>> >>> tests/testdistro.py: arch = distro.ArchDistribution(archdir)
>> >>>
>> >>> A library really shouldn't be changing the current directory, anyway.
>> >>> Which program triggers it?
>> >>
>> >> 0compile; it's in autocompile.py
>> >
>> > OK, fixed. I also updated ro_rmtree to issue a warning if you try to
>> > delete the current directory,
>>
>> Or any of its ancestors, I presume.
>>
>> > so hopefully we'll spot these problems in future.
>>
>> Why only a warning? Do you really want to allow code into the codebase
>> that's going to fail on Windows?
>
> Making it an error would break existing software (including the
> released versions of 0compile).

Ah. That would be bad.  Maybe it would be a good idea to make warnings
into errors in the test suite or something, though.

--

-- 
Dave Abrahams
BoostPro Computing
http://www.boostpro.com

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/