From 3a3a314f20e530e331c68e3b9afcc16a84f93f08 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jun 05 2018 08:03:34 +0000 Subject: [PATCH 1/2] use relative symlinks for hub imports Fixes: https://pagure.io/koji/issue/913 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 3087e88..19f5cc2 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -66,6 +66,16 @@ def log_error(msg): logger.error(msg) +def move_and_symlink(src, dst, relative=False, create_dir=False): + """Move src to dest and create symlink instead of original file""" + if create_dir: + koji.ensuredir(os.path.dirname(dst)) + safer_move(src, dst) + if relative: + dst = os.path.relpath(dst, os.path.dirname(src)) + os.symlink(dst, src) + + class Task(object): """A task for the build hosts""" @@ -5917,8 +5927,7 @@ def import_build_log(fn, buildinfo, subdir=None): raise koji.GenericError("Error importing build log. %s already exists." % final_path) if os.path.islink(fn) or not os.path.isfile(fn): raise koji.GenericError("Error importing build log. %s is not a regular file." % fn) - safer_move(fn, final_path) - os.symlink(final_path, fn) + move_and_symlink(fn, final_path, relative=True) def import_rpm_file(fn, buildinfo, rpminfo): """Move the rpm file into the proper place @@ -6363,9 +6372,7 @@ def _import_archive_file(filepath, destdir): raise koji.GenericError("Error importing archive file, %s already exists" % final_path) if os.path.islink(filepath) or not os.path.isfile(filepath): raise koji.GenericError("Error importing archive file, %s is not a regular file" % filepath) - koji.ensuredir(destdir) - safer_move(filepath, final_path) - os.symlink(final_path, filepath) + move_and_symlink(filepath, final_path, relative=True, create_dir=True) def _generate_maven_metadata(mavendir): """ @@ -8528,8 +8535,7 @@ def importImageInternal(task_id, build_id, imgdata): raise koji.GenericError("Error importing build log. %s already exists." % final_path) if os.path.islink(logsrc) or not os.path.isfile(logsrc): raise koji.GenericError("Error importing build log. %s is not a regular file." % logsrc) - safer_move(logsrc, final_path) - os.symlink(final_path, logsrc) + move_and_symlink(logsrc, final_path, relative=True, create_dir=True) # record all of the RPMs installed in the image(s) # verify they were built in Koji or in an external repo @@ -11800,8 +11806,7 @@ class HostExports(object): for relpath in [srpm] + rpms: fn = "%s/%s" % (uploadpath, relpath) dest = "%s/%s" % (dir, os.path.basename(fn)) - safer_move(fn, dest) - os.symlink(dest, fn) + move_and_symlink(fn, dest, relative=True) if logs: for key, files in logs.iteritems(): if key: @@ -11812,8 +11817,7 @@ class HostExports(object): for relpath in files: fn = "%s/%s" % (uploadpath, relpath) dest = "%s/%s" % (logdir, os.path.basename(fn)) - safer_move(fn, dest) - os.symlink(dest, fn) + move_and_symlink(fn, dest, relative=True) def moveMavenBuildToScratch(self, task_id, results, rpm_results): "Move a completed Maven scratch build into place (not imported)" @@ -11834,18 +11838,14 @@ class HostExports(object): relpath = filename src = os.path.join(koji.pathinfo.task(results['task_id']), relpath) dest = os.path.join(destdir, relpath) - koji.ensuredir(os.path.dirname(dest)) - safer_move(src, dest) - os.symlink(dest, src) + move_and_symlink(src, dest, relative=True, create_dir=True) if rpm_results: for relpath in [rpm_results['srpm']] + rpm_results['rpms'] + \ rpm_results['logs']: src = os.path.join(koji.pathinfo.task(rpm_results['task_id']), relpath) dest = os.path.join(destdir, 'rpms', relpath) - koji.ensuredir(os.path.dirname(dest)) - safer_move(src, dest) - os.symlink(dest, src) + move_and_symlink(src, dest, relative=True, create_dir=True) def moveWinBuildToScratch(self, task_id, results, rpm_results): "Move a completed Windows scratch build into place (not imported)" @@ -11861,18 +11861,14 @@ class HostExports(object): for relpath in results['output'].keys() + results['logs']: filename = os.path.join(koji.pathinfo.task(results['task_id']), relpath) dest = os.path.join(destdir, relpath) - koji.ensuredir(os.path.dirname(dest)) - safer_move(filename, dest) - os.symlink(dest, filename) + move_and_symlink(filename, dest, relative=True, create_dir=True) if rpm_results: for relpath in [rpm_results['srpm']] + rpm_results['rpms'] + \ rpm_results['logs']: filename = os.path.join(koji.pathinfo.task(rpm_results['task_id']), relpath) dest = os.path.join(destdir, 'rpms', relpath) - koji.ensuredir(os.path.dirname(dest)) - safer_move(filename, dest) - os.symlink(dest, filename) + move_and_symlink(filename, dest, relative=True, create_dir=True) def moveImageBuildToScratch(self, task_id, results): """move a completed image scratch build into place""" @@ -11893,10 +11889,8 @@ class HostExports(object): for img in sub_results['files'] + sub_results['logs']: src = os.path.join(workdir, img) dest = os.path.join(destdir, img) - koji.ensuredir(destdir) logger.debug('renaming %s to %s' % (src, dest)) - safer_move(src, dest) - os.symlink(dest, src) + move_and_symlink(src, dest, relative=True, create_dir=True) if 'rpmresults' in sub_results: rpm_results = sub_results['rpmresults'] for relpath in [rpm_results['srpm']] + rpm_results['rpms'] + \ @@ -11904,9 +11898,7 @@ class HostExports(object): src = os.path.join(koji.pathinfo.task( rpm_results['task_id']), relpath) dest = os.path.join(destdir, 'rpms', relpath) - koji.ensuredir(os.path.dirname(dest)) - safer_move(src, dest) - os.symlink(dest, src) + move_and_symlink(src, dest, relative=True, create_dir=True) def initBuild(self, data): """Create a stub (rpm) build entry. diff --git a/tests/test_hub/test_import_image_internal.py b/tests/test_hub/test_import_image_internal.py index 75dedf4..f159c5f 100644 --- a/tests/test_hub/test_import_image_internal.py +++ b/tests/test_hub/test_import_image_internal.py @@ -99,6 +99,7 @@ class TestImportImageInternal(unittest.TestCase): # Check that the log symlink made it to where it was supposed to. dest = os.readlink(workdir + '/foo.log') + dest = os.path.abspath(os.path.join(workdir, dest)) self.assertEquals(dest, self.tempdir + '/data/logs/image/foo.log') # And.. check all the sql statements diff --git a/tests/test_hub/test_move_and_symlink.py b/tests/test_hub/test_move_and_symlink.py new file mode 100644 index 0000000..24099e3 --- /dev/null +++ b/tests/test_hub/test_move_and_symlink.py @@ -0,0 +1,38 @@ +import mock +try: + import unittest2 as unittest +except ImportError: + import unittest + +import kojihub + +class TestMoveAndSymlink(unittest.TestCase): + @mock.patch('koji.ensuredir') + @mock.patch('kojihub.safer_move') + @mock.patch('os.symlink') + def test_valid(self, symlink, safer_move, ensuredir): + kojihub.move_and_symlink('/dir_a/src', '/dir_b/dst', relative=False, create_dir=False) + + ensuredir.assert_not_called() + safer_move.assert_called_once_with('/dir_a/src', '/dir_b/dst') + symlink.assert_called_once_with('/dir_b/dst', '/dir_a/src') + + @mock.patch('koji.ensuredir') + @mock.patch('kojihub.safer_move') + @mock.patch('os.symlink') + def test_valid_relative(self, symlink, safer_move, ensuredir): + kojihub.move_and_symlink('/a/src', '/b/dst', relative=True, create_dir=False) + + safer_move.assert_called_once_with('/a/src', '/b/dst') + symlink.assert_called_once_with('../b/dst', '/a/src') + ensuredir.assert_not_called() + + @mock.patch('koji.ensuredir') + @mock.patch('kojihub.safer_move') + @mock.patch('os.symlink') + def test_valid_create_dir(self, symlink, safer_move, ensuredir): + kojihub.move_and_symlink('a/src', 'b/dst', relative=True, create_dir=True) + + safer_move.assert_called_once_with('a/src', 'b/dst') + symlink.assert_called_once_with('../b/dst', 'a/src') + ensuredir.assert_called_once_with('b') From c27c24e3f793409c3d7be34c94dc09dcc88acd8b Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jun 05 2018 08:11:18 +0000 Subject: [PATCH 2/2] move move_and_symlink to koji.util --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 19f5cc2..7a4579a 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -57,6 +57,7 @@ import koji.xmlrpcplus from koji.context import context from koji.util import dslice from koji.util import md5_constructor +from koji.util import move_and_symlink from koji.util import multi_fnmatch from koji.util import safer_move from koji.util import sha1_constructor @@ -66,16 +67,6 @@ def log_error(msg): logger.error(msg) -def move_and_symlink(src, dst, relative=False, create_dir=False): - """Move src to dest and create symlink instead of original file""" - if create_dir: - koji.ensuredir(os.path.dirname(dst)) - safer_move(src, dst) - if relative: - dst = os.path.relpath(dst, os.path.dirname(src)) - os.symlink(dst, src) - - class Task(object): """A task for the build hosts""" @@ -5927,7 +5918,7 @@ def import_build_log(fn, buildinfo, subdir=None): raise koji.GenericError("Error importing build log. %s already exists." % final_path) if os.path.islink(fn) or not os.path.isfile(fn): raise koji.GenericError("Error importing build log. %s is not a regular file." % fn) - move_and_symlink(fn, final_path, relative=True) + move_and_symlink(fn, final_path) def import_rpm_file(fn, buildinfo, rpminfo): """Move the rpm file into the proper place @@ -6372,7 +6363,7 @@ def _import_archive_file(filepath, destdir): raise koji.GenericError("Error importing archive file, %s already exists" % final_path) if os.path.islink(filepath) or not os.path.isfile(filepath): raise koji.GenericError("Error importing archive file, %s is not a regular file" % filepath) - move_and_symlink(filepath, final_path, relative=True, create_dir=True) + move_and_symlink(filepath, final_path, create_dir=True) def _generate_maven_metadata(mavendir): """ @@ -8535,7 +8526,7 @@ def importImageInternal(task_id, build_id, imgdata): raise koji.GenericError("Error importing build log. %s already exists." % final_path) if os.path.islink(logsrc) or not os.path.isfile(logsrc): raise koji.GenericError("Error importing build log. %s is not a regular file." % logsrc) - move_and_symlink(logsrc, final_path, relative=True, create_dir=True) + move_and_symlink(logsrc, final_path, create_dir=True) # record all of the RPMs installed in the image(s) # verify they were built in Koji or in an external repo @@ -11806,7 +11797,7 @@ class HostExports(object): for relpath in [srpm] + rpms: fn = "%s/%s" % (uploadpath, relpath) dest = "%s/%s" % (dir, os.path.basename(fn)) - move_and_symlink(fn, dest, relative=True) + move_and_symlink(fn, dest) if logs: for key, files in logs.iteritems(): if key: @@ -11817,7 +11808,7 @@ class HostExports(object): for relpath in files: fn = "%s/%s" % (uploadpath, relpath) dest = "%s/%s" % (logdir, os.path.basename(fn)) - move_and_symlink(fn, dest, relative=True) + move_and_symlink(fn, dest) def moveMavenBuildToScratch(self, task_id, results, rpm_results): "Move a completed Maven scratch build into place (not imported)" @@ -11838,14 +11829,14 @@ class HostExports(object): relpath = filename src = os.path.join(koji.pathinfo.task(results['task_id']), relpath) dest = os.path.join(destdir, relpath) - move_and_symlink(src, dest, relative=True, create_dir=True) + move_and_symlink(src, dest, create_dir=True) if rpm_results: for relpath in [rpm_results['srpm']] + rpm_results['rpms'] + \ rpm_results['logs']: src = os.path.join(koji.pathinfo.task(rpm_results['task_id']), relpath) dest = os.path.join(destdir, 'rpms', relpath) - move_and_symlink(src, dest, relative=True, create_dir=True) + move_and_symlink(src, dest, create_dir=True) def moveWinBuildToScratch(self, task_id, results, rpm_results): "Move a completed Windows scratch build into place (not imported)" @@ -11861,14 +11852,14 @@ class HostExports(object): for relpath in results['output'].keys() + results['logs']: filename = os.path.join(koji.pathinfo.task(results['task_id']), relpath) dest = os.path.join(destdir, relpath) - move_and_symlink(filename, dest, relative=True, create_dir=True) + move_and_symlink(filename, dest, create_dir=True) if rpm_results: for relpath in [rpm_results['srpm']] + rpm_results['rpms'] + \ rpm_results['logs']: filename = os.path.join(koji.pathinfo.task(rpm_results['task_id']), relpath) dest = os.path.join(destdir, 'rpms', relpath) - move_and_symlink(filename, dest, relative=True, create_dir=True) + move_and_symlink(filename, dest, create_dir=True) def moveImageBuildToScratch(self, task_id, results): """move a completed image scratch build into place""" @@ -11890,7 +11881,7 @@ class HostExports(object): src = os.path.join(workdir, img) dest = os.path.join(destdir, img) logger.debug('renaming %s to %s' % (src, dest)) - move_and_symlink(src, dest, relative=True, create_dir=True) + move_and_symlink(src, dest, create_dir=True) if 'rpmresults' in sub_results: rpm_results = sub_results['rpmresults'] for relpath in [rpm_results['srpm']] + rpm_results['rpms'] + \ @@ -11898,7 +11889,7 @@ class HostExports(object): src = os.path.join(koji.pathinfo.task( rpm_results['task_id']), relpath) dest = os.path.join(destdir, 'rpms', relpath) - move_and_symlink(src, dest, relative=True, create_dir=True) + move_and_symlink(src, dest, create_dir=True) def initBuild(self, data): """Create a stub (rpm) build entry. diff --git a/koji/util.py b/koji/util.py index eaf7813..a3bcd48 100644 --- a/koji/util.py +++ b/koji/util.py @@ -449,6 +449,16 @@ def safer_move(src, dst): shutil.move(src, dst) +def move_and_symlink(src, dst, relative=True, create_dir=False): + """Move src to dest and create symlink instead of original file""" + if create_dir: + koji.ensuredir(os.path.dirname(dst)) + safer_move(src, dst) + if relative: + dst = os.path.relpath(dst, os.path.dirname(src)) + os.symlink(dst, src) + + def relpath(*args, **kwargs): deprecated("koji.util.relpath() is deprecated and will be removed in a " "future version. See: https://pagure.io/koji/issue/834") diff --git a/tests/test_hub/test_move_and_symlink.py b/tests/test_hub/test_move_and_symlink.py deleted file mode 100644 index 24099e3..0000000 --- a/tests/test_hub/test_move_and_symlink.py +++ /dev/null @@ -1,38 +0,0 @@ -import mock -try: - import unittest2 as unittest -except ImportError: - import unittest - -import kojihub - -class TestMoveAndSymlink(unittest.TestCase): - @mock.patch('koji.ensuredir') - @mock.patch('kojihub.safer_move') - @mock.patch('os.symlink') - def test_valid(self, symlink, safer_move, ensuredir): - kojihub.move_and_symlink('/dir_a/src', '/dir_b/dst', relative=False, create_dir=False) - - ensuredir.assert_not_called() - safer_move.assert_called_once_with('/dir_a/src', '/dir_b/dst') - symlink.assert_called_once_with('/dir_b/dst', '/dir_a/src') - - @mock.patch('koji.ensuredir') - @mock.patch('kojihub.safer_move') - @mock.patch('os.symlink') - def test_valid_relative(self, symlink, safer_move, ensuredir): - kojihub.move_and_symlink('/a/src', '/b/dst', relative=True, create_dir=False) - - safer_move.assert_called_once_with('/a/src', '/b/dst') - symlink.assert_called_once_with('../b/dst', '/a/src') - ensuredir.assert_not_called() - - @mock.patch('koji.ensuredir') - @mock.patch('kojihub.safer_move') - @mock.patch('os.symlink') - def test_valid_create_dir(self, symlink, safer_move, ensuredir): - kojihub.move_and_symlink('a/src', 'b/dst', relative=True, create_dir=True) - - safer_move.assert_called_once_with('a/src', 'b/dst') - symlink.assert_called_once_with('../b/dst', 'a/src') - ensuredir.assert_called_once_with('b') diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py index 226534a..b885976 100644 --- a/tests/test_lib/test_utils.py +++ b/tests/test_lib/test_utils.py @@ -1153,6 +1153,36 @@ class TestRmtree(unittest.TestCase): isdir.assert_has_calls([call('mode'), call('mode')]) lstat.assert_has_calls([call('a'), call('b')]) +class TestMoveAndSymlink(unittest.TestCase): + @mock.patch('koji.ensuredir') + @mock.patch('koji.util.safer_move') + @mock.patch('os.symlink') + def test_valid(self, symlink, safer_move, ensuredir): + koji.util.move_and_symlink('/dir_a/src', '/dir_b/dst', relative=False, create_dir=False) + + ensuredir.assert_not_called() + safer_move.assert_called_once_with('/dir_a/src', '/dir_b/dst') + symlink.assert_called_once_with('/dir_b/dst', '/dir_a/src') + + @mock.patch('koji.ensuredir') + @mock.patch('koji.util.safer_move') + @mock.patch('os.symlink') + def test_valid_relative(self, symlink, safer_move, ensuredir): + koji.util.move_and_symlink('/a/src', '/b/dst', relative=True, create_dir=False) + + safer_move.assert_called_once_with('/a/src', '/b/dst') + symlink.assert_called_once_with('../b/dst', '/a/src') + ensuredir.assert_not_called() + + @mock.patch('koji.ensuredir') + @mock.patch('koji.util.safer_move') + @mock.patch('os.symlink') + def test_valid_create_dir(self, symlink, safer_move, ensuredir): + koji.util.move_and_symlink('a/src', 'b/dst', relative=True, create_dir=True) + + safer_move.assert_called_once_with('a/src', 'b/dst') + symlink.assert_called_once_with('../b/dst', 'a/src') + ensuredir.assert_called_once_with('b') if __name__ == '__main__': unittest.main()