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

Feature/inspect #3634

Merged
merged 18 commits into from Oct 8, 2018

Conversation

Projects
None yet
4 participants
@memsharded
Copy link
Contributor

commented Sep 28, 2018

Changelog: Feature: New conan inspect command that provides individual attributes of a recipe, like name, version, or options. Work with -r=remote repos too, and is able to produce --json output.

Close #1985
Close #3269

Docs in: conan-io/docs#866

@memsharded memsharded added this to the 1.8 milestone Sep 28, 2018

@ghost ghost assigned memsharded Sep 28, 2018

@ghost ghost added the stage: review label Sep 28, 2018

@lasote
Copy link
Contributor

left a comment

  • Missing changelog
  • What about python_requires?
@danimtb
Copy link
Member

left a comment

Here we have another example of confusing option types again

client.save({"conanfile.py": conanfile})
client.run("inspect . -a=options")
self.assertEquals(client.out, """options:
option1: ['False', 'True'], default=False

This comment has been minimized.

Copy link
@danimtb

danimtb Oct 1, 2018

Member

Shouldn't it be option1: [False, True], default=False ?

This comment has been minimized.

Copy link
@memsharded

memsharded Oct 1, 2018

Author Contributor

The options values are always text values. I am not adding the quotes, it is the output because those values are actually strings. I am not sure how to improve that, because you cannot just unquote, because it is possible that the user already provided them as strings, and there is now way to know.

client.run("inspect . -a=options")
self.assertEquals(client.out, """options:
option1: ['False', 'True'], default=False
option2: ['1', '2', '3'], default=2

This comment has been minimized.

Copy link
@danimtb

danimtb Oct 1, 2018

Member

Same here, I think the output should be option2: [1, 2, 3], default=2

@memsharded memsharded removed their assignment Oct 1, 2018

@memsharded

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2018

Even if using a temporary "data" folder, the registry becomes dirty. Either:

  • Inspect will retrieve recipes, exactly the same as conan info does
  • Inspect doesn't inspect in remotes.
  • Backup the registry.txt and restore it after the command (concurrency?).

I'll say go for 1st one, not a big deal, isn't it? Delaying to 1.9.

@memsharded memsharded modified the milestones: 1.8, 1.9 Oct 3, 2018

@uilianries

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

Inspect doesn't inspect in remotes.

This remember me about a Red Hat story when they needed to inspect remote Docker images and the project didn't accept such feature request, so Red Hat created CRI-O -- Of course a docker image could have 10GB and download it just to inspect metainfo could be a pain.

@ghost ghost assigned memsharded Oct 7, 2018

@memsharded memsharded modified the milestones: 1.9, 1.8 Oct 7, 2018

@memsharded

This comment has been minimized.

Copy link
Contributor Author

commented Oct 7, 2018

This remember me about a Red Hat story when they needed to inspect remote Docker images and the project didn't accept such feature request, so Red Hat created CRI-O -- Of course a docker image could have 10GB and download it just to inspect metainfo could be a pain.

Thanks for this feedback. However there are 2 things here to take into account:

  • There is no way that parsing python recipes in server to extract data will be a thing. Dangerous, complex... it is not going to happen
  • The conan recipes are very lightweight in 99,99% of the cases. Unless you exports something very big, it will be just a few bytes. And such exports is very likely to be a mistake, it won't be necessary and could be replaced with exports_sources.
@lasote
Copy link
Contributor

left a comment

Minor details, please review.


args = parser.parse_args(*args)
result = self._conan.inspect(args.path_or_reference, args.attribute, args.remote)
for k, v in result.items():

This comment has been minimized.

Copy link
@lasote

lasote Oct 8, 2018

Contributor

Move this to printer.py

@@ -32,7 +32,8 @@
from conans.model.version import Version
from conans.paths import get_conan_user_home, CONANINFO, BUILD_INFO
from conans.util.env_reader import get_env
from conans.util.files import save_files, exception_message_safe, mkdir
from conans.util.files import save_files, exception_message_safe, mkdir,\
mkdir_tmp

This comment has been minimized.

Copy link
@lasote

lasote Oct 8, 2018

Contributor

Unused mkdir_tmp

from conans.client.graph.graph_manager import GraphManager
from conans.client.loader import ConanFileLoader
from conans.client.graph.proxy import ConanProxy
from conans.client.graph.python_requires import ConanPythonRequire
from conans.client.graph.range_resolver import RangeResolver
from collections import OrderedDict
import shutil

This comment has been minimized.

Copy link
@lasote

lasote Oct 8, 2018

Contributor

Unused shutil?

@danimtb

danimtb approved these changes Oct 8, 2018

@lasote

lasote approved these changes Oct 8, 2018

@lasote lasote merged commit a75f348 into conan-io:develop Oct 8, 2018

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Oct 8, 2018

@uilianries

This comment has been minimized.

Copy link
Member

commented Oct 8, 2018

* The conan recipes are very lightweight in 99,99% of the cases. Unless you `exports` something very big, it will be just a few bytes. And such exports is very likely to be a mistake, it won't be necessary and could be replaced with `exports_sources`.

I agree, recipes are really lightweight and we don't need binary packages to inspect. :)

@memsharded memsharded deleted the memsharded:feature/inspect branch Oct 8, 2018

grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018

Feature/inspect (conan-io#3634)
* feature display

* display command and preexport method

* add display to commands help

* fix test

* minor improvements

* blank lines

* inspect command

* inspect

* inspect

* using temporary data folder for cache

* inspect fetches recipes into local cache

* review

* making sure update=True for remote inspects

* changed conan inspect -a=options to simple
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.