Repository: conan-io/conan
Pull request: Do not allow an alias to override an existing package
Changelog: Fix: Do not allow an alias to override an existing package
Docs: omit

closes #4423 

✅ Do not pass the `cache` object to the `export_alias` function.

@REVISIONS: 1

Problem statement:
Prevent conan alias from overwriting existing packages
When creating an alias package with `conan alias` it is easy to mix up the order of parameters:

* Correct: `conan alias boost/ALIAS@conan/stable boost/1.69.0@conan/stable`
* Wrong: `conan alias boost/1.69.0@conan/stable boost/ALIAS@conan/stable`

In case you're accidentally calling the wrong order you're overwriting the package you initially wanted to reference. If this happens you render your original package useless and need to remove it. Then you can start all over again.
It would be perfect to have a built-in check that tells the user that the existing package `boost/1.69.0@conan/stable` already exists and will be overwritten by an alias package - and if the user is really sure this is what he wants to do.

Happens to me with Conan 1.11.0.

Hints from issue discussion:
I just found that a similar feature has been discussed within #3805 - the issue is just named in a way that it's hard to see on first glance what's going on there.
I have added it as "ux", because it is mostly a check. There are some possibilities:

- Throw an error if the package exists, and require ``--force``. Good, but will be breaking.
- Interactive: "are you sure you want to overwrite?". This might break also users using alias in CI (which is typical)
- Opt-in: define a ``--check`` argument (or something similar), that forces the alias to do this check, and will throw an error

It is a simple feature, but I see no way to do it both effectively and nonbreaking.
Agree, `--check` is not the most intuitive. This kind of issue happens to people who are using `conan alias` for the first few times - typically before you know about optional parameters. When a user knows about the possible parameters he probably also knows in which order to use `conan alias`.

I'd say it's better to have it than to miss it.

There's another option that is non-breaking and more intuitive: Add a new command like `conan alias-with-check` (I'm sure there's a better name for it) that'll do the same as `conan alias --check`. This would be non-breaking, but newer users would stumble upon it intuitively. Though this would add to the complexity of conan itself (new command, possible issues within future versions, users mixing up alias and alias-with-check...). Just wanted to throw it out there.
Another idea which might get the consensus as non-breaking: throw an error if the package already exists and it is NOT an alias already. This was the case that would be breaking, when you want to redefine an existing alias. But a check that you are not breaking an existing package can be totally OK

Commit message: Merge remote-tracking branch 'conan/develop' into fix/alias-existing

Patch to review:
diff --git a/conans/client/cmd/export.py b/conans/client/cmd/export.py
index a85796c7206..f4c26225676 100644
--- a/conans/client/cmd/export.py
+++ b/conans/client/cmd/export.py
@@ -15,29 +15,25 @@
 from conans.model.scm import detect_repo_type
 from conans.paths import CONANFILE
 from conans.search.search import search_recipes, search_packages
-from conans.util.files import is_dirty, load, mkdir, rmdir, save, set_dirty, remove
+from conans.util.files import is_dirty, load, rmdir, save, set_dirty, remove
 from conans.util.log import logger
 
 
-def export_alias(reference, target_reference, cache, output):
-    if reference.name != target_reference.name:
-        raise ConanException("An alias can only be defined to a package with the same name")
+def export_alias(ref_layout, target_reference, output, revisions_enabled):
     conanfile = """
 from conans import ConanFile
 
 class AliasConanfile(ConanFile):
     alias = "%s"
-""" % target_reference.full_repr()
+""" % target_reference
 
-    export_path = cache.export(reference)
-    mkdir(export_path)
-    save(os.path.join(export_path, CONANFILE), conanfile)
-    mkdir(cache.export_sources(reference))
-    digest = FileTreeManifest.create(export_path)
-    digest.save(export_path)
+    save(ref_layout.conanfile(), conanfile)
+    digest = FileTreeManifest.create(ref_layout.export())
+    digest.save(folder=ref_layout.export())
 
     # Create the metadata for the alias
-    _update_revision_in_metadata(cache, output, None, reference, digest)
+    _update_revision_in_metadata(ref_layout=ref_layout, revisions_enabled=revisions_enabled,
+                                 output=output, path=None, digest=digest)
 
 
 def cmd_export(conanfile_path, conanfile, ref, keep_source, output, cache, hook_manager):
@@ -98,7 +94,11 @@ def cmd_export(conanfile_path, conanfile, ref, keep_source, output, cache, hook_
         digest.save(package_layout.export())
 
     # Compute the revision for the recipe
-    _update_revision_in_metadata(cache, output, os.path.dirname(conanfile_path), ref, digest)
+    _update_revision_in_metadata(ref_layout=package_layout,
+                                 revisions_enabled=cache.config.revisions_enabled,
+                                 output=output,
+                                 path=os.path.dirname(conanfile_path),
+                                 digest=digest)
 
     # FIXME: Conan 2.0 Clear the registry entry if the recipe has changed
     source_folder = package_layout.source()
@@ -229,18 +229,18 @@ def _detect_scm_revision(path):
     return repo_obj.get_revision(), repo_type
 
 
-def _update_revision_in_metadata(cache, output, path, ref, digest):
+def _update_revision_in_metadata(ref_layout, revisions_enabled, output, path, digest):
 
     scm_revision_detected, repo_type = _detect_scm_revision(path)
     revision = scm_revision_detected or digest.summary_hash
-    if cache.config.revisions_enabled:
+    if revisions_enabled:
         if scm_revision_detected:
             output.info("Using {} commit as the recipe"
                         " revision: {} ".format(repo_type, revision))
         else:
             output.info("Using the exported files summary hash as the recipe"
                         " revision: {} ".format(revision))
-    with cache.package_layout(ref).update_metadata() as metadata:
+    with ref_layout.update_metadata() as metadata:
         metadata.recipe.revision = revision
         metadata.recipe.time = None
 
diff --git a/conans/client/conan_api.py b/conans/client/conan_api.py
index 0f079fa4827..8fa80feba7e 100644
--- a/conans/client/conan_api.py
+++ b/conans/client/conan_api.py
@@ -45,6 +45,7 @@
 from conans.client.userio import UserIO
 from conans.errors import ConanException, NotFoundException
 from conans.model.conan_file import get_env_context_manager
+from conans.model.editable_cpp_info import get_editable_abs_path
 from conans.model.graph_info import GraphInfo, GRAPH_INFO_FILE
 from conans.model.ref import ConanFileReference, PackageReference, check_valid_ref
 from conans.model.version import Version
@@ -56,7 +57,6 @@
 from conans.util.files import exception_message_safe, mkdir, save_files
 from conans.util.log import configure_logger
 from conans.util.tracer import log_command, log_exception
-from conans.model.editable_cpp_info import get_editable_abs_path
 
 default_manifest_folder = '.conan_manifests'
 
@@ -934,7 +934,23 @@ def get_path(self, reference, package_id=None, path=None, remote_name=None):
     def export_alias(self, reference, target_reference):
         ref = ConanFileReference.loads(reference)
         target_ref = ConanFileReference.loads(target_reference)
-        return export_alias(ref, target_ref, self._cache, self._user_io.out)
+
+        if ref.name != target_ref.name:
+            raise ConanException("An alias can only be defined to a package with the same name")
+
+        # Do not allow to override an existing package
+        alias_conanfile_path = self._cache.package_layout(ref).conanfile()
+        if os.path.exists(alias_conanfile_path):
+            conanfile_class = self._loader.load_class(alias_conanfile_path)
+            conanfile = conanfile_class(self._user_io.out, None, str(ref))
+            if not getattr(conanfile, 'alias', None):
+                raise ConanException("Reference '{}' is already a package, remove it before creating"
+                                     " and alias with the same name".format(ref))
+
+        ref_layout = self._cache.package_layout(ref)
+        return export_alias(ref_layout, str(target_ref),
+                            revisions_enabled=self._cache.config.revisions_enabled,
+                            output=self._user_io.out)
 
     @api_method
     def get_default_remote(self):
diff --git a/conans/test/functional/command/alias_test.py b/conans/test/functional/command/alias_test.py
index bfb2d1ce47a..2cc7611b9e0 100644
--- a/conans/test/functional/command/alias_test.py
+++ b/conans/test/functional/command/alias_test.py
@@ -1,4 +1,5 @@
 import os
+import textwrap
 import unittest
 
 from parameterized.parameterized import parameterized
@@ -429,23 +430,24 @@ def test_basic_test(self):
         servers = {"default": test_server}
         client = TestClient(servers=servers, users={"default": [("lasote", "mypass")]})
         for i in (1, 2):
-            conanfile = """from conans import ConanFile
+            conanfile = textwrap.dedent("""
+                from conans import ConanFile
 
-class TestConan(ConanFile):
-    name = "Hello"
-    version = "0.%s"
-    """ % i
+                class TestConan(ConanFile):
+                    name = "Hello"
+                    version = "0.%s"
+                    """ % i)
             client.save({"conanfile.py": conanfile})
             client.run("export . lasote/channel")
 
         client.run("alias Hello/0.X@lasote/channel Hello/0.1@lasote/channel")
-        conanfile_chat = """from conans import ConanFile
-
-class TestConan(ConanFile):
-    name = "Chat"
-    version = "1.0"
-    requires = "Hello/0.X@lasote/channel"
-    """
+        conanfile_chat = textwrap.dedent("""
+            from conans import ConanFile
+            class TestConan(ConanFile):
+                name = "Chat"
+                version = "1.0"
+                requires = "Hello/0.X@lasote/channel"
+                """)
         client.save({"conanfile.py": conanfile_chat}, clean_first=True)
         client.run("export . lasote/channel")
         client.save({"conanfile.txt": "[requires]\nChat/1.0@lasote/channel"}, clean_first=True)
@@ -469,3 +471,43 @@ class TestConan(ConanFile):
         client.run("install . --build=missing")
         self.assertIn("Hello/0.2", client.user_io.out)
         self.assertNotIn("Hello/0.1", client.user_io.out)
+
+    def test_not_override_package(self):
+        """ Do not override a package with an alias
+
+            If we create an alias with the same name as an existing package, it will
+            override the package without any warning.
+        """
+        t = TestClient()
+        conanfile = textwrap.dedent("""
+            from conans import ConanFile
+            class Pkg(ConanFile):
+                description = "{}"
+            """)
+
+        # Create two packages
+        reference1 = "PkgA/0.1@user/testing"
+        t.save({"conanfile.py": conanfile.format(reference1)})
+        t.run("export . {}".format(reference1))
+
+        reference2 = "PkgA/0.2@user/testing"
+        t.save({"conanfile.py": conanfile.format(reference2)})
+        t.run("export . {}".format(reference2))
+
+        # Now create an alias overriding one of them
+        alias = reference2
+        t.run("alias {alias} {reference}".format(alias=alias, reference=reference1),
+              assert_error=True)
+        self.assertIn("ERROR: Reference '{}' is already a package".format(alias), t.out)
+
+        # Check that the package is not damaged
+        t.run("inspect {} -a description".format(reference2))
+        self.assertIn("description: {}".format(reference2), t.out)
+
+        # Remove it, and create the alias again (twice, override an alias is allowed)
+        t.run("remove {} -f".format(reference2))
+        t.run("alias {alias} {reference}".format(alias=alias, reference=reference1))
+        t.run("alias {alias} {reference}".format(alias=alias, reference=reference1))
+
+        t.run("inspect {} -a description".format(reference2))
+        self.assertIn("description: None", t.out)  # The alias conanfile doesn't have description


Code review comments:
diff_hunk: @@ -15,29 +15,25 @@
 from conans.model.scm import detect_repo_type
 from conans.paths import CONANFILE
 from conans.search.search import search_recipes, search_packages
-from conans.util.files import is_dirty, load, mkdir, rmdir, save, set_dirty, remove
+from conans.util.files import is_dirty, load, rmdir, save, set_dirty, remove
 from conans.util.log import logger
 
 
-def export_alias(reference, target_reference, cache, output):
-    if reference.name != target_reference.name:
-        raise ConanException("An alias can only be defined to a package with the same name")
+def export_alias(ref_layout, target_reference, output, revisions_enabled):
     conanfile = """
 from conans import ConanFile
 
 class AliasConanfile(ConanFile):
     alias = "%s"
-""" % target_reference.full_repr()
+""" % target_reference
path: conans/client/cmd/export.py
text: @user2:
This is changing behavior, not allowing alias to revisions, why? Discuss with @user1 what behavior we want for alias.

@user1:
I relaunched the CI for this with revisions enabled, because yes, it should break the alias test with revisions.

diff_hunk: @@ -15,29 +15,25 @@
 from conans.model.scm import detect_repo_type
 from conans.paths import CONANFILE
 from conans.search.search import search_recipes, search_packages
-from conans.util.files import is_dirty, load, mkdir, rmdir, save, set_dirty, remove
+from conans.util.files import is_dirty, load, rmdir, save, set_dirty, remove
 from conans.util.log import logger
 
 
-def export_alias(reference, target_reference, cache, output):
-    if reference.name != target_reference.name:
-        raise ConanException("An alias can only be defined to a package with the same name")
+def export_alias(ref_layout, target_reference, output, revisions_enabled):
     conanfile = """
 from conans import ConanFile
 
 class AliasConanfile(ConanFile):
     alias = "%s"
-""" % target_reference.full_repr()
+""" % target_reference
 
-    export_path = cache.export(reference)
-    mkdir(export_path)
-    save(os.path.join(export_path, CONANFILE), conanfile)
-    mkdir(cache.export_sources(reference))
-    digest = FileTreeManifest.create(export_path)
-    digest.save(export_path)
+    save(ref_layout.conanfile(), conanfile)
+    digest = FileTreeManifest.create(ref_layout.export())
+    digest.save(folder=ref_layout.export())
 
     # Create the metadata for the alias
-    _update_revision_in_metadata(cache, output, None, reference, digest)
+    _update_revision_in_metadata(ref_layout=ref_layout, revisions_enabled=revisions_enabled,
path: conans/client/cmd/export.py
text: @user1:
Changing this to use layouts should have been done in the other PR, not in this one.