Merge pull request #1207 from dmach/linkpac-locked

linkpac: Fix linking a locked package, improve CLI code

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

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
@@ -2998,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
+        The TARGET_PACKAGE name is optional; the source packages' name will be used if
-        DESTPAC is omitted.
+        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.
@@ -3013,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:
+        args = list(args)
-            self.argparse_error("Incorrect number of arguments.")
+        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])
+        if src_project == tgt_project and not opts.cicount:
-        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:
             # in this case, the user usually wants to build different spec
             # files from the same source
             opts.cicount = "copy"
@@ -3047,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,
         )

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)
 
 
@@ -5493,11 +5497,11 @@ def link_pac(
     dst_project: str,
     dst_package: str,
     force: bool,
-    rev="",
+    rev=None,
-    cicount="",
+    cicount=None,
     disable_publish=False,
     missing_target=False,
-    vrev="",
+    vrev=None,
     disable_build=False,
 ):
     """
@@ -5505,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']

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__":