-
Notifications
You must be signed in to change notification settings - Fork 54
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
Ensure that ROBOT plugin dir exists before download #1044
Conversation
Similar to the default goal, we need to make sure that the robot plugin dir exists before external plugins are downloaded.
This should not be necessary. Plugins defined in the configuration file are listed after the default plugins (the plugins that are bundled with the ODK, for now Have you run into a case when this does not happen? |
Not 100% certain that is the reason, but seems so? |
OK, the problem is that the rule in the components/upheno-relations.owl: $(SRCMERGED) | $(ROBOT_PLUGINS_DIRECTORY)/upheno.jar The documented way of using plugins is to depend on |
Wouldnt a direct dependency on a .PHONY goal mean that the |
Why? The |
FWIW I got it from https://github.com/obophenotype/uberon/blob/f33fde102f005c892537e116532ce06d7293a2d7/src/ontology/uberon.Makefile#L1153 Can you send me another example where I see what you mean? For now I am doing this: But is there any harm in merging this here in case someone wants to depend just on the one jar rather than all the plugins? |
Yes, because Uberon started using plugins before the feature was available in the ODK. There was no standard
What is there to see? All you have to do is to depend on $(EDIT_PREPROCESSED): $(SRC) all_robot_plugins
$(ROBOT) flybase:rewrite-def -i $< --dot-definitions --filter-prefix FBbt -o $@ with the FlyBase plugin being declared here: robot_plugins:
plugins:
- name: flybase
mirror_from: https://github.com/FlyBase/flybase-robot-plugin/releases/download/flybase-robot-plugin-0.1.1/flybase.jar
No harm, but I don't understand why you would prefer to do that rather than just fixing |
This is my problem. Imagine this Makefile:
If you run this for the first time, it runs everything as expected:
If you run this for the second time, it correctly determines there is nothing to be done:
Now, if you put the
this is what happens:
So far so good. Now the problem. If you run the same goal for the second time all dependencies get executed again:
If we were able to depend on plugins directly as files, this problem would not occur! |
Seems to me that you can fix that simply by making the dependency to the .PHONY: phony_goal
phony_goal:
@echo "This is a phony goal"
tmp/hugefile.owl: | phony_goal
@echo "ENORMOUS TIME CONSUMING STEP IN MAKE"
touch $@
tmp/release.owl: tmp/hugefile.owl
@echo "UPDATE"
touch $@ By doing that, you’re basically declaring that all that you need is for the This wouldn’t be appropriate if there was a chance that the outcome of the |
Anyway, feel free to merge this if you want. But the official and documented way to use plugins will still be to depend on |
Nah, you are right, the |
Similar to the default goal, we need to make sure that the robot plugin dir exists before external plugins are downloaded.