Repository: conan-io/conan

Issue:
Could "conan get" use package reference instead of reference + -p=ID?
From: https://github.com/conan-io/conan/issues/4479

Now:
```bash
$ conan get zlib/1.2.11@conan/stable conanmanifest.txt -p 6cc50b139b9c3d27b3e9042d5f5372d327b3a9f7
```

It would feel more natural:
```bash
$ conan get zlib/1.2.11@conan/stable:6cc50b139b9c3d27b3e9042d5f5372d327b3a9f7 conanmanifest.txt
```




Discussion:
danimtb: Forgot to mention that it would be nice to have somewhere in the docs the definitions for "reference" and "package reference" and "package id" (also for "recipe" and the controversial "package", but that is another story)
jgsogo: Much better now, please @danimtb review it again (I've added the test you were requesting) and consider merging. Thanks!
danimtb: ✅ LGTM. Add the docs and I will merge accordingly 😀 
jgsogo: Done! Thanks for the kindly reminder...
jgsogo: I think that everything is ready, isn't it? 💫 
memsharded: I am fine with ``get_content``.
danimtb: @jgsogo have a look at the review comments and solve conflicts
jgsogo: Not a strong opinion, but there are several items that should have the same nomenclature for consistency:
 * the argument in the CLI is `path` (we can change it because it is a positional one)
 * the function in the `conan_api` is `get_path` (we can change it...)
 * the function in the `layout`, change suggested from `get_path` to `get_content`.
 * functions in the remotes/api related to this that are called `get_path`.

So, although I prefer `get_content` too, I'm not sure if it is better to keep the `get_path` nomenclature. I'm going to fix the conflicts, I'll read your opinion when I'm back.


uilianries: Let's use `get_path` and `get_content` just to confuse all programmers :smiling_imp: 

Now really, if we have the opportunity to change all those methods and follow the correct representation of them, so I think it will be better. When I read `get_path` I understand that function will return a file/dir path only.
jgsogo: @uilianries, @memsharded, I'm not going to do the renaming in this PR, nevertheless I've written down it here (https://github.com/conan-io/conan/issues/4132#issuecomment-464658464): still we need to find a better name than `get_content`, so feel free to comment there so we can address it in a future PR.

Thanks.

Patch:
diff --git a/conans/client/command.py b/conans/client/command.py
index 579c44347..6a60b7cc1 100644
--- a/conans/client/command.py
+++ b/conans/client/command.py
@@ -1353,7 +1353,8 @@ class Command(object):
                                  'conanfile if only a reference is specified and a conaninfo.txt '
                                  'file contents if the package is also specified',
                             default=None, nargs="?")
-        parser.add_argument("-p", "--package", default=None, help='Package ID',
+        parser.add_argument("-p", "--package", default=None,
+                            help="Package ID [DEPRECATED: use full reference instead]",
                             action=OnceArgument)
         parser.add_argument("-r", "--remote", action=OnceArgument,
                             help='Get from this specific remote')
@@ -1361,7 +1362,24 @@ class Command(object):
                             help='Do not decorate the text')
         args = parser.parse_args(*args)
 
-        ret, path = self._conan.get_path(args.reference, args.package, args.path, args.remote)
+        try:
+            pref = PackageReference.loads(args.reference, validate=True)
+            reference = pref.ref.full_repr()
+            package_id = pref.id
+        except ConanException:
+            reference = args.reference
+            package_id = args.package
+
+            if package_id:
+                self._user_io.out.warn("Usage of `--package` argument is deprecated."
+                                       " Use a full reference instead: "
+                                       "`conan get [...] {}:{}`".format(reference, package_id))
+        else:
+            if args.package:
+                raise ConanException("Use a full package reference (preferred) or the `--package`"
+                                     " command argument, but not both.")
+
+        ret, path = self._conan.get_path(reference, package_id, args.path, args.remote)
         if isinstance(ret, list):
             self._outputer.print_dir_list(ret, path, args.raw)
         else:
diff --git a/conans/client/conan_api.py b/conans/client/conan_api.py
index 538bdee2e..bd13f228f 100644
--- a/conans/client/conan_api.py
+++ b/conans/client/conan_api.py
@@ -925,8 +925,8 @@ class ConanAPIV1(object):
             path = "conanfile.py" if not package_id else "conaninfo.txt"
 
         if not remote_name:
-            from conans.client.local_file_getter import get_path
-            return get_path(self._cache, ref, package_id, path), path
+            package_layout = self._cache.package_layout(ref, short_paths=None)
+            return package_layout.get_path(path=path, package_id=package_id), path
         else:
             remote = self.get_remote_by_name(remote_name)
             if self._cache.config.revisions_enabled and not ref.revision:
diff --git a/conans/client/file_copier.py b/conans/client/file_copier.py
index 463aba1c7..c21c69a48 100644
--- a/conans/client/file_copier.py
+++ b/conans/client/file_copier.py
@@ -16,12 +16,12 @@ def report_copied_files(copied, output, message_suffix="Copied"):
         return False
 
     for ext, files in ext_files.items():
-        files_str = (", ".join(files)) if len(files) < 5 else ""
+        files_str = (": " + ", ".join(files)) if len(files) < 5 else ""
         file_or_files = "file" if len(files) == 1 else "files"
         if not ext:
-            output.info("%s %d %s: %s" % (message_suffix, len(files), file_or_files, files_str))
+            output.info("%s %d %s%s" % (message_suffix, len(files), file_or_files, files_str))
         else:
-            output.info("%s %d '%s' %s: %s" % (message_suffix, len(files), ext, file_or_files, files_str))
+            output.info("%s %d '%s' %s%s" % (message_suffix, len(files), ext, file_or_files, files_str))
     return True
 
 
diff --git a/conans/client/local_file_getter.py b/conans/client/local_file_getter.py
deleted file mode 100644
index f91328677..000000000
--- a/conans/client/local_file_getter.py
+++ /dev/null
@@ -1,29 +0,0 @@
-import os
-
-from conans.errors import NotFoundException
-from conans.model.manifest import discarded_file
-from conans.model.ref import PackageReference
-from conans.util.files import load
-
-
-def get_path(cache, ref, package_id, path):
-    """
-    :param cache: Conan's client cache
-    :param ref: Specified reference in the conan get command
-    :param package_id: Specified package id (can be None)
-    :param path: Path to a file, subfolder of exports (if only ref)
-                 or package (if package_id defined)
-    :return: The real path in the local cache for the specified parameters
-    """
-    if package_id is None:  # Get the file in the exported files
-        folder = cache.export(ref)
-    else:
-        folder = cache.package(PackageReference(ref, package_id), short_paths=None)
-
-    abs_path = os.path.join(folder, path)
-    if not os.path.exists(abs_path):
-        raise NotFoundException("The specified path doesn't exist")
-    if os.path.isdir(abs_path):
-        return sorted([path for path in os.listdir(abs_path) if not discarded_file(path)])
-    else:
-        return load(abs_path)
diff --git a/conans/errors.py b/conans/errors.py
index b4677b1ce..8333b58b2 100644
--- a/conans/errors.py
+++ b/conans/errors.py
@@ -187,7 +187,7 @@ class PackageNotFoundException(NotFoundException):
 
     def __str__(self):
         tmp = self.pref.full_repr() if self.print_rev else str(self.pref)
-        return "Binary package not found: '{}'%s".format(tmp, self.remote_message())
+        return "Binary package not found: '{}'{}".format(tmp, self.remote_message())
 
 
 class UserInterfaceErrorException(RequestErrorException):
diff --git a/conans/model/ref.py b/conans/model/ref.py
index c986237a6..cdb58e074 100644
--- a/conans/model/ref.py
+++ b/conans/model/ref.py
@@ -154,7 +154,7 @@ class PackageReference(namedtuple("PackageReference", "ref id revision")):
         text = text.strip()
         tmp = text.split(":")
         try:
-            ref = ConanFileReference.loads(tmp[0].strip())
+            ref = ConanFileReference.loads(tmp[0].strip(), validate=validate)
             package_id = tmp[1].strip()
         except IndexError:
             raise ConanException("Wrong package reference  %s" % text)
diff --git a/conans/paths/package_layouts/package_cache_layout.py b/conans/paths/package_layouts/package_cache_layout.py
index 3552c5760..b74a01061 100644
--- a/conans/paths/package_layouts/package_cache_layout.py
+++ b/conans/paths/package_layouts/package_cache_layout.py
@@ -4,8 +4,10 @@ import os
 import platform
 from contextlib import contextmanager
 
+from conans.errors import NotFoundException
 from conans.errors import RecipeNotFoundException, PackageNotFoundException
 from conans.model.manifest import FileTreeManifest
+from conans.model.manifest import discarded_file
 from conans.model.package_metadata import PackageMetadata
 from conans.model.ref import ConanFileReference
 from conans.model.ref import PackageReference
@@ -144,3 +146,28 @@ class PackageCacheLayout(object):
         except IOError:
             raise PackageNotFoundException(pref)
         return read_manifest.summary_hash
+
+    # Raw access to file
+    def get_path(self, path, package_id=None):
+        """ Return the contents for the given `path` inside current layout, it can
+            be a single file or the list of files in a directory
+
+            :param package_id: will retrieve the contents from the package directory
+            :param path: path relative to the cache reference or package folder
+        """
+
+        assert not os.path.isabs(path)
+
+        if package_id is None:  # Get the file in the exported files
+            folder = self.export()
+        else:
+            pref = PackageReference(self._ref, package_id)
+            folder = self.package(pref)
+
+        abs_path = os.path.join(folder, path)
+        if not os.path.exists(abs_path):
+            raise NotFoundException("The specified path doesn't exist")
+        if os.path.isdir(abs_path):
+            return sorted([path for path in os.listdir(abs_path) if not discarded_file(path)])
+        else:
+            return load(abs_path)
diff --git a/conans/paths/package_layouts/package_editable_layout.py b/conans/paths/package_layouts/package_editable_layout.py
index 066043e6c..3ca70b1d9 100644
--- a/conans/paths/package_layouts/package_editable_layout.py
+++ b/conans/paths/package_layouts/package_editable_layout.py
@@ -53,3 +53,6 @@ class PackageEditableLayout(object):
 
     def package_metadata(self):
         raise ConanException("Package metadata is not available for editable packages")
+
+    def get_path(self, package_id=None, path=None):
+        raise ConanException("Operation not allowed on a package installed as editable")


Test patch:
diff --git a/conans/test/functional/command/conan_get_test.py b/conans/test/functional/command/conan_get_test.py
index 81c8925af..8c5ef688d 100644
--- a/conans/test/functional/command/conan_get_test.py
+++ b/conans/test/functional/command/conan_get_test.py
@@ -1,6 +1,8 @@
 import unittest
 
 import mock
+import textwrap
+from parameterized import parameterized
 
 from conans.test.utils.tools import NO_SETTINGS_PACKAGE_ID, TestClient, TestServer
 
@@ -8,50 +10,51 @@ from conans.test.utils.tools import NO_SETTINGS_PACKAGE_ID, TestClient, TestServ
 class ConanGetTest(unittest.TestCase):
 
     def setUp(self):
-        self.conanfile = '''
-from conans import ConanFile
-
-class HelloConan(ConanFile):
-    name = "Hello0"
-    version = "0.1"
-    exports_sources = "path*"
-    exports = "other*"
-        '''
-
-        test_server = TestServer([],  # write permissions
-                                 users={"lasote": "mypass"})  # exported users and passwords
+        self.reference = "Hello0/0.1@lasote/channel"
+        self.conanfile = textwrap.dedent('''
+            from conans import ConanFile
+            
+            class HelloConan(ConanFile):
+                name = "Hello0"
+                version = "0.1"
+                exports_sources = "path*"
+                exports = "other*"
+            ''')
+
+        test_server = TestServer([], users={"lasote": "mypass"})
         servers = {"default": test_server}
         self.client = TestClient(servers=servers, users={"default": [("lasote", "mypass")]})
-        files = {"conanfile.py": self.conanfile, "path/to/exported_source": "1",
+        files = {"conanfile.py": self.conanfile,
+                 "path/to/exported_source": "1",
                  "other/path/to/exported": "2"}
         self.client.save(files)
         self.client.run("export . lasote/channel")
-        self.client.run("install Hello0/0.1@lasote/channel --build missing")
+        self.client.run("install {} --build missing".format(self.reference))
 
-    def test_get_local(self):
+    def test_get_local_reference(self):
         # Local search, dir list
-        self.client.run('get Hello0/0.1@lasote/channel .')
+        self.client.run('get {} .'.format(self.reference))
         self.assertEquals("""Listing directory '.':
  conanfile.py
  conanmanifest.txt
  other
 """, self.client.user_io.out)
 
-        self.client.run('get Hello0/0.1@lasote/channel other --raw')
+        self.client.run('get {} other --raw'.format(self.reference))
         self.assertEquals("path\n", self.client.user_io.out)
 
-        self.client.run('get Hello0/0.1@lasote/channel other/path --raw')
+        self.client.run('get {} other/path --raw'.format(self.reference))
         self.assertEquals("to\n", self.client.user_io.out)
 
-        self.client.run('get Hello0/0.1@lasote/channel other/path/to')
+        self.client.run('get {} other/path/to'.format(self.reference))
         self.assertEquals("Listing directory 'other/path/to':\n exported\n",
                           self.client.user_io.out)
 
-        self.client.run('get Hello0/0.1@lasote/channel other/path/to/exported')
+        self.client.run('get {} other/path/to/exported'.format(self.reference))
         self.assertEquals("2\n", self.client.user_io.out)
 
         # Local search, conanfile print
-        self.client.run('get Hello0/0.1@lasote/channel --raw')
+        self.client.run('get {} --raw'.format(self.reference))
         self.assertIn(self.conanfile, self.client.user_io.out)
 
         # Get conanfile, verify the formatter applied
@@ -63,55 +66,101 @@ class HelloConan(ConanFile):
 
         with mock.patch("pygments.highlight", verify_python_lexer):
             with mock.patch("conans.client.output.ConanOutput.is_terminal", True):
-                self.client.run('get Hello0/0.1@lasote/channel')
+                self.client.run('get {}'.format(self.reference))
         self.assertIn("Hi there! I'm a mock!", self.client.out)
 
-        # Local search print package info
-        self.client.run('get Hello0/0.1@lasote/channel -p %s --raw' % NO_SETTINGS_PACKAGE_ID)
-        self.assertIn("""
-[requires]
-
-
-[options]
-
-
-[full_settings]
-
-
-[full_requires]
-
-
-[full_options]
-
-
-[recipe_hash]
-    dcb3ad84f4e46fb3da40dbbea0017094
+    @parameterized.expand([(True,), (False,)])
+    def test_get_local_package_reference(self, use_pkg_reference):
+        args_reference = self.reference if not use_pkg_reference else \
+            "{}:{}".format(self.reference, NO_SETTINGS_PACKAGE_ID)
+        args_package = " -p {}".format(NO_SETTINGS_PACKAGE_ID) if not use_pkg_reference else ""
 
-""", self.client.user_io.out)
+        # Local search print package info
+        self.client.run('get {} {} --raw'.format(args_reference, args_package))
+        self.assertIn(textwrap.dedent("""
+            [requires]
+            
+            
+            [options]
+            
+            
+            [full_settings]
+            
+            
+            [full_requires]
+            
+            
+            [full_options]
+            
+            
+            [recipe_hash]
+                07e4bf611af374672215a94d84146e2d
+            
+            """), self.client.user_io.out)
 
         # List package dir
-        self.client.run('get Hello0/0.1@lasote/channel "." -p %s --raw' % NO_SETTINGS_PACKAGE_ID)
-        self.assertEquals("conaninfo.txt\nconanmanifest.txt\n", self.client.user_io.out)
+        self.client.run('get {} "." {} --raw'.format(args_reference, args_package))
+        assert_cmp = self.assertEqual if use_pkg_reference else self.assertIn
+        assert_cmp("conaninfo.txt\nconanmanifest.txt\n", self.client.user_io.out)
 
-    def test_get_remote(self):
+    def test_get_remote_reference(self):
         self.client.run('upload "Hello*" --all -c')
 
         # Remote search, dir list
-        self.client.run('get Hello0/0.1@lasote/channel . -r default --raw')
+        self.client.run('get {} . -r default --raw'.format(self.reference))
         self.assertIn("conan_export.tgz\nconan_sources.tgz\nconanfile.py\nconanmanifest.txt",
                       self.client.user_io.out)
 
         # Remote search, conanfile print
-        self.client.run('get Hello0/0.1@lasote/channel -r default --raw')
+        self.client.run('get {} -r default --raw'.format(self.reference))
         self.assertIn(self.conanfile, self.client.user_io.out)
 
+    @parameterized.expand([(True,), (False,)])
+    def test_get_remote_package_reference(self, use_pkg_reference):
+        args_reference = self.reference if not use_pkg_reference else \
+            "{}:{}".format(self.reference, NO_SETTINGS_PACKAGE_ID)
+        args_package = " -p {}".format(NO_SETTINGS_PACKAGE_ID) if not use_pkg_reference else ""
+
+        self.client.run('upload "Hello*" --all -c')
+
         # List package dir
-        self.client.run('get Hello0/0.1@lasote/channel "." '
-                        '  -p %s --raw -r default' % NO_SETTINGS_PACKAGE_ID)
-        self.assertEquals("conan_package.tgz\nconaninfo.txt\nconanmanifest.txt\n",
-                          self.client.user_io.out)
+        self.client.run('get {} "." {} --raw -r default'.format(args_reference, args_package))
+        assert_cmp = self.assertEqual if use_pkg_reference else self.assertIn
+        assert_cmp("conan_package.tgz\nconaninfo.txt\nconanmanifest.txt\n", self.client.user_io.out)
+
+    def test_not_found_reference(self):
+        self.client.run('get {} "." -r default'.format(self.reference), assert_error=True)
+        self.assertIn("Recipe not found: '{}'".format(self.reference), self.client.user_io.out)
+
+    @parameterized.expand([(True,), (False,)])
+    def test_not_found_package_reference(self, use_pkg_reference):
+        fake_package_id = "123123123123123"
+        args_reference = self.reference if not use_pkg_reference else \
+            "{}:{}".format(self.reference, fake_package_id)
+        args_package = " -p {}".format(fake_package_id) if not use_pkg_reference else ""
+
+        self.client.run('get {} "." -r default {}'.format(args_reference, args_package),
+                        assert_error=True)
+        self.assertIn("ERROR: Binary package not found: '{}:{}'".format(self.reference,
+                                                                        fake_package_id),
+                      self.client.user_io.out)
+
+    def test_duplicated_input(self):
+        """ Fail if given the full reference and the `-p` argument (even if they are equal)"""
+        self.client.run('get {reference}:{pkg_id} -p {pkg_id}'.format(reference=self.reference,
+                                                                      pkg_id=NO_SETTINGS_PACKAGE_ID),
+                        assert_error=True)
+        self.assertIn("Use a full package reference (preferred) or the `--package` "
+                      "command argument, but not both.", self.client.out)
+
+    def test_deprecated_argument(self):
+        """ Argument `-p` should print a deprecation warning (and work)"""
+        self.client.run('get {reference} -p {pkg_id}'.format(reference=self.reference,
+                                                             pkg_id=NO_SETTINGS_PACKAGE_ID))
+        self.assertIn("WARN: Usage of `--package` argument is deprecated. Use a full reference "
+                      "instead: `conan get [...] ", self.client.out)
 
-    def test_not_found(self):
+    """
         self.client.run('get Hello0/0.1@lasote/channel "." -r default', assert_error=True)
         self.assertIn("Recipe not found: 'Hello0/0.1@lasote/channel'", self.client.user_io.out)
 
@@ -123,3 +172,4 @@ class HelloConan(ConanFile):
         else:
             self.assertIn("Binary package not found: 'Hello0/0.1@lasote/channel:123123123123123'",
                           self.client.user_io.out)
+    """
\ No newline at end of file
diff --git a/conans/test/unittests/client/file_copier/__init__.py b/conans/test/unittests/client/file_copier/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/conans/test/unittests/client/file_copier/test_report_copied_files.py b/conans/test/unittests/client/file_copier/test_report_copied_files.py
new file mode 100644
index 000000000..37737d7e1
--- /dev/null
+++ b/conans/test/unittests/client/file_copier/test_report_copied_files.py
@@ -0,0 +1,29 @@
+# coding=utf-8
+
+import unittest
+
+from conans.client.file_copier import report_copied_files
+from conans.test.utils.tools import TestBufferConanOutput
+
+
+class ReportCopiedFilesTestCase(unittest.TestCase):
+
+    def test_output_string(self):
+        output = TestBufferConanOutput()
+
+        files = ['/abs/path/to/file.pdf',
+                 '../rel/path/to/file2.pdf',
+                 '../rel/path/to/file3.pdf',
+                 '../rel/path/to/file4.pdf',
+                 '../rel/path/to/file5.pdf',
+                 '../rel/path/to/file6.pdf',
+                 '../rel/path/to/file7.pdf',
+                 '/without/ext/no_ext1',
+                 'no_ext2',
+                 'a/other.txt']
+
+        report_copied_files(files, output)
+        lines = sorted(str(output).splitlines())
+        self.assertEqual("Copied 7 '.pdf' files", lines[2])
+        self.assertEqual("Copied 2 files: no_ext1, no_ext2", lines[1])
+        self.assertEqual("Copied 1 '.txt' file: other.txt", lines[0])
diff --git a/conans/test/utils/conanfile.py b/conans/test/utils/conanfile.py
index b9e6adb9f..0be3bb807 100644
--- a/conans/test/utils/conanfile.py
+++ b/conans/test/utils/conanfile.py
@@ -1,7 +1,7 @@
 import os
 from collections import namedtuple
 
-from conans import ConanFile, Options
+from conans import Options
 from conans.model.conan_file import ConanFile
 from conans.model.options import PackageOptions
 from conans.test.utils.tools import TestBufferConanOutput