particle-os/debian-koji
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

consolidate safe_rmtree, rmtree and shutil.rmtree

Browse source 
shutil.rmtree should be avoided in almost all cases
safe_rmtree has its usage in tasks module, but innards are replaced with
koji.util.rmtree, so we don't have two implementations of same task

Related: https://pagure.io/koji/issue/648
This commit is contained in:
Tomas Kopecek 2017-10-26 15:11:04 +02:00 • committed by Mike McLean
parent 70988911ea
commit 5b23fb1629
5 changed files with 256 additions and 82 deletions
Download patch file Download diff file Expand all files Collapse all files

2
hub/kojihub.py
View file

@ -4970,7 +4970,7 @@ def recycle_build(old, data):
update.execute()
builddir = koji.pathinfo.build(data)
if os.path.exists(builddir):
shutil.rmtree(builddir)
koji.util.rmtree(builddir)
koji.plugin.run_callbacks('postBuildStateChange', attribute='state',
old=old['state'], new=data['state'], info=data)

26
koji/tasks.py
View file

@ -84,26 +84,16 @@ def safe_rmtree(path, unmount=False, strict=True):
if not os.path.exists(path):
logger.debug("No such path: %s" % path)
return
#first rm -f non-directories
logger.debug('Scrubbing files in %s' % path)
rv = os.system("find '%s' -xdev \\! -type d -print0 |xargs -0 rm -f" % path)
msg = 'file removal failed (code %r) for %s' % (rv, path)
if rv != 0:
logger.warn(msg)
try:
koji.util.rmtree(path)
except:
logger.warn('file removal failed for %s' % path)
if strict:
raise koji.GenericError(msg)
else:
return rv
#them rmdir directories
#with -depth, we start at the bottom and work up
logger.debug('Scrubbing directories in %s' % path)
rv = os.system("find '%s' -xdev -depth -type d -print0 |xargs -0 rmdir" % path)
msg = 'dir removal failed (code %r) for %s' % (rv, path)
if rv != 0:
logger.warn(msg)
if strict:
raise koji.GenericError(msg)
return rv
raise
return 1
class ServerExit(Exception):
"""Raised to shutdown the server"""

5
plugins/hub/rpm2maven.py
View file

@ -8,6 +8,7 @@
import koji
from koji.context import context
from koji.plugin import callback
from koji.util import rmtree
import ConfigParser
import fnmatch
import os
@ -42,13 +43,13 @@ def maven_import(cbtype, *args, **kws):
tmpdir = os.path.join(koji.pathinfo.work(), 'rpm2maven', koji.buildLabel(buildinfo))
try:
if os.path.exists(tmpdir):
shutil.rmtree(tmpdir)
rmtree(tmpdir)
koji.ensuredir(tmpdir)
expand_rpm(filepath, tmpdir)
scan_and_import(buildinfo, rpminfo, tmpdir)
finally:
if os.path.exists(tmpdir):
shutil.rmtree(tmpdir)
rmtree(tmpdir)
def expand_rpm(filepath, tmpdir):
devnull = file('/dev/null', 'r+')

191
tests/test_lib/test_utils.py
View file

@ -1,7 +1,7 @@
from __future__ import absolute_import
import mock
import unittest
from mock import call
from mock import call, patch
import os
import optparse
@ -488,6 +488,195 @@ class MavenUtilTestCase(unittest.TestCase):
config.readfp(conf_file)
return config
class TestRmtree(unittest.TestCase):
@patch('koji.util._rmtree')
@patch('os.rmdir')
@patch('os.chdir')
@patch('os.getcwd')
@patch('stat.S_ISDIR')
@patch('os.lstat')
def test_rmtree_file(self, lstat, isdir, getcwd, chdir, rmdir, _rmtree):
""" Tests that the koji.util.rmtree function raises error when the
path parameter is not a directory.
"""
stat = mock.MagicMock()
stat.st_dev = 'dev'
lstat.return_value = stat
isdir.return_value = False
getcwd.return_value = 'cwd'
with self.assertRaises(koji.GenericError):
koji.util.rmtree('/mnt/folder/some_file')
_rmtree.assert_not_called()
rmdir.assert_not_called()
@patch('koji.util._rmtree')
@patch('os.rmdir')
@patch('os.chdir')
@patch('os.getcwd')
@patch('stat.S_ISDIR')
@patch('os.lstat')
def test_rmtree_directory(self, lstat, isdir, getcwd, chdir, rmdir, _rmtree):
""" Tests that the koji.util.rmtree function returns nothing when the path is a directory.
"""
stat = mock.MagicMock()
stat.st_dev = 'dev'
lstat.return_value = stat
isdir.return_value = True
getcwd.return_value = 'cwd'
path = '/mnt/folder'
self.assertEquals(koji.util.rmtree(path), None)
chdir.assert_called_with('cwd')
_rmtree.assert_called_once_with('dev')
rmdir.assert_called_once_with(path)
@patch('koji.util._rmtree')
@patch('os.rmdir')
@patch('os.chdir')
@patch('os.getcwd')
@patch('stat.S_ISDIR')
@patch('os.lstat')
def test_rmtree_directory_scrub_failure(self, lstat, isdir, getcwd, chdir, rmdir, _rmtree):
""" Tests that the koji.util.rmtree function returns a GeneralException
when the scrub of the files in the directory fails.
"""
stat = mock.MagicMock()
stat.st_dev = 'dev'
lstat.return_value = stat
isdir.return_value = True
getcwd.return_value = 'cwd'
path = '/mnt/folder'
_rmtree.side_effect = OSError('xyz')
with self.assertRaises(OSError):
koji.util.rmtree(path)
@patch('os.chdir')
@patch('os.rmdir')
@patch('koji.util._stripcwd')
def test_rmtree_internal_empty(self, stripcwd, rmdir, chdir):
dev = 'dev'
stripcwd.return_value = []
koji.util._rmtree(dev)
stripcwd.assert_called_once_with(dev)
rmdir.assert_not_called()
chdir.assert_not_called()
@patch('os.chdir')
@patch('os.rmdir')
@patch('koji.util._stripcwd')
def test_rmtree_internal_dirs(self, stripcwd, rmdir, chdir):
dev = 'dev'
stripcwd.side_effect = (['a', 'b'], [], [])
koji.util._rmtree(dev)
stripcwd.assert_has_calls([call(dev), call(dev), call(dev)])
rmdir.assert_has_calls([call('b'), call('a')])
chdir.assert_has_calls([call('b'), call('..'), call('a'), call('..')])
@patch('os.chdir')
@patch('os.rmdir')
@patch('koji.util._stripcwd')
def test_rmtree_internal_fail(self, stripcwd, rmdir, chdir):
dev = 'dev'
stripcwd.side_effect = (['a', 'b'], [], [])
rmdir.side_effect = OSError()
# don't fail on anything
koji.util._rmtree(dev)
stripcwd.assert_has_calls([call(dev), call(dev), call(dev)])
rmdir.assert_has_calls([call('b'), call('a')])
chdir.assert_has_calls([call('b'), call('..'), call('a'), call('..')])
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_empty(dev, unlink, isdir, lstat, listdir):
# simple empty directory
dev = 'dev'
listdir.return_value = []
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_not_called()
isdir.assert_not_called()
lstat.assert_not_called()
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_all(dev, unlink, isdir, lstat, listdir):
# test valid file + dir
dev = 'dev'
listdir.return_value = ['a', 'b']
st = mock.MagicMock()
st.st_dev = dev
st.st_mode = 'mode'
lstat.return_value = st
isdir.side_effect = [True, False]
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_called_once_with('b')
isdir.assert_has_calls([call('mode'), call('mode')])
lstat.assert_has_calls([call('a'), call('b')])
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_diffdev(dev, unlink, isdir, lstat, listdir):
# ignore files on different devices
dev = 'dev'
listdir.return_value = ['a', 'b']
st1 = mock.MagicMock()
st1.st_dev = dev
st1.st_mode = 'mode'
st2 = mock.MagicMock()
st2.st_dev = 'other_dev'
st2.st_mode = 'mode'
lstat.side_effect = [st1, st2]
isdir.side_effect = [True, False]
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_not_called()
isdir.assert_called_once_with('mode')
lstat.assert_has_calls([call('a'), call('b')])
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_fails(dev, unlink, isdir, lstat, listdir):
# ignore all unlink errors
dev = 'dev'
listdir.return_value = ['a', 'b']
st = mock.MagicMock()
st.st_dev = dev
st.st_mode = 'mode'
lstat.return_value = st
isdir.side_effect = [True, False]
unlink.side_effect = OSError()
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_called_once_with('b')
isdir.assert_has_calls([call('mode'), call('mode')])
lstat.assert_has_calls([call('a'), call('b')])
if __name__ == '__main__':
unittest.main()

114
tests/test_lib_py2only/test_tasks.py
View file

@ -128,66 +128,6 @@ class TasksTestCase(TestCase):
except koji.GenericError as e:
self.assertEquals(e.args[0], 'Unmounting incomplete: [\'/dev/shm\', \'/dev/mqueue\']')
@patch('os.path.isfile', return_value=True)
@patch('os.remove')
def test_safe_rmtree_file(self, mock_remove, mock_isfile):
""" Tests that the safe_rmtree function returns nothing when the path parameter is a file.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_file', False, True), None)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=True)
@patch('os.remove')
def test_safe_rmtree_link(self, mock_remove, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns nothing when the path parameter is a link.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_link', False, True), None)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=False)
def test_safe_rmtree_does_not_exist(self, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns nothing if the path does not exist.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_file', False, True), None)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
@patch('os.system', side_effect=[0, 0])
def test_safe_rmtree_directory(self, mock_os_system, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns nothing when the path is a directory.
"""
self.assertEquals(safe_rmtree('/mnt/folder', False, True), 0)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
@patch('os.system', side_effect=[1, 0])
def test_safe_rmtree_directory_scrub_file_failure(self, mock_os_system, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the files in the directory fails.
"""
try:
safe_rmtree('/mnt/folder', False, True)
raise Exception('A GenericError was not raised during the test')
except koji.GenericError as e:
self.assertEquals(e.args[0], 'file removal failed (code 1) for /mnt/folder')
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
@patch('os.system', side_effect=[0, 1])
def test_safe_rmtree_directory_scrub_directory_failure(self, mock_os_system, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the directories in the directory fails.
"""
try:
safe_rmtree('/mnt/folder', False, True)
raise Exception('A GenericError was not raised during the test')
except koji.GenericError as e:
self.assertEquals(e.args[0], 'dir removal failed (code 1) for /mnt/folder')
def test_BaseTaskHandler_handler_not_set(self):
""" Tests that an exception is thrown when the handler function is not overwritten by the child class.
"""
@ -759,3 +699,57 @@ class TasksTestCase(TestCase):
# getTaskResult should be called in 2nd round only for task 3, as 4
# will be skipped as 'canfail'
obj.session.getTaskResult.assert_has_calls([call(3)])
@patch('koji.util.rmtree')
def test_safe_rmtree_file(self, mock_rmtree):
""" Tests that the koji.util.rmtree function returns nothing when the path parameter is a file.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_file', False, True), None)
@patch('koji.util.rmtree')
def test_rmtree_link(self, rmtree):
""" Tests that the koji.util.rmtree function returns nothing when the path parameter is a link.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_link', False, True), None)
@patch('koji.util.rmtree')
def test_rmtree_does_not_exist(self, rmtree):
""" Tests that the koji.util.rmtree function returns nothing if the path does not exist.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_file', False, True), None)
@patch('koji.util.rmtree')
def test_rmtree_directory(self, rmtree):
""" Tests that the koji.util.rmtree function returns nothing when the path is a directory.
"""
self.assertEquals(safe_rmtree('/mnt/folder', False, True), None)
@patch('koji.util.rmtree')
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
def test_rmtree_directory_scrub_file_failure(self, exists, islink, isfile, rmtree):
""" Tests that the koji.util.rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the files in the directory fails.
"""
rmtree.side_effect = koji.GenericError('xyz')
try:
safe_rmtree('/mnt/folder', False, True)
raise Exception('A GenericError was not raised during the test')
except koji.GenericError as e:
self.assertEquals(e.args[0], 'xyz')
@patch('koji.util.rmtree')
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
def test_safe_rmtree_directory_scrub_directory_failure(self, exists, islink, isfile, rmtree):
""" Tests that the koji.util.rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the directories in the directory fails.
"""
rmtree.side_effect = OSError('xyz')
try:
safe_rmtree('/mnt/folder', False, True)
raise Exception('An OSError was not raised during the test')
except OSError as e:
self.assertEquals(e.args[0], 'xyz')