Repository: conan-io/conan
Pull request: Do not raise when accessing the metadata of editable packages
Changelog: Bugfix: Do not raise when accessing the metadata of editable packages
Docs: omit

- [x] Refer to the issue that supports this Pull Request: closes #5424
- [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.
- [x] 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:
conan remote remove fails with editable packages
Trying to update the metadata, it fails because the `PackageEditableLayout` has no `update_metadata()`. The main questions are:
- Should the editable packages still have the metadata methods available? 
or 
- Should we force always to retrieve the `PackageCacheLayout` sometimes, e.g from the `remote_registry.py`?

```
Traceback (most recent call last):                                                                                                                                                                                                                                             
  File "/home/luism/workspace/conan_sources/conans/client/command.py", line 1832, in run                                                                                                                                                                                       
    method(args[0][1:])                                                                                                                                                                                                                                                        
  File "/home/luism/workspace/conan_sources/conans/client/command.py", line 1423, in remote                                                                                                                                                                                    
    return self._conan.remote_remove(remote_name)                                                                                                                                                                                                                              
  File "/home/luism/workspace/conan_sources/conans/client/conan_api.py", line 77, in wrapper                                                                                                                                                                                   
    return f(*args, **kwargs)
  File "/home/luism/workspace/conan_sources/conans/client/conan_api.py", line 922, in remote_remove
    return self._cache.registry.remove(remote_name)
  File "/home/luism/workspace/conan_sources/conans/client/cache/remote_registry.py", line 301, in remove
    with self._cache.package_layout(ref).update_metadata() as metadata:
AttributeError: 'PackageEditableLayout' object has no attribute 'update_metadata'
```

Hints from issue discussion:
@memsharded I would like to know what do you think about this.
I think that so far it doesn't make sense, as metadata are revisions (we don't have them while editable) and remote (same). If the package is in editable mode, I think doesn't make much sense to return PackageCacheLayout for it, so far the PacakgeEditableLayout have placeholder methods to provide better error messages.
Agree. But then what, how to solve this issue? Should the `remote remove` just ignore the editable layouts? When the package is no longer in "editable mode" it will have metadata with a wrong remote (removed).
I see, you are right.

I think there are 2 approaches:

- Explicit force to get the cache metadata, like: ``with self._cache.package_layout(ref, from-cache=True).update_metadata() as metadata:``
- Explicit disabling of ``editables``, like in the ``RemoteRegistry.remove`` make cache.editables = {} and at the end of the function restore it.

I think at the moment I prefer the later one, until we have more cases.

Commit message: fix missing call

Patch to review:
diff --git a/conans/client/cache/remote_registry.py b/conans/client/cache/remote_registry.py
index c202e8ec32a..3bd1b173c15 100644
--- a/conans/client/cache/remote_registry.py
+++ b/conans/client/cache/remote_registry.py
@@ -1,6 +1,7 @@
 import json
 import os
 from collections import OrderedDict, namedtuple
+from contextlib import contextmanager
 from six.moves.urllib.parse import urlparse
 
 from conans.errors import ConanException, NoRemoteAvailable
@@ -267,6 +268,16 @@ def _validate_url(self, url):
         else:
             self._output.warn("The URL is empty. It must contain scheme and hostname.")
 
+    @contextmanager
+    def _editables_metadata_from_cache(self):
+        """
+        Hide editable packages to get the cache layout instead of the editable one
+        """
+        editables = self._cache.editable_packages
+        self._cache.editable_packages = {}
+        yield
+        self._cache.editable_packages = editables
+
     def load_remotes(self):
         if not os.path.exists(self._filename):
             self._output.warn("Remotes registry file missing, "
@@ -284,13 +295,14 @@ def add(self, remote_name, url, verify_ssl=True, insert=None, force=None):
         renamed = remotes.add(remote_name, url, verify_ssl, insert, force)
         remotes.save(self._filename)
         if renamed:
-            for ref in self._cache.all_refs():
-                with self._cache.package_layout(ref).update_metadata() as metadata:
-                    if metadata.recipe.remote == renamed:
-                        metadata.recipe.remote = remote_name
-                    for pkg_metadata in metadata.packages.values():
-                        if pkg_metadata.remote == renamed:
-                            pkg_metadata.remote = remote_name
+            with self._editables_metadata_from_cache():
+                for ref in self._cache.all_refs():
+                    with self._cache.package_layout(ref).update_metadata() as metadata:
+                        if metadata.recipe.remote == renamed:
+                            metadata.recipe.remote = remote_name
+                        for pkg_metadata in metadata.packages.values():
+                            if pkg_metadata.remote == renamed:
+                                pkg_metadata.remote = remote_name
 
     def update(self, remote_name, url, verify_ssl=True, insert=None):
         self._validate_url(url)
@@ -301,52 +313,54 @@ def update(self, remote_name, url, verify_ssl=True, insert=None):
     def clear(self):
         remotes = self.load_remotes()
         remotes.clear()
-        for ref in self._cache.all_refs():
-            with self._cache.package_layout(ref).update_metadata() as metadata:
-                metadata.recipe.remote = None
-                for pkg_metadata in metadata.packages.values():
-                    pkg_metadata.remote = None
-        remotes.save(self._filename)
+        with self._editables_metadata_from_cache():
+            for ref in self._cache.all_refs():
+                with self._cache.package_layout(ref).update_metadata() as metadata:
+                    metadata.recipe.remote = None
+                    for pkg_metadata in metadata.packages.values():
+                        pkg_metadata.remote = None
+            remotes.save(self._filename)
 
     def remove(self, remote_name):
         remotes = self.load_remotes()
         del remotes[remote_name]
+        with self._editables_metadata_from_cache():
+            for ref in self._cache.all_refs():
+                with self._cache.package_layout(ref).update_metadata() as metadata:
+                    if metadata.recipe.remote == remote_name:
+                        metadata.recipe.remote = None
+                    for pkg_metadata in metadata.packages.values():
+                        if pkg_metadata.remote == remote_name:
+                            pkg_metadata.remote = None
 
-        for ref in self._cache.all_refs():
-            with self._cache.package_layout(ref).update_metadata() as metadata:
-                if metadata.recipe.remote == remote_name:
-                    metadata.recipe.remote = None
-                for pkg_metadata in metadata.packages.values():
-                    if pkg_metadata.remote == remote_name:
-                        pkg_metadata.remote = None
-
-        remotes.save(self._filename)
+            remotes.save(self._filename)
 
     def define(self, remotes):
         # For definition from conan config install
-        for ref in self._cache.all_refs():
-            with self._cache.package_layout(ref).update_metadata() as metadata:
-                if metadata.recipe.remote not in remotes:
-                    metadata.recipe.remote = None
-                for pkg_metadata in metadata.packages.values():
-                    if pkg_metadata.remote not in remotes:
-                        pkg_metadata.remote = None
+        with self._editables_metadata_from_cache():
+            for ref in self._cache.all_refs():
+                with self._cache.package_layout(ref).update_metadata() as metadata:
+                    if metadata.recipe.remote not in remotes:
+                        metadata.recipe.remote = None
+                    for pkg_metadata in metadata.packages.values():
+                        if pkg_metadata.remote not in remotes:
+                            pkg_metadata.remote = None
 
-        remotes.save(self._filename)
+            remotes.save(self._filename)
 
     def rename(self, remote_name, new_remote_name):
         remotes = self.load_remotes()
         remotes.rename(remote_name, new_remote_name)
+        with self._editables_metadata_from_cache():
+            for ref in self._cache.all_refs():
+                with self._cache.package_layout(ref).update_metadata() as metadata:
+                    if metadata.recipe.remote == remote_name:
+                        metadata.recipe.remote = new_remote_name
+                    for pkg_metadata in metadata.packages.values():
+                        if pkg_metadata.remote == remote_name:
+                            pkg_metadata.remote = new_remote_name
 
-        for ref in self._cache.all_refs():
-            with self._cache.package_layout(ref).update_metadata() as metadata:
-                if metadata.recipe.remote == remote_name:
-                    metadata.recipe.remote = new_remote_name
-                for pkg_metadata in metadata.packages.values():
-                    if pkg_metadata.remote == remote_name:
-                        pkg_metadata.remote = new_remote_name
-
-        remotes.save(self._filename)
+            remotes.save(self._filename)
 
     @property
     def refs_list(self):
diff --git a/conans/test/functional/command/remote_test.py b/conans/test/functional/command/remote_test.py
index ee6a25b77e6..3e7e9600dee 100644
--- a/conans/test/functional/command/remote_test.py
+++ b/conans/test/functional/command/remote_test.py
@@ -392,3 +392,43 @@ def test_invalid_url(self):
                       self.client.user_io.out)
         self.client.run("remote list")
         self.assertIn("pepe.org", self.client.out)
+
+    def test_metadata_editable_packages(self):
+        """
+        conan remote remove fails with editable packages
+        """
+        self.client.save({"conanfile.py": """from conans import ConanFile
+class Conan(ConanFile):
+    pass"""})
+        self.client.run("create . pkg/1.1@lasote/stable")
+        self.client.run("upload pkg/1.1@lasote/stable --all -c --remote remote1")
+        self.client.run("remove -f pkg/1.1@lasote/stable")
+        self.client.run("install pkg/1.1@lasote/stable")
+        self.assertIn("pkg/1.1@lasote/stable: Package installed", self.client.out)
+        self.client.run("remote list_ref")
+        self.assertIn("pkg/1.1@lasote/stable: remote1", self.client.out)
+        self.client.run("editable add . pkg/1.1@lasote/stable")
+        # Check add --force, update and rename
+        self.client.run("remote add remote2 %s --force" % self.servers["remote1"].fake_url)
+        self.client.run("remote update remote2 %sfake" % self.servers["remote1"].fake_url)
+        self.client.run("remote rename remote2 remote-fake")
+        self.client.run("editable remove pkg/1.1@lasote/stable")
+        # Check associated remote has changed name
+        self.client.run("remote list_ref")
+        self.assertIn("pkg/1.1@lasote/stable: remote-fake", self.client.out)
+        # Check remove
+        self.client.run("editable add . pkg/1.1@lasote/stable")
+        self.client.run("remote remove remote-fake")
+        self.client.run("remote list")
+        self.assertIn("remote0: %s" % self.servers["remote0"].fake_url, self.client.out)
+        self.assertNotIn("remote-fake", self.client.out)
+        # Check clean
+        self.client.run("editable remove pkg/1.1@lasote/stable")
+        self.client.run("remove -f pkg/1.1@lasote/stable")
+        self.client.run("remote add remote1 %s" % self.servers["remote1"].fake_url)
+        self.client.run("install pkg/1.1@lasote/stable")
+        self.client.run("editable add . pkg/1.1@lasote/stable")
+        self.client.run("remote clean")
+        self.client.run("remote list")
+        self.assertNotIn("remote1", self.client.out)
+        self.assertNotIn("remote0", self.client.out)


Code review comments:
diff_hunk: @@ -267,6 +268,16 @@ def _validate_url(self, url):
         else:
             self._output.warn("The URL is empty. It must contain scheme and hostname.")
 
+    @contextmanager
+    def _editables_metadata_from_cache(self):
+        """
+        Hide editable packages to get the cache layout instead of the editable one
+        """
+        editables = self._cache.editable_packages
+        self._cache.editable_packages = {}
path: conans/client/cache/remote_registry.py
text: @user1:
The member `editable_packages` is not a dictionary, it is an instance of `EditablePackages` class, we should override the `_edited_refs` member. I would like to keep the same type, one never knows if there is a `isinstance` check somewhere.