From ba32ff57cb8f4a6e0eb4968918798b20c81d6bfb Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Tue, 20 Dec 2022 14:42:19 +0100 Subject: [PATCH 1/3] linkpac: Fix linking a locked package by not copying the lock to the target package --- behave/features/linkpac.feature | 13 +++++++++++++ osc/core.py | 6 +++++- 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 behave/features/linkpac.feature diff --git a/behave/features/linkpac.feature b/behave/features/linkpac.feature new file mode 100644 index 00000000..4a235a9a --- /dev/null +++ b/behave/features/linkpac.feature @@ -0,0 +1,13 @@ +Feature: `osc linkpac` command + + +# common steps for all scenarios +Background: + Given I set working directory to "{context.osc.temp}" + + +@destructive +Scenario: Run `osc linkpac on a locked package` + Given I execute osc with args "lock test:factory/test-pkgA" + When I execute osc with args "linkpac test:factory/test-pkgA home:Admin/test-pkgA" + Then the exit code is 0 diff --git a/osc/core.py b/osc/core.py index feef1515..0f054403 100644 --- a/osc/core.py +++ b/osc/core.py @@ -5451,7 +5451,8 @@ def checkout_package( def replace_pkg_meta( - pkgmeta, new_name: str, new_prj: str, keep_maintainers=False, dst_userid=None, keep_develproject=False + pkgmeta, new_name: str, new_prj: str, keep_maintainers=False, dst_userid=None, keep_develproject=False, + keep_lock: bool = False, ): """ update pkgmeta with new new_name and new_prj and set calling user as the @@ -5472,6 +5473,9 @@ def replace_pkg_meta( if not keep_develproject: for dp in root.findall('devel'): root.remove(dp) + if not keep_lock: + for node in root.findall("lock"): + root.remove(node) return ET.tostring(root, encoding=ET_ENCODING) From ce4cd4e4e9d3380e8538870aa3c0533158e1395c Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Tue, 20 Dec 2022 16:53:21 +0100 Subject: [PATCH 2/3] commandline: Add pop_project_package_targetproject_targetpackage_from_args() function --- osc/commandline.py | 39 +++++++++++ tests/test_commandline.py | 144 +++++++++++++++++++++++++++++++++++--- 2 files changed, 173 insertions(+), 10 deletions(-) diff --git a/osc/commandline.py b/osc/commandline.py index db6a1fe3..9032b805 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -204,6 +204,45 @@ def pop_project_package_repository_arch_from_args(args): return project, package, repository, arch +def pop_project_package_targetproject_targetpackage_from_args(args, target_package_is_optional=False): + """ + Get project, package, target_project and target_package from given `args`. + + :param args: List of command-line arguments. + WARNING: `args` gets modified in this function call! + :type args: list(str) + :param target_package_is_optional: Whether to error out when target package name cannot be retrieved. + :type target_package_is_optional: bool + :returns: Project name, package name, target project name and target package name. + :rtype: tuple(str) + """ + args_backup = args.copy() + #try_working_copy = True + + try_working_copy = True + try: + # try this sequence first: project package target_project target_package + project, package = pop_project_package_from_args(args, package_is_optional=False) + if args: + # we got more than 2 arguments -> we shouldn't try to retrieve project and package from a working copy + try_working_copy = False + target_project, target_package = pop_project_package_from_args( + args, package_is_optional=target_package_is_optional + ) + except oscerr.OscValueError as ex: + if not try_working_copy: + raise ex from None + # then read project and package from working copy and target_project target_package + args[:] = args_backup.copy() + project, package = pop_project_package_from_args( + [], default_project=".", default_package=".", package_is_optional=False + ) + target_project, target_package = pop_project_package_from_args( + args, package_is_optional=target_package_is_optional + ) + return project, package, target_project, target_package + + def ensure_no_remaining_args(args): if not args: return diff --git a/tests/test_commandline.py b/tests/test_commandline.py index 26021ccc..815a4bb8 100644 --- a/tests/test_commandline.py +++ b/tests/test_commandline.py @@ -5,6 +5,7 @@ import unittest from osc.commandline import pop_project_package_from_args from osc.commandline import pop_project_package_repository_arch_from_args +from osc.commandline import pop_project_package_targetproject_targetpackage_from_args from osc.commandline import pop_repository_arch_from_args from osc.oscerr import NoWorkingCopy, OscValueError from osc.store import Store @@ -40,7 +41,12 @@ class TestPopProjectPackageFromArgs(unittest.TestCase): def test_defaults(self): args = ["project"] - self.assertRaises(OscValueError, pop_project_package_from_args, args, default_package="default-package") + self.assertRaises( + OscValueError, + pop_project_package_from_args, + args, + default_package="default-package", + ) args = ["project"] project, package = pop_project_package_from_args( @@ -60,7 +66,10 @@ class TestPopProjectPackageFromArgs(unittest.TestCase): args = [] project, package = pop_project_package_from_args( - args, default_project="default-project", default_package="default-package", package_is_optional=True + args, + default_project="default-project", + default_package="default-package", + package_is_optional=True, ) self.assertEqual(project, "default-project") self.assertEqual(package, "default-package") @@ -102,7 +111,13 @@ class TestPopProjectPackageFromArgs(unittest.TestCase): self.assertEqual(args, []) args = [] - self.assertRaises(NoWorkingCopy, pop_project_package_from_args, args, default_project=".", default_package=".") + self.assertRaises( + NoWorkingCopy, + pop_project_package_from_args, + args, + default_project=".", + default_package=".", + ) args = [] project, package = pop_project_package_from_args( @@ -128,7 +143,9 @@ class TestPopProjectPackageFromArgs(unittest.TestCase): self.assertEqual(args, []) args = [] - project, package = pop_project_package_from_args(args, default_project=".", default_package=".") + project, package = pop_project_package_from_args( + args, default_project=".", default_package="." + ) self.assertEqual(project, "store_project") self.assertEqual(package, "store_package") self.assertEqual(args, []) @@ -189,7 +206,9 @@ class TestPopProjectPackageRepositoryArchFromArgs(unittest.TestCase): def test_individual_args(self): args = ["project", "package", "repo", "arch", "another-arg"] - project, package, repo, arch = pop_project_package_repository_arch_from_args(args) + project, package, repo, arch = pop_project_package_repository_arch_from_args( + args + ) self.assertEqual(project, "project") self.assertEqual(package, "package") self.assertEqual(repo, "repo") @@ -198,7 +217,9 @@ class TestPopProjectPackageRepositoryArchFromArgs(unittest.TestCase): def test_slash_separator(self): args = ["project/package", "repo/arch", "another-arg"] - project, package, repo, arch = pop_project_package_repository_arch_from_args(args) + project, package, repo, arch = pop_project_package_repository_arch_from_args( + args + ) self.assertEqual(project, "project") self.assertEqual(package, "package") self.assertEqual(repo, "repo") @@ -207,16 +228,22 @@ class TestPopProjectPackageRepositoryArchFromArgs(unittest.TestCase): def test_missing_arch(self): args = ["project", "package", "repo"] - self.assertRaises(OscValueError, pop_project_package_repository_arch_from_args, args) + self.assertRaises( + OscValueError, pop_project_package_repository_arch_from_args, args + ) def test_no_working_copy(self): args = ["repo", "arch"] - self.assertRaises(NoWorkingCopy, pop_project_package_repository_arch_from_args, args) + self.assertRaises( + NoWorkingCopy, pop_project_package_repository_arch_from_args, args + ) def test_working_copy(self): self._write_store("store_project", "store_package") args = ["repo", "arch"] - project, package, repo, arch = pop_project_package_repository_arch_from_args(args) + project, package, repo, arch = pop_project_package_repository_arch_from_args( + args + ) self.assertEqual(project, "store_project") self.assertEqual(package, "store_package") self.assertEqual(repo, "repo") @@ -226,7 +253,104 @@ class TestPopProjectPackageRepositoryArchFromArgs(unittest.TestCase): self._write_store("store_project", "store_package") args = ["repo", "arch", "another-arg"] # example of invalid usage, working copy is not used when there's 3+ args; [project, package, ...] are expected - self.assertRaises(OscValueError, pop_project_package_repository_arch_from_args, args) + self.assertRaises( + OscValueError, pop_project_package_repository_arch_from_args, args + ) + + +class TestPopProjectPackageTargetProjectTargetPackageFromArgs(unittest.TestCase): + def _write_store(self, project=None, package=None): + store = Store(self.tmpdir, check=False) + if project: + store.project = project + store.is_project = True + if package: + store.package = package + store.is_project = False + store.is_package = True + + def setUp(self): + self.tmpdir = tempfile.mkdtemp(prefix="osc_test") + os.chdir(self.tmpdir) + + def tearDown(self): + try: + shutil.rmtree(self.tmpdir) + except OSError: + pass + + def test_individual_args(self): + args = ["project", "package", "target-project", "target-package", "another-arg"] + project, package, target_project, target_package = pop_project_package_targetproject_targetpackage_from_args( + args + ) + self.assertEqual(project, "project") + self.assertEqual(package, "package") + self.assertEqual(target_project, "target-project") + self.assertEqual(target_package, "target-package") + self.assertEqual(args, ["another-arg"]) + + def test_slash_separator(self): + args = ["project/package", "target-project/target-package", "another-arg"] + project, package, target_project, target_package = pop_project_package_targetproject_targetpackage_from_args( + args + ) + self.assertEqual(project, "project") + self.assertEqual(package, "package") + self.assertEqual(target_project, "target-project") + self.assertEqual(target_package, "target-package") + self.assertEqual(args, ["another-arg"]) + + def test_missing_target_package(self): + args = ["project", "package", "target-project"] + project, package, target_project, target_package = pop_project_package_targetproject_targetpackage_from_args( + args, target_package_is_optional=True + ) + self.assertEqual(project, "project") + self.assertEqual(package, "package") + self.assertEqual(target_project, "target-project") + self.assertEqual(target_package, None) + self.assertEqual(args, []) + + def test_no_working_copy(self): + args = ["target-project", "target-package"] + self.assertRaises( + NoWorkingCopy, + pop_project_package_targetproject_targetpackage_from_args, + args, + ) + + def test_working_copy(self): + self._write_store("store_project", "store_package") + args = ["target-project", "target-package"] + project, package, target_project, target_package = pop_project_package_targetproject_targetpackage_from_args( + args + ) + self.assertEqual(project, "store_project") + self.assertEqual(package, "store_package") + self.assertEqual(target_project, "target-project") + self.assertEqual(target_package, "target-package") + + def test_working_copy_missing_target_package(self): + self._write_store("store_project", "store_package") + args = ["target-project"] + project, package, target_project, target_package = pop_project_package_targetproject_targetpackage_from_args( + args, target_package_is_optional=True + ) + self.assertEqual(project, "store_project") + self.assertEqual(package, "store_package") + self.assertEqual(target_project, "target-project") + self.assertEqual(target_package, None) + + def test_working_copy_extra_arg(self): + self._write_store("store_project", "store_package") + args = ["target-project", "target-package", "another-arg"] + # example of invalid usage, working copy is not used when there's 3+ args; [project, package, ...] are expected + self.assertRaises( + OscValueError, + pop_project_package_targetproject_targetpackage_from_args, + args, + ) if __name__ == "__main__": From 1b034921c8ef59d374e03d165bd7069c448d2de9 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Tue, 20 Dec 2022 16:57:36 +0100 Subject: [PATCH 3/3] linkpac: Improve command-line handling --- osc/commandline.py | 40 +++++++++++++++------------------------- osc/core.py | 12 +++++++++--- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/osc/commandline.py b/osc/commandline.py index 9032b805..aad58489 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -3037,10 +3037,10 @@ Please submit there instead, or use --nodevelproject to force direct submission. A linked package is a clone of another package, but plus local modifications. It can be cross-project. - The DESTPAC name is optional; the source packages' name will be used if - DESTPAC is omitted. + The TARGET_PACKAGE name is optional; the source packages' name will be used if + TARGET_PACKAGE is omitted. - Afterwards, you will want to 'checkout DESTPRJ DESTPAC'. + Afterwards, you will want to 'checkout TARGET_PROJECT TARGET_PACKAGE'. To add a patch, add the patch as file and add it to the _link file. You can also specify text which will be inserted at the top of the spec file. @@ -3052,30 +3052,24 @@ Please submit there instead, or use --nodevelproject to force direct submission. will be cleaned up automatically after it was submitted back. usage: - osc linkpac SOURCEPRJ SOURCEPAC DESTPRJ [DESTPAC] + osc linkpac PROJECT PACKAGE TARGET_PROJECT [TARGET_PACKAGE] + osc linkpac TARGET_PROJECT [TARGET_PACKAGE] # from a package checkout """ - - args = slash_split(args) apiurl = self.get_api_url() - if not args or len(args) < 3: - self.argparse_error("Incorrect number of arguments.") + args = list(args) + src_project, src_package, tgt_project, tgt_package = pop_project_package_targetproject_targetpackage_from_args( + args, target_package_is_optional=True, + ) + ensure_no_remaining_args(args) + + if not tgt_package: + tgt_package = src_package rev, dummy = parseRevisionOption(opts.revision) vrev = None - src_project = self._process_project_name(args[0]) - src_package = args[1] - dst_project = self._process_project_name(args[2]) - if len(args) > 3: - dst_package = args[3] - else: - dst_package = src_package - - if src_project == dst_project and src_package == dst_package: - raise oscerr.WrongArgs('Error: source and destination are the same.') - - if src_project == dst_project and not opts.cicount: + if src_project == tgt_project and not opts.cicount: # in this case, the user usually wants to build different spec # files from the same source opts.cicount = "copy" @@ -3086,12 +3080,8 @@ Please submit there instead, or use --nodevelproject to force direct submission. # vrev is only needed for srcmd5 and OBS instances < 2.1.17 do not support it vrev = None - if rev and not checkRevision(src_project, src_package, rev): - print('Revision \'%s\' does not exist' % rev, file=sys.stderr) - sys.exit(1) - link_pac( - src_project, src_package, dst_project, dst_package, opts.force, rev, opts.cicount, + src_project, src_package, tgt_project, tgt_package, opts.force, rev, opts.cicount, opts.disable_publish, opts.new_package, vrev, disable_build=opts.disable_build, ) diff --git a/osc/core.py b/osc/core.py index 0f054403..168e8655 100644 --- a/osc/core.py +++ b/osc/core.py @@ -5497,11 +5497,11 @@ def link_pac( dst_project: str, dst_package: str, force: bool, - rev="", - cicount="", + rev=None, + cicount=None, disable_publish=False, missing_target=False, - vrev="", + vrev=None, disable_build=False, ): """ @@ -5509,6 +5509,12 @@ def link_pac( - "src" is the original package - "dst" is the "link" package that we are creating here """ + if src_project == dst_project and src_package == dst_package: + raise oscerr.OscValueError("Cannot link package. Source and target are the same.") + + if rev and not checkRevision(src_project, src_package, rev): + raise oscerr.OscValueError(f"Revision doesn't exist: {rev}") + meta_change = False dst_meta = '' apiurl = conf.config['apiurl']