Download component from source url workflow can't find file to copy

[anitacaron]

When the component is different, it copies from the tmp folder, but when it's downloading for the first time, it shouldn't try to copy.

Error on test PR https://github.com/obophenotype/uberon/actions/runs/8389532879/job/22975944727

[anitacaron]

I found out that the issue is in the QC. Now, we need to set MIR=false, and to download the component, the condition is MIR=true.

ontology-development-kit/template/src/ontology/Makefile.jinja2

Lines 615 to 618 in dbcab69

	component-download-{{ component.filename }}: | $(TMPDIR)
	if [ $(MIR) = true ] && [ $(COMP) = true ]; then $(ROBOT) merge -I {{ component.source }} \{%- if component.make_base %}
	remove {% if component.base_iris is not none %}{% for iri in component.base_iris %}--base-iri {{iri}} {% endfor %}{% else %}--base-iri $(OBOBASE)/{{ project.id.upper() }} {% endif %}--axioms external --preserve-structure false --trim false \{% endif %}
	annotate --ontology-iri $(ONTBASE)/$@ $(ANNOTATE_ONTOLOGY_VERSION) -o $(TMPDIR)/[email protected]; fi

What are your thoughts? @matentzn @gouttegd

[gouttegd]

I think that deep down, it’s all a consequence of the old approach of using shell conditionals instead of Make conditionals (see #912). Make is unaware of shell conditionals, so when you have a rule like this:

component-download-{{ component.filename }}: | $(TMPDIR) 
 	if [ $(MIR) = true ] && [ $(COMP) = true ]; then ...ALL THE CODE OF THE RULE... ; fi

where the entirety of what the rule does is within a shell conditional, when the condition is false Make will have no idea that the rule didn’t actually do anything. From Make’s point of view, the shell code has been executed, that’s all that matters.

We should convert all those shell conditionals to Make conditionals as we recently did for the mirroring rules (#973).

Something like that:

ifeq ($(COMP),true)
# COMP=true -> we regenerate the components

ifeq ($(MIR),true)
# MIR=true -> even the components that are mirrored from a foreign source

.PHONY: component-download-hra_subset.owl
component-download-hra_subset.owl: | $(TMPDIR)
	$(ROBOT) merge -I https://raw.githubusercontent.com/hubmapconsortium/ccf-validation-tools/master/owl/UB_ASCTB_subset.owl \
	annotate --ontology-iri $(ONTBASE)/$@ $(ANNOTATE_ONTOLOGY_VERSION) -o $(TMPDIR)/$@.owl

$(COMPONENTSDIR)/hra_subset.owl: component-download-hra_subset.owl
	if cmp -s $(TMPDIR)/component-download-hra_subset.owl.owl $(TMPDIR)/component-download-hra_subset.owl.tmp.owl ; then echo "Component identical."; \
      else echo "Component is different, updating." &&\
		cp $(TMPDIR)/component-download-hra_subset.owl.owl $(TMPDIR)/component-download-hra_subset.owl.tmp.owl &&\
		$(ROBOT) annotate -i $(TMPDIR)/component-download-hra_subset.owl.owl --ontology-iri $(ONTBASE)/$@ $(ANNOTATE_ONTOLOGY_VERSION) -o $@; fi

.PRECIOUS: $(COMPONENTSDIR)/hra_subset.owl

endif

# Here, we can re-generate the other components (those that don't depend
# on a foreign source, and therefore that can be built even under MIR=false)

else

# COMP=false -> Not regenerating any component; if any rule depends on a
# component, Make would simply notice that the component files already exist
# in `$(COMPONENTSDIR)` (assuming the components are committed to the
# repository, which they normally should be), and will not do anything.

endif

[anitacaron]

Yeah, but still, if MIR=false, it won't download the component, right?

[gouttegd]

No indeed, but isn’t that what is wanted here? We only want to refresh that component when MIR=true.

[anitacaron]

Actually, the second goal only checks for COMP=true and not the MIR. I agree that using the make condition would be clearer.

ontology-development-kit/template/src/ontology/Makefile.jinja2

Lines 620 to 624 in dbcab69

	$(COMPONENTSDIR)/{{ component.filename }}: component-download-{{ component.filename }}
	if [ $(COMP) = true ]; then if cmp -s $(TMPDIR)/component-download-{{ component.filename }}.owl $(TMPDIR)/component-download-{{ component.filename }}.tmp.owl ; then echo "Component identical."; \
	else echo "Component is different, updating." &&\
	cp $(TMPDIR)/component-download-{{ component.filename }}.owl $(TMPDIR)/component-download-{{ component.filename }}.tmp.owl &&\
	$(ROBOT) annotate -i $(TMPDIR)/component-download-{{ component.filename }}.owl --ontology-iri $(ONTBASE)/$@ $(ANNOTATE_ONTOLOGY_VERSION) -o $@; fi; fi

[anitacaron]

I'll create the PR, thanks.

[gouttegd]

Actually, the second goal only checks for COMP=true and not the MIR

Yeah, but since the rule that this one depends on does also check MIR, this does not make any sense. In effect, and unless we happen to have a $(TMPDIR)/component-download-{{ component.filename }}.owl file that would be a leftover from a previous run (the kind of things we should definitely not rely upon), we can only regenerate that component when both MIR and COMP are true. I assume this is the desired behaviour.