Ensure that ROBOT plugin dir exists before download

[matentzn]

Similar to the default goal, we need to make sure that the robot plugin dir exists before external plugins are downloaded.

[gouttegd]

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 kgcl and sssom) in the prerequisites of all_robot_plugins, so the rule that installs those default plugins is always called before any rule that installs an extra plugin -- and that rule takes care of creating the $TMPDIR/plugins directory. So that directory should always already exists by the time we try to install supplementary plugins.

Have you run into a case when this does not happen?

[matentzn]

https://github.com/obophenotype/human-phenotype-ontology/actions/runs/8588313034/job/23532997472#step:4:1

Not 100% certain that is the reason, but seems so?

[gouttegd]

OK, the problem is that the rule in the upheno.Makefile tries to depend directly on a specific plugin:

components/upheno-relations.owl: $(SRCMERGED) | $(ROBOT_PLUGINS_DIRECTORY)/upheno.jar

The documented way of using plugins is to depend on all_robot_plugins -- that's how we ensure the plugins directory is always created.

[matentzn]

Wouldnt a direct dependency on a .PHONY goal mean that the make cache will not work properly? I guess I could stick a $(MAKE) all_robot_plugins inside the rule..

[gouttegd]

Why? The all_robot_plugins rule will always be invoked, but if it had been invoked before Make will then realise that its dependencies already exist and that there is nothing to do.

[matentzn]

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:
https://github.com/obophenotype/human-phenotype-ontology/pull/10446/files#diff-1ab39677a03254561c3579b6ca5fb769cafbbb875c41703f9fcee8bc0bab6d45

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?

[gouttegd]

FWIW I got it from https://github.com/obophenotype/uberon/blob/f33fde102f005c892537e116532ce06d7293a2d7/src/ontology/uberon.Makefile#L1153

Yes, because Uberon started using plugins before the feature was available in the ODK. There was no standard all_robot_plugins target back then. This should be updated to make use of the standard rules and targets.

Can you send me another example where I see what you mean?

What is there to see? All you have to do is to depend on all_robot_plugins. But you can have a look at FBbt here:

$(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

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?

No harm, but I don't understand why you would prefer to do that rather than just fixing upheno.Makefile. Any change you're making here would not find its way into uPheno until the next ODK release, whereas you can fix the original problem in uPheno in a sec.

[matentzn]

This is my problem. Imagine this Makefile:

.PHONY: phony_goal
phony_goal:
	@echo "This is a phony goal"

tmp/hugefile.owl:
	@echo "ENORMOUS TIME CONSUMING STEP IN MAKE"
	touch $@

tmp/release.owl: tmp/hugefile.owl
	@echo "UPDATE" 
	touch $@

If you run this for the first time, it runs everything as expected:

(.venv) ➜  ontology git:(test) ✗ make tmp/release.owl
ENORMOUS TIME CONSUMING STEP IN MAKE
touch tmp/hugefile.owl
UPDATE
touch tmp/release.owl

If you run this for the second time, it correctly determines there is nothing to be done:

(.venv) ➜  ontology git:(test) ✗ make tmp/release.owl
make: `tmp/release.owl' is up to date.

Now, if you put the phony_goal as a dependency,

.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 $@

this is what happens:

(.venv) ➜  ontology git:(test) ✗ make tmp/release.owl
This is a phony goal
ENORMOUS TIME CONSUMING STEP IN MAKE
touch tmp/hugefile.owl
UPDATE
touch tmp/release.owl

So far so good. Now the problem. If you run the same goal for the second time all dependencies get executed again:

(.venv) ➜  ontology git:(test) ✗ make tmp/release.owl
This is a phony goal
ENORMOUS TIME CONSUMING STEP IN MAKE
touch tmp/hugefile.owl
UPDATE
touch tmp/release.owl

If we were able to depend on plugins directly as files, this problem would not occur!

[gouttegd]

Seems to me that you can fix that simply by making the dependency to the phony_goal an “order-only dependency”:

.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 phony_goal (i.e. the all_robot_plugins) to have been invoked prior to tmp/hugefile.owl, without requiring that it must be invoked again and trigger a rebuild of all the rules that depends on it.

This wouldn’t be appropriate if there was a chance that the outcome of the phony_goal might differ from one run to the next. But here, this is not the case. Once the plugins have been installed, there is no need to ever installing them again.

[gouttegd]

Anyway, feel free to merge this if you want. But the official and documented way to use plugins will still be to depend on all_robot_plugins. If people start depending on individual plugin files and things start to break, that won’t be my problem.

[matentzn]

Nah, you are right, the order-only dependency solution works. Better have one documented way of doing things! Didnt know this was possible! Thanks!