Repository: conan-io/conan
Pull request: Fix several issues with download command and revisions
Changelog: Bugfix: Fix different problems when using `conan download` with revisions. 
Docs: Omit

This PR fixes the following problems when using revisions with `conan download` command:

- If you had revisions disabled and added a revision to download command it did not fail but even create the revision in the cache.
- With revisions enabled but without user and channel the command failed.
- When revisions enabled and specifying a recipe revision it did not download the correct recipe revision but the last one.
- When revisions enabled and specifying a package revision it did not download the correct package revision.

It also fixes that when using `create` in the `TurboTestClient` without user and channel the command failed.

Closes #6106 


- [X] Refer to the issue that supports this Pull Request.
- [X] If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
- [X] I've read the [Contributing guide](https://github.com/conan-io/conan/blob/develop/.github/CONTRIBUTING.md).
- [X] I've followed the PEP8 style guides for Python code.
- [ ] I've opened another PR in the Conan docs repo to the ``develop`` branch, documenting this one. 

<sup>**Note:** By default this PR will skip the slower tests and will use a limited set of python versions. Check [here](https://github.com/conan-io/conan/blob/develop/.github/PR_INCREASE_TESTING.md) how to increase the testing level by writing some tags in the current PR body text.</sup>

Problem statement:
[bug] conan download pkg/version@user/channel#randomrev downloads latest
Latest 1.20, Windows 10, python 3.7

With revisions disable this doesn't fail (it should). And worse, it creates an entry with revision ``randomrev`` in the cache.

Also, it seems that it is not possible to ``conan download`` a package with a given package revision.

Hints from issue discussion:
It is important to follow up here, this could be problematic or show underlying problems, for example if using lockfiles, like the locked version is not the one being retrieved.

Commit message: add test without user channel

Patch to review:
diff --git a/conans/client/command.py b/conans/client/command.py
index e0cc756b3cf..687a1307f62 100644
--- a/conans/client/command.py
+++ b/conans/client/command.py
@@ -403,8 +403,12 @@ def download(self, *args):
         else:
             reference = repr(pref.ref)
             if pref.ref.user is None:
-                reference += "@"
-            packages_list = [pref.id]
+                if pref.ref.revision:
+                    reference = "%s/%s@#%s" % (pref.ref.name, pref.ref.version, pref.ref.revision)
+                else:
+                    reference += "@"
+            pkgref = "{}#{}".format(pref.id, pref.revision) if pref.revision else "{}".format(pref.id)
+            packages_list = [pkgref]
             if args.package:
                 raise ConanException("Use a full package reference (preferred) or the `--package`"
                                      " command argument, but not both.")
diff --git a/conans/client/conan_api.py b/conans/client/conan_api.py
index 2d3df0d106a..866e73206f4 100644
--- a/conans/client/conan_api.py
+++ b/conans/client/conan_api.py
@@ -430,6 +430,9 @@ def download(self, reference, remote_name=None, packages=None, recipe=False):
         # Install packages without settings (fixed ids or all)
         if check_valid_ref(reference):
             ref = ConanFileReference.loads(reference)
+            if ref.revision and not self.app.config.revisions_enabled:
+                raise ConanException("Revisions not enabled in the client, specify a "
+                                     "reference without revision")
             if packages and ref.revision is None:
                 for package_id in packages:
                     if "#" in package_id:
diff --git a/conans/test/functional/command/download_test.py b/conans/test/functional/command/download_test.py
index d133bf6404e..31dc33ef8d7 100644
--- a/conans/test/functional/command/download_test.py
+++ b/conans/test/functional/command/download_test.py
@@ -220,3 +220,57 @@ def no_user_channel_test(self):
         client.run("download pkg/1.0@")
         self.assertIn("pkg/1.0: Downloading pkg/1.0:%s" % NO_SETTINGS_PACKAGE_ID, client.out)
         self.assertIn("pkg/1.0: Package installed %s" % NO_SETTINGS_PACKAGE_ID, client.out)
+
+    def download_revs_disabled_with_rrev_test(self):
+        # https://github.com/conan-io/conan/issues/6106
+        client = TestClient(default_server_user=True, revisions_enabled=False)
+        client.save({"conanfile.py": GenConanfile()})
+        client.run("create . pkg/1.0@user/channel")
+        client.run("upload * --all --confirm")
+        client.run("remove * -f")
+        client.run("download pkg/1.0@user/channel#fakerevision", assert_error=True)
+        self.assertIn(
+            "ERROR: Revisions not enabled in the client, specify a reference without revision",
+            client.out)
+
+    def download_revs_enabled_with_rrev_test(self):
+        ref = ConanFileReference.loads("pkg/1.0@user/channel")
+        client = TurboTestClient(default_server_user=True, revisions_enabled=True)
+        pref = client.create(ref, conanfile=GenConanfile())
+        client.run("upload * --all --confirm")
+        client.run("remove * -f")
+        client.run("download pkg/1.0@user/channel#{}".format(pref.ref.revision))
+        self.assertIn("pkg/1.0@user/channel: Package installed {}".format(pref.id), client.out)
+        search_result = client.search("pkg/1.0@user/channel --revisions")[0]
+        self.assertIn(pref.ref.revision, search_result["revision"])
+
+        ref = ConanFileReference.loads("pkg/1.0@")
+        servers = {"default": TestServer([("*/*@*/*", "*")], [("*/*@*/*", "*")],
+                                         users={"user": "password"})}
+        client = TurboTestClient(servers=servers, revisions_enabled=True,
+                                 users={"default": [("user", "password")]})
+        pref = client.create(ref, conanfile=GenConanfile())
+        client.run("upload * --all --confirm")
+        client.run("remove * -f")
+        client.run("download pkg/1.0@#{}".format(pref.ref.revision))
+        self.assertIn("pkg/1.0: Package installed {}".format(pref.id), client.out)
+        search_result = client.search("pkg/1.0@ --revisions")[0]
+        self.assertIn(pref.ref.revision, search_result["revision"])
+
+
+    def download_revs_enabled_with_prev_test(self):
+        # https://github.com/conan-io/conan/issues/6106
+        ref = ConanFileReference.loads("pkg/1.0@user/channel")
+        client = TurboTestClient(default_server_user=True, revisions_enabled=True)
+        pref = client.create(ref, conanfile=GenConanfile())
+        client.run("upload * --all --confirm")
+        client.run("remove * -f")
+        client.run("download pkg/1.0@user/channel#{}:{}#{}".format(pref.ref.revision,
+                                                                   pref.id,
+                                                                   pref.revision))
+        self.assertIn("pkg/1.0@user/channel: Package installed {}".format(pref.id), client.out)
+        search_result = client.search("pkg/1.0@user/channel --revisions")[0]
+        self.assertIn(pref.ref.revision, search_result["revision"])
+        search_result = client.search(
+            "pkg/1.0@user/channel#{}:{} --revisions".format(pref.ref.revision, pref.id))[0]
+        self.assertIn(pref.revision, search_result["revision"])
diff --git a/conans/test/utils/tools.py b/conans/test/utils/tools.py
index aa9e3f82ecb..65fdd689907 100644
--- a/conans/test/utils/tools.py
+++ b/conans/test/utils/tools.py
@@ -1236,7 +1236,8 @@ def export(self, ref, conanfile=GenConanfile(), args=None, assert_error=False):
     def create(self, ref, conanfile=GenConanfile(), args=None, assert_error=False):
         if conanfile:
             self.save({"conanfile.py": conanfile})
-        self.run("create . {} {} --json {}".format(ref.full_str(),
+        full_str = "{}@".format(ref.full_str()) if not ref.user else ref.full_str()
+        self.run("create . {} {} --json {}".format(full_str,
                                                    args or "", self.tmp_json_name),
                  assert_error=assert_error)
         rrev = self.cache.package_layout(ref).recipe_revision()


Code review comments:
diff_hunk: @@ -220,3 +220,57 @@ def no_user_channel_test(self):
         client.run("download pkg/1.0@")
         self.assertIn("pkg/1.0: Downloading pkg/1.0:%s" % NO_SETTINGS_PACKAGE_ID, client.out)
         self.assertIn("pkg/1.0: Package installed %s" % NO_SETTINGS_PACKAGE_ID, client.out)
+
+    def download_revs_disabled_with_rrev_test(self):
+        # https://github.com/conan-io/conan/issues/6106
+        client = TestClient(default_server_user=True, revisions_enabled=False)
+        client.save({"conanfile.py": GenConanfile()})
+        client.run("create . pkg/1.0@user/channel")
+        client.run("upload * --all --confirm")
+        client.run("remove * -f")
+        client.run("download pkg/1.0@user/channel#fakerevision", assert_error=True)
path: conans/test/functional/command/download_test.py
text: @user1:
You probably can write this test without server and uploading.

diff_hunk: @@ -220,3 +220,57 @@ def no_user_channel_test(self):
         client.run("download pkg/1.0@")
         self.assertIn("pkg/1.0: Downloading pkg/1.0:%s" % NO_SETTINGS_PACKAGE_ID, client.out)
         self.assertIn("pkg/1.0: Package installed %s" % NO_SETTINGS_PACKAGE_ID, client.out)
+
+    def download_revs_disabled_with_rrev_test(self):
+        # https://github.com/conan-io/conan/issues/6106
+        client = TestClient(default_server_user=True, revisions_enabled=False)
path: conans/test/functional/command/download_test.py
text: @user2:
Tests that hardcode revisions should use something like:

```python
@user1.skipIf(get_env("TESTING_REVISIONS_ENABLED", False), "This test is insane to be "
                                                                  "tested with revisions, in "
                                                                  "general all the module")
```
So they are only run in the configuration they want to run, not twice.

diff_hunk: @@ -403,8 +403,12 @@ def download(self, *args):
         else:
             reference = repr(pref.ref)
             if pref.ref.user is None:
-                reference += "@"
-            packages_list = [pref.id]
+                if pref.ref.revision:
+                    reference = "%s/%s@#%s" % (pref.ref.name, pref.ref.version, pref.ref.revision)
+                else:
+                    reference += "@"
+            pkgref = "{}#{}".format(pref.id, pref.revision) if pref.revision else "{}".format(pref.id)
path: conans/client/command.py
text: @user1:
why ``"{}".format(pref.id)`` instead of just ``pref.id``?