Skip to content
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

Schnapsidee: Favour Make conditionals over shell conditionals #912

Open
gouttegd opened this issue Aug 12, 2023 · 1 comment
Open

Schnapsidee: Favour Make conditionals over shell conditionals #912

gouttegd opened this issue Aug 12, 2023 · 1 comment
Milestone

Comments

@gouttegd
Copy link
Contributor

The standard ODK-generated Makefile contains several instances where the entire recipe of a rule is enclosed in a shell conditional construct (if... then... fi) based on a Make variable, such as in this example:

(TMPDIR)/pattern_owl_seed.txt: $(PATTERNDIR)/pattern.owl
        if [ $(PAT) = true ]; then $(ROBOT) query --use-graphs true -f csv -i $< --query ../sparql/terms.sparql $@; fi

Since what is tested in the condition is a Make variable, not a shell variable, this kind of construct could be replaced by a Make conditional instead:

ifeq ($(PAT),true)
(TMPDIR)/pattern_owl_seed.txt: $(PATTERNDIR)/pattern.owl
        $(ROBOT) query --use-graphs true -f csv -i $< --query ../sparql/terms.sparql $@
endif

I think this would be less surprising to readers, and easier to debug when things go wrong.

We’d need to be careful about prerequisites, though, because the two above constructs are not strictly equivalent: in the first case (shell conditional), even if the recipe does nothing when PAT is false, the rule is still there and its prerequisites are evaluated (so here, Make will find that it needs to build $(PATTERNDIR)/pattern.owl), whereas in the second case (Make conditional), the entire rule is skipped when PAT is false, including its prerequisites. But we should always be careful about prerequisites anyway, whether we use shell or Make conditionals.

This idea would also apply to the various custom.Makefile that exist in the wild, where constructs similar to those in use in the ODK Makefile are also widely used.

@matentzn
Copy link
Contributor

That does not sound like a Schnapsidee at all :D Yeah, I support this. I think this is much better, and the reason it does not exist is simply that when I first introduced that construct, I did not know the make conditionals were a thing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants