-
Notifications
You must be signed in to change notification settings - Fork 1k
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] Add hooks for validate and validate_build methods #17856
base: develop2
Are you sure you want to change the base?
Conversation
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
@@ -40,6 +41,8 @@ def __init__(self, conan_app, global_conf): | |||
python_mode = global_conf.get("core.package_id:default_python_mode", default="minor_mode") | |||
build_mode = global_conf.get("core.package_id:default_build_mode", default=None) | |||
self._modes = unknown_mode, non_embed, embed_mode, python_mode, build_mode | |||
self._hook_manager = HookManager(HomePaths(self._home_folder).hooks_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can make the batch processing at scale in repos like ConanCenter very slow, as this will be instantiated tons of times. Need to be factored out from here (this might be complicated and require some re-architecture)
cc @AbrilRBS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just need to access the Conan App to capture the hook paths, then, instantiate the hook manager. It would be great doing it in run_validate_package_id
directly, but I have no access to Conan App. Is there a way to capture the Conan App globally? I also don't like this current approach of forwarding the hook manager til run_validate_package_id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there's an easy lazy load optimization available for the HookManager that would solve the issue, something like this might be worth looking into:
that would almost solve it, but you still would need to modify the class instance instead of self._hooks
to avoid the slowness:
diff --git a/conans/client/hook_manager.py b/conans/client/hook_manager.py
index d213b209d..9eb87bdb3 100644
--- a/conans/client/hook_manager.py
+++ b/conans/client/hook_manager.py
@@ -12,11 +12,18 @@ valid_hook_methods = ["pre_export", "post_export",
class HookManager:
+ _hooks = None
def __init__(self, hooks_folder):
self._hooks_folder = hooks_folder
- self.hooks = {}
- self._load_hooks() # A bit dirty, but avoid breaking tests
+
+ @property
+ def hooks(self):
+ if self._hooks is None:
+ # Have this before the call below, or its accesses to hook will be recursive
+ self._hooks = {}
+ self._load_hooks()
+ return self._hooks
def execute(self, method_name, conanfile):
assert method_name in valid_hook_methods, \
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then if you access/modify just HookManager._hooks
, you'll have issues with the tests keping states between each call so, pick your poison I guess
Motivation
Hello!
This PR brings a new hooks for validate() and validate_build() methods.
The intention is to help CI services validate a build without touching the recipe, by adding an extra layer of validations. For instance, the CI environment could not build a determined package due to custom settings, or some system library is not available because the O.S. is too old.
Details
Both
validate
andvalidate_build
don't have a specific command, like export or build, so I usedconan create
to get a ride and check if is working.Also, both methods are not defined by default, which means, the hook will only run when one of these methods is defined in the recipe.
Automation
Changelog: Feature: Add hooks pre_validate, post_validate, pre_validate_build and post_validate_build
Docs: TODO
/cc @jcar87
develop
branch, documenting this one.