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

Conan plugins #3555

Merged
merged 47 commits into from Sep 24, 2018

Conversation

Projects
None yet
5 participants
@danimtb
Copy link
Member

commented Sep 14, 2018

  • Refer to the issue that supports this Pull Request: Related to #480, #773, #1115, #2342
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code: Actually some PEP8 compromised for test readability
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one. Also adding a description of the changes in the changelog.rst file. https://github.com/conan-io/docs

Plugins are stored ~/.conan/plugins folder. They can be activated in the conan.conf via general.plugins and CONAN_PLUGINS both accepting a list.

This includes a default activated plugin called attribute_checker that includes the basic checks of url/description/license (Removed from code).

The available methods for a plugin to define are:

def pre_export(output, conanfile, conanfile_path, reference, **kwargs):

def post_export(output, conanfile, conanfile_path, reference, **kwargs):

def pre_source(output, conanfile, conanfile_path, **kwargs):

def post_source(output, conanfile, conanfile_path, **kwargs):

def pre_build(output, conanfile, **kwargs):

def post_build(output, conanfile, **kwargs):

def pre_package(output, conanfile, conanfile_path, **kwargs):

def post_package(output, conanfile, conanfile_path, **kwargs):

def pre_upload(output, conanfile_path, reference, remote, **kwargs):

def post_upload(output, conanfile_path, reference, remote, **kwargs):

def pre_upload_recipe(output, conanfile_path, reference, remote, **kwargs):

def post_upload_recipe(output, conanfile_path, reference, remote, **kwargs):

def pre_upload_package(output, conanfile_path, reference, package_id, remote, **kwargs):

def post_upload_package(output, conanfile_path, reference, package_id, remote, **kwargs):

def pre_download(output, reference, remote, **kwargs):

def post_download(output, conanfile_path, reference, remote, **kwargs):

def pre_download_recipe(output, reference, remote, **kwargs):

def post_download_recipe(output, conanfile_path, reference, remote, **kwargs):

def pre_download_package(output, conanfile_path, reference, package_id, remote, **kwargs):

def post_download_package(output, conanfile_path, reference, package_id, remote, **kwargs):

Changelog: Feature: Experimental support for Conan plugins.

plugin_manager.execute("pre_package", conanfile=conanfile, conanfile_path=conanfile_path,
reference=str(reference), package_id=pkg_id)

package_output = ScopedOutput("%s package()" % output.scope, output)

This comment has been minimized.

Copy link
@jgsogo

jgsogo Sep 17, 2018

Member

This package_output is not being used!

This comment has been minimized.

Copy link
@danimtb

danimtb Sep 17, 2018

Author Member

Yes, it is only used for the copy report some lines below

This comment has been minimized.

Copy link
@jgsogo

jgsogo Sep 19, 2018

Member

uupss! 😓

@danimtb danimtb assigned lasote and unassigned danimtb Sep 19, 2018

@jgsogo

jgsogo approved these changes Sep 19, 2018

def plugins(self):
"""Returns a list of plugins inside the plugins folder"""
plugins = []
for plugin_name in os.listdir(self.plugins_path):

This comment has been minimized.

Copy link
@lasote

lasote Sep 20, 2018

Contributor

if os.path.isfile(plugin_name)

Also, don't forget to mention in the docs that any py file there will be considered a plugin, and suggest subfolders.

@@ -188,6 +204,10 @@ def get_recipe(self, conan_reference, remote):
# Make sure that the source dir is deleted
rm_conandir(self._client_cache.source(conan_reference))
touch_folder(dest_folder)
conanfile_path = self._client_cache.conanfile(conan_reference)
self._plugin_manager.execute("post_download_recipe", conanfile_path=conanfile_path,
reference=str(conan_reference),

This comment has been minimized.

Copy link
@lasote

lasote Sep 20, 2018

Contributor

i'm not sure.

If passing a conanfile object, something in the conans namespace

Well, the conanfile is something in the userspace too, is the self within a recipe.
But I don't know, I agree that if they are going to use .name, .version, .user, .channel is useful to pass (and document, of course) a ConanFileReference.

On the other hand, if you receive a ConanFileReference you have to know that it is not comparable with a string directly nor formatted in a string without casting to str.
You could always ref = ConanFileReference.loads(ref).

But yeah, probably people will be going to check the channels and things like that, so let's pass references, also for future revisions, not included in a regular str(ref) but a ref.full_reference(). The str conversion/formatting and object in general HAS TO BE well documented in the plugin section.

@@ -147,7 +149,7 @@
class ConanClientConfigParser(ConfigParser, object):

def __init__(self, filename):
ConfigParser.__init__(self)
ConfigParser.__init__(self, allow_no_value=True)

This comment has been minimized.

Copy link
@lasote

lasote Sep 20, 2018

Contributor

allow_no_value not used, right?

This comment has been minimized.

Copy link
@danimtb

danimtb Sep 20, 2018

Author Member

I think it is actually needed for conan config set plugins.my_plugin as the normal behaviour would be conan config set plugins.my_plugin=plugin_value_if_something

This comment has been minimized.

Copy link
@danimtb

danimtb Sep 20, 2018

Author Member

Yes, that parameter is needed

@@ -41,10 +41,6 @@ def errors_test(self):
self.assertIn("You can't set a full section, please specify a key=value",
self.client.user_io.out)

error = self.client.run('config set proxies.http:Value', ignore_error=True)

This comment has been minimized.

Copy link
@lasote

lasote Sep 20, 2018

Contributor

It is a bit dirty but I would prefer even check if the property being set belongs to "plugins" and allow only in that case, recovering the test. I don't like to deteriorate the current behavior.

@ghost ghost assigned danimtb Sep 20, 2018

danimtb added some commits Sep 20, 2018

@danimtb danimtb removed their assignment Sep 20, 2018

@memsharded

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2018

@danimtb for the documentation, do not document ConanFileReference. Document that what is passed to the plugins is an object with .name, .version and the str() method converts it to string. So we can decouple it in the future.

@lasote

lasote approved these changes Sep 21, 2018

@lasote

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2018

Resolve conflicts!

@ghost ghost assigned danimtb Sep 21, 2018

@memsharded
Copy link
Contributor

left a comment

I have a question: I want to run a "binary package quality" check plugin, to both the packages that I am building, but also to the packages I am retrieving from remote servers. The same applies to recipes, what if I want to check something in every recipe that is either exported into the cache, or it is downloaded into the cache from a remote.

@@ -59,6 +57,10 @@ def cmd_export(conanfile_path, conanfile, reference, keep_source, output, client
with client_cache.conanfile_write_lock(reference):
_export_conanfile(conanfile_path, conanfile.output, client_cache, conanfile, reference,
keep_source)
conanfile_cache_path = client_cache.conanfile(reference)
assert os.path.exists(conanfile_cache_path)

This comment has been minimized.

Copy link
@memsharded

memsharded Sep 21, 2018

Contributor

assert can be removed

@danimtb

This comment has been minimized.

Copy link
Member Author

commented Sep 24, 2018

You can make those checks in post_package() when creating those packages locally and in post_download_package() when downloading them from remotes. For the first one you would have the conanfile.package_folder to inspect the binary files, for the second one I guess you would need to compound the path to the package folder using the conanfile_path argument.

@lasote

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

I think it should work for the use case you mentioned @memsharded. But with the real usage, we will polish it (experimental). We have to reserve some time to implement some real plugin like the ones we want for conan-center or a cool binary linter for libraries. Do you see any architectural issue right now @memsharded ?

@memsharded

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

Yeah, agree, can be iterated.

Architecture is good, lets merge this!

@lasote lasote merged commit c352051 into conan-io:develop Sep 24, 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 Sep 24, 2018

@danimtb danimtb referenced this pull request Sep 25, 2018

Merged

Plugins documentation #846

@solvingj

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2018

Where does one go for continued discussion on this feature?

@memsharded

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2018

@solvingj

Open a new issue with [plugin] in the title. We also have a "plugin" label for them. There is also the https://github.com/conan-io/plugins repo, for discussions, PR about specific plugins, could be done there, I guess.

@jgsogo

This comment has been minimized.

Copy link
Member

commented Oct 9, 2018

👍 to move the discussion about plugins themselves to the new repo.

Some issues labeled as plugins should be moved there as they are more a plugin request than an issue related to the plugins architecture. I think that @danimtb and I can take care of this task.

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

Conan plugins (conan-io#3555)
* plugin loader

* Added plugin folder and conf

* POC linter plugin

* Execute method of all plugins

* PluginManager with initialize and execute

* moved output to pluginmanager and use conanfile path

* scoped output and removed reference

* added recipe plugin and moved to plugin module

* default plugin creation and deinitilize

* Initial tests

* initialize output

* plugin default initial test

* Completed default test and fixed PY2 import module

* remove execfile

* added plugin manager to install process

* check parameters for each method

* fixed tests

* Removed ConanPlugin and changed to execute

* remove plugins load from api wrapper

* fixed plugins basic tests

* minimal complete test

* complete test and moved plugin manager to remote manager

* moved plugin manager to cmd_export

* renamed plugins file and check output exception

* use conanfile cache path for post_export

* finish check path

* fix some tests

* removed file and fixed some tests

* remove changes in command

* removed asserts from test

* partial review

* review plugins and added import one

* review: lazy default plugins creation, valid methods, optimization & [plugins]

* review 2

* Revert test and file check

* used ConanFileReference

* remove load import

* removed assert
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.