[patch] Patch to fix double call to "mock clean" in case, if mock will failed on "prep" stage.

[Alexandr Kanevskiy]

Hi all.

I found one small issue, that in case of mock will fail on prep stage
(no space on device, or repository access failed) mock will return
earlier, and in _status_prepping(), _watch_mock function will call
_start_cleanup(). After that _start_cleanup will change next state to be
"cleanup". But, this "if" below, will change next status to
"building"
regardless of previous error:
            if not self._mock_using_repo():
                self._status = 'building'

that meas scenario: 
	_status_prepping
		_watch_mock
      			mock clean
	_status_building
		_watch_mock
			mock clean

Attached patch solves that.
-- 
Alexandr Kanevskiy.

[Dan Williams]

On Sun, 2005-10-30 at 21:26 +0200, Alexandr Kanevskiy wrote:
>  Hi all.
>  
>  I found one small issue, that in case of mock will fail on prep stage
>  (no space on device, or repository access failed) mock will return
>  earlier, and in _status_prepping(), _watch_mock function will call
>  _start_cleanup(). After that _start_cleanup will change next state to be
>  "cleanup". But, this "if" below, will change next status to
> "building"
>  regardless of previous error:
>              if not self._mock_using_repo():
>                  self._status = 'building'
>  
>  that meas scenario: 
>  	_status_prepping
>  		_watch_mock
>        			mock clean
>  	_status_building
>  		_watch_mock
>  			mock clean
>  
>  Attached patch solves that. 
Good catch.  Thanks!  It's been committed to HEAD and STABLE_4.

Dan

[Alexandr Kanevskiy]

В Пнд, 31/10/2005 в 12:02 -0500, Dan Williams пишет:
>  On Sun, 2005-10-30 at 21:26 +0200, Alexandr Kanevskiy wrote:
>  > Attached patch solves that.
>  
>  Good catch.  Thanks!  It's been committed to HEAD and STABLE_4. 
Welcome ;)

Another findings from today looking at code: 
You have work_dir declared in global scope (line 847)
and use it from get_url_for_file(), line 62. 
In that function will be redefining of global variable. 
But expected to be local variable assignment.
Maybe rewrite that function to use _work_dir instead work_dir ?

The same thing is for 'hostname', 'cfg','port' in
get_url_for_file()...

or maybe it will be better that global scope code as 
def main():
     .....

and put at bottom:

if __name__ == '__main__':
	main()
?


More funny things with work_dir, are in BuilderMock.__init__(), line
102, and in BuilderMock.run(), line 438. 

I think, for BuilderMock class, all references to work_dir better to be
rewritten as "self._work_dir" ? 

And few more findings in import section:

import time - listed twice.
SimpleXMLRPCServer, xmlrpclib imported, but not used.

-- 
Alexandr Kanevskiy