Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not raise when accessing the metadata of editable packages #5461

Merged
merged 10 commits into from Jul 29, 2019
@@ -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 = {}
This conversation was marked as resolved by danimtb

This comment has been minimized.

Copy link
@jgsogo

jgsogo Jul 15, 2019

Member

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.

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():

This comment has been minimized.

Copy link
@SSE4

SSE4 Jul 10, 2019

Contributor

is it possible to exract method? similar code repeats 5 times

This comment has been minimized.

Copy link
@danimtb

danimtb Jul 10, 2019

Author Member

IMO that should be done in a refactor PR, not in this one. I don't refuse to make the changes here but I would like to wait for other reviews before. Thanks!

This comment has been minimized.

Copy link
@SSE4

SSE4 Jul 10, 2019

Contributor

okay

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):
@@ -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
This conversation was marked as resolved by danimtb

This comment has been minimized.

Copy link
@jgsogo

jgsogo Jul 15, 2019

Member

Be positive ;D

Suggested change
conan remote remove fails with editable packages
Check that 'conan remote' commands work 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
This conversation was marked as resolved by danimtb

This comment has been minimized.

Copy link
@lasote

lasote Jul 9, 2019

Contributor

assert

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")
This conversation was marked as resolved by danimtb

This comment has been minimized.

Copy link
@lasote

lasote Jul 9, 2019

Contributor

assert

self.client.run("remote list")
self.assertNotIn("remote1", self.client.out)
self.assertNotIn("remote0", self.client.out)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.