[PATCH] mock _init _umountall

[Paul B Schroeder]

On 07/01/2010 10:55 AM, Clark Williams wrote:
> On Wed, 30 Jun 2010 16:10:20 -0500
> Paul B Schroeder<paulbsch at vbridges.com>  wrote:
>
>> Here is an updated patch.  I found that _umountall was unmounting in the same order of the mounts and this was also causing some issues.  _umountall now unwinds the mounts by executing the umount commands in reverse order of the mounts.
>> I also went ahead an created a trac ticket:  https://fedorahosted.org/mock/ticket/8
>>
>> Cheers...Paul...
>>
>>
>
> Thanks Paul, just wanted to let you know that we're not *completely*
> ignoring you, just busy with release stuff. I'll merge this (or a
> tweaked version of it) and get it out probably just after the 4th.
>
> Clark
Great..  Thanks..  You probably noticed already, but just in case you 
didn't, I also provided a patch to implement the --scrub option we 
talked about way back when:
https://bugzilla.redhat.com/show_bug.cgi?id=450726

Cheers...Paul...


>
>> diff --git a/py/mock/backend.py b/py/mock/backend.py
>> index ca06f59..bad33b5 100644
>> --- a/py/mock/backend.py
>> +++ b/py/mock/backend.py
>> @@ -282,24 +282,24 @@ class Root(object):
>>               self._mountall()
>>               if self.chrootWasCleaned:
>>                   self._yum(self.chroot_setup_cmd, returnOutput=1)
>> -        finally:
>> -            self._umountall()
>>
>> -        # create user
>> -        self._makeBuildUser()
>> +            # create user
>> +            self._makeBuildUser()
>>
>> -        # create rpmbuild dir
>> -        self._buildDirSetup()
>> +            # create rpmbuild dir
>> +            self._buildDirSetup()
>>
>> -        # set up timezone to match host
>> -        localtimedir = self.makeChrootPath('etc')
>> -        localtimepath = self.makeChrootPath('etc', 'localtime')
>> -        if os.path.exists(localtimepath):
>> -            os.remove(localtimepath)
>> -        shutil.copy2('/etc/localtime', localtimedir)
>> +            # set up timezone to match host
>> +            localtimedir = self.makeChrootPath('etc')
>> +            localtimepath = self.makeChrootPath('etc', 'localtime')
>> +            if os.path.exists(localtimepath):
>> +                os.remove(localtimepath)
>> +            shutil.copy2('/etc/localtime', localtimedir)
>>
>> -        # done with init
>> -        self._callHooks('postinit')
>> +            # done with init
>> +            self._callHooks('postinit')
>> +        finally:
>> +            self._umountall()
>>
>>       decorate(traceLog())
>>       def _setupDev(self):
>> @@ -589,7 +589,10 @@ class Root(object):
>>       decorate(traceLog())
>>       def _umountall(self):
>>           """umount all mounted chroot fs."""
>> -        for cmd in self.umountCmds:
>> +        # Unwind mounts by umounting in the opposite order of the mounts
>> +        umountCmds = self.umountCmds
>> +        umountCmds.reverse()
>> +        for cmd in umountCmds:
>>               self.root_log.debug(cmd)
>>               mock.util.do(cmd, raiseExc=0, shell=True)
>>
>>
>>
>> On 06/25/2010 03:11 PM, Paul B Schroeder wrote:
>>> Hello all...
>>>
>>> Recently, I was doing some work which needed a more extensive /dev than the mock internal dev setup.  So I tried doing this in site-defaults.cfg:
>>> config_opts['internal_dev_setup'] = False
>>> config_opts['plugin_conf']['bind_mount_enable'] = True
>>> config_opts['plugin_conf']['bind_mount_opts']['dirs'].append(('/dev', '/dev' ))
>>>
>>> And discovered that mock chroot init was failing due _umountall getting called (in _init) before _makeBuildUser.  In _makeBuildUser there is a perl command which, for whatever reason, would fail due to /dev/null not being available at that point.
>>>
>>> At any rate, the following patch seems to take care of the issue.  And it would seem to be a better way to handle this in general as I would expect _umountall to be the last method called.
>>>
>>> It would be awesome if this or something similar could be applied..
>>>
>>> Thanks...Paul...
>>>
>>>
>>> diff --git a/py/mock/backend.py b/py/mock/backend.py
>>> index ca06f59..9df438b 100644
>>> --- a/py/mock/backend.py
>>> +++ b/py/mock/backend.py
>>> @@ -282,24 +282,24 @@ class Root(object):
>>>                self._mountall()
>>>                if self.chrootWasCleaned:
>>>                    self._yum(self.chroot_setup_cmd, returnOutput=1)
>>> -        finally:
>>> -            self._umountall()
>>>
>>> -        # create user
>>> -        self._makeBuildUser()
>>> +            # create user
>>> +            self._makeBuildUser()
>>>
>>> -        # create rpmbuild dir
>>> -        self._buildDirSetup()
>>> +            # create rpmbuild dir
>>> +            self._buildDirSetup()
>>>
>>> -        # set up timezone to match host
>>> -        localtimedir = self.makeChrootPath('etc')
>>> -        localtimepath = self.makeChrootPath('etc', 'localtime')
>>> -        if os.path.exists(localtimepath):
>>> -            os.remove(localtimepath)
>>> -        shutil.copy2('/etc/localtime', localtimedir)
>>> +            # set up timezone to match host
>>> +            localtimedir = self.makeChrootPath('etc')
>>> +            localtimepath = self.makeChrootPath('etc', 'localtime')
>>> +            if os.path.exists(localtimepath):
>>> +                os.remove(localtimepath)
>>> +            shutil.copy2('/etc/localtime', localtimedir)
>>>
>>> -        # done with init
>>> -        self._callHooks('postinit')
>>> +            # done with init
>>> +            self._callHooks('postinit')
>>> +        finally:
>>> +            self._umountall()
>>>
>>>        decorate(traceLog())
>>>        def _setupDev(self):
>>>
>>>
>>>
>>>
>>
>> --
>> ---
>> Paul B Schroeder
>> <paulbsch "at" vbridges "dot" com>
>> --
>> buildsys mailing list
>> buildsys at lists.fedoraproject.org
>> https://admin.fedoraproject.org/mailman/listinfo/buildsys

-- 
---
Paul B Schroeder
<paulbsch "at" vbridges "dot" com>