-
Notifications
You must be signed in to change notification settings - Fork 53
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
Deliver ROBOT plugins in ODK #968
Conversation
Within an ODK workflow, ROBOT plugins should be put into the $(TMPDIR)/plugins directory for ROBOT to find them. This commit updates the ODK-generated Makefile to: *) export the ROBOT_PLUGINS_DIRECTORY variable, so that ROBOT knows where to look for plugins; *) add a all_robot_plugins target, that automatically copies any ODK-provided plugin (currently, only the SSSOM plugin) to the intended plugins directory. Therefore, rules that require the use of a plugin (be they standard rules or rules from the custom Makefile) only need to depend on the all_robot_plugins target. There are two ways for a repository to add extra plugins: either by putting them into a top-level "plugins" directory (the "all_robot_plugins" rule will ensure that they got copied to the runtime plugins directory in $(TMPDIR)), or by overriding the "custom_robot_plugins" rule in the custom Makefile.
Add a (optional) "robot_plugins" section to the ODK configuration file where users can list supplementary ROBOT plugins that are not provided by the ODK. Plugins listed here will be automatically installed by the "all_robot_plugins" rule, without requiring users to manually override the "custom_robot_plugins" rule.
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.
great!! I will test this a bit later, but the design looks good.
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.
Sorry meant to say "comment". Approval after testing !
Initial review dismissed because it was submitted prematurely.
Of note, I am open to the argument that this may be too flexible: maybe the new ODK configuration section is not needed, and it would actually be enough if supplementary plugins can only be installed through custom rules in the custom Makefile, without the possibility of declaring them in the |
FYI: Why did you use a fork for this PR? Makes it a (tiny) bit more cumbersome for me to test (not much just a bit) |
Didn’t do that on purpose, I just forgot that on my local copy the |
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.
As this is tested by you, I am included to leave at my line by line review and not test it (I am a bit out of breath). If you feel like I should test this, please merge this into the dev
branch and I will do it next week.
I am happy with the amount of testing I’ve done myself. In any case the PR can’t break an existing standard workflow, since there are currently no standard workflows to deal with plugins. What’s missing now is documenting the plugin delivery mechanism – such a mechanism would not be very useful if people don’t know how to use it (or even that it exists)… Since we agree on the mechanism itself (so it won’t be changed), I’ll do that shortly. |
Document how ROBOT plugins must be declared before they can be used.
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 cant see any other obvious problem. I tested three pipelines and couldn't see an obvious issue.
This PR proposes a method to manage ROBOT plugins in the ODK. The aims are:
The core principle is that we define a runtime plugins directory, which is
$(TMPDIR)/plugins
. The$(ROBOT_PLUGINS_DIRECTORY)
variable is defined to point to that directory and exported to the environment, so that ROBOT can find any plugin located in that directory.Once that principle is posed, the rest is only a matter of ensuring that the plugins end up being placed in that directory. This is done by the
all_robot_plugins
target. Any rule (whether it is a standard rule in the ODK-generated Makefile or a custom rule in the custom Makefile) that requires the use of a plugin should depend on that target, to ensure that the plugins are ready to use.There are several ways for a user to use extra plugins and/or to override the ODK-provided plugins:
A) Putting the plugin(s) they want to use in a top-level
plugins
directory. The default rule to copy plugins into the runtime directory will look into that top-levelplugins
directory in addition to looking into the ODK’s/tools/robot-plugins
directory, so any plugin found there will be automatically copied as if it had been provided by the ODK. The top-levelplugins
directory takes precedence over the ODK’s/tools/robot-plugins
directory.B) Using custom rules in the custom Makefile. To override a ODK-provided plugin, one may override the default installation rule with a specific rule. For example, to force the use of version 0.7.0 of the SSSOM plugin (instead of whatever version is provided in the ODK), one may do the following:
To install a supplementary plugin (instead of overriding a default one), one can override the
custom_robot_plugins
target, which is itself a dependency ofall_robot_plugins
. For example, to add the KGCL plugin:C) Explicitly listing the supplementary plugins in the ODK configuration. This PR adds a new possible configuration section in the ODK config file, allowing to list extra plugins to install. For example, another way of adding the KGCL plugin would be to amend the ODK configuration as follows:
The generated Makefile will then contain the required code to automatically download that plugin and place it into the runtime plugin directory.
If no
mirror_from
key is specified, then the ODK-generated Makefile will contain a dummy rule instead – it will be up to the user to override that rule in the custom Makefile.closes #909