From 0e043e1e6b580c36af3dd12f4c21795877dc0f50 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 28 2018 21:27:58 +0000 Subject: PR#920: use relative symlinks for hub imports Merges #920 https://pagure.io/koji/pull-request/920 Fixes: #913 https://pagure.io/koji/issue/913 hub uses absolute symlinks for imported work files --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 2b267a5..3207b79 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -59,6 +59,7 @@ import koji.tasks 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 @@ -6036,8 +6037,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) def import_rpm_file(fn, buildinfo, rpminfo): """Move the rpm file into the proper place @@ -6482,9 +6482,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, create_dir=True) def _generate_maven_metadata(mavendir): """ @@ -8653,8 +8651,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, create_dir=True) # record all of the RPMs installed in the image(s) # verify they were built in Koji or in an external repo @@ -11938,8 +11935,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) if logs: for key, files in six.iteritems(logs): if key: @@ -11950,8 +11946,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) def moveMavenBuildToScratch(self, task_id, results, rpm_results): "Move a completed Maven scratch build into place (not imported)" @@ -11972,18 +11967,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, 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, create_dir=True) def moveWinBuildToScratch(self, task_id, results, rpm_results): "Move a completed Windows scratch build into place (not imported)" @@ -11999,18 +11990,14 @@ class HostExports(object): for relpath in to_list(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, 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, create_dir=True) def moveImageBuildToScratch(self, task_id, results): """move a completed image scratch build into place""" @@ -12031,10 +12018,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, create_dir=True) if 'rpmresults' in sub_results: rpm_results = sub_results['rpmresults'] for relpath in [rpm_results['srpm']] + rpm_results['rpms'] + \ @@ -12042,9 +12027,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, create_dir=True) def initBuild(self, data): """Create a stub (rpm) build entry. diff --git a/koji/util.py b/koji/util.py index fcc48c0..bf2e2b2 100644 --- a/koji/util.py +++ b/koji/util.py @@ -450,6 +450,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_import_image_internal.py b/tests/test_hub/test_import_image_internal.py index 2527d4d..5de7f9f 100644 --- a/tests/test_hub/test_import_image_internal.py +++ b/tests/test_hub/test_import_image_internal.py @@ -102,6 +102,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_lib/test_utils.py b/tests/test_lib/test_utils.py index 65c9377..02e5d32 100644 --- a/tests/test_lib/test_utils.py +++ b/tests/test_lib/test_utils.py @@ -1170,6 +1170,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()