Refactor the Makefile template by gouttegd · Pull Request #20 · INCATools/odkcore

gouttegd · 2026-08-09T20:41:45Z

This PR will completely refactor and re-format the standard Makefile template, as planned in INCATools/ontology-development-kit#1217.

There is no feature change, apart from the fact some remote files that were previously downloaded using curl or wget (such as remote SSSOM mapping sets or remote DOSDP templates) are now downloaded using the ODK’s own odk-helper download command (already used for mirrored ontologies and remote components).

Tested on FBbi and Uberon.

Rename the "Standard Constants" section to "Common variables" (those variables do not follow any kind of "standard"). Remove some variables that are of dubious use, such as `FORMATS_INCL_TSV` (used only once). Move some variables into the section in which they are actually used. Only keep in the "Common variables" section the variables that are actually common throughout the entire Makefile.

Since ODK 1.6, the mechanism to update a ODK-managed repository no longer involves a temporary `target` directory. The repo is updated "in place".

Reformat the beginning of the Makefile template, from the header to the section about ROBOT plugins (included). Reformatting includes: * changing the order of the "top-level targets" so that they match the order in which they are declared in the `all_odk` rule, for better readability; * fixing Jinja2 whitespace handling.

Move the "Quality control" section near the beginning of the Makefile and reformat it.

The code related to imports was split over three sections, found at different places in the Makefile: one section for the import modules themselves (the code that produces the actual modules), one section for the generation of the seed, and one section for mirroring the remote ontologies. We regroup all this code in one large "imports" section, containing one subsection for mirrors, one subsection for the seed (which may in fact be skipped entirely if there is no seed to generate), and one subsection for the imports. As part of those moves: * the code that produces the "simple seed" is moved to the section about the release artefacts, since this is only place where that seed (which has nothing to about imports) is needed; * the code that produces the SRCMERGED and EDIT_PREPROCESSED intermediate files is moved to a dedicated section, since on the contrary those files are used for many more things than just preparing the import modules.

Components are pretty similar to import modules, so we move their section just after the imports section.

Move the sections about SSSOM mappings and bridges into a single "mappings and bridges" section. As part of the "re-formatting", also apply to the downloading of SSSOM sets the same logic as the one used for the downloading of mirrors and components -- ensuring that (i) a remote mapping set is automatically refreshed under MIR=true (previously, once downloaded a first time, a mapping set was never automatically refreshed), and (ii) if the refreshed mapping set has not in fact changed since the last download, the refresh does not trigger the rebuilding of any file that depends on the mapping set.

The ORCIDIO module is akin to a component, so we move it to just after the components section.

Move the section about DOSDP patterns closer to the sections about components (same rationale as for the SSSOM or ORCIDIO sections: this is a section about something that contributes to the contents of the ontology, like a component module). As part of the re-formatting: * we update the downloading of external patterns so that it uses the same odk-helper-based logic as the downloading of all other remote resources; * we replace `$(patsubst)` by `$(basename)` to extract the base name of DOSDP pattern files (simpler and makes the intention clearer).

Move the subsets section before the release section (release should come last, after all the rules needed to generate the contents of the ontology) and re-format it. The re-formatting notably involves removing stuff like this: $(ROBOT) [...] --output $@.tmp && mv $@.tmp $@

SPARQL exports are very similar to SPARQL checks, so we can group both in a single "SPARQL checks and reports" section. We also update the code for the SPARQL exports so that it no longer uses the deprecated `--select` option of `robot query`, but the `--queries` option instead (which also dispenses us from having to construct a `SPARQL_EXPORTS_ARGS` variable).

Move the translation section before the release section and re-format it. Re-formatting notably includes removing some repeated tests "if project.babelon_translation_group is not none" to instead enclose most of the section in one single such test. Also, discovered the Jinja2 "loop" variable, much useful when having to format for loops.

Re-format all the rules to produce the release artefacts and move them before the release section. The re-formatting makes intensive use of Jinja's whitespace control to try making the rules as readable as possible.

Move most of the assets definitions to their corresponding sections (i.e. mapping assets in the mappings section, subset assets in the subsets section, etc.) I think it is better to have the list of assets defined close to the place where the rules that build them are also located. The "assets" subsection in the release section can then focus on assembling the complete list of assets. (We only need an exception for imports, because IMPORT_FILES depend both on the imports section _and_ on the ORCIDIO section.) Also move the code that creates the "release diff" to a temporary place in the "editor utilities" section. Despite the "release diff", it does not really has anything to do with the release process (and it is not part of it).

If the only export format is OWL, then we do not need an "export formats" section, since all artefacts are already produced in OWL. Of note, OWL is a _mandatory_ export format: even if the project file does not include it in its `export_formats` setting, it is always forcefully added. Previously, adding OWL back as an export format was done in the Makefile template. Here, we do that at the level of the configuration object in Python code, because this kind of logic does not belong to the template.

Previously (as in: two commits ago), we defined the import assets in the general "assets" section, on the rationale that the definition of those was dependent on both the imports section proper _and_ the ORCIDIO section (if defined). However I now believe this creates needless confusion. Since the ORCIDIO import is not in fact managed by the imports section (it is not really an "import", despite its name; or at the very least it is a special kind of import), it is more consistent _not_ to have it listed in the IMPORT_FILES variable, but to instead explicitly include it in the general ASSETS list. (We still make the `all_imports` target refresh the ORCIDIO import if present though, for convenience.) Two consequences (presumably harmless) of not listing the ORCIDIO import in IMPORT_FILES: * the module will _not_ be exported to OBO even if import_group.export_obo is enabled (this should not be an issue; OBO export for the ORCIDIO module would make no sense anyway, as ORCIDIO only contains individuals and the OBO format cannot represent those); * the module will _not_ be treated as a _release_ asset even if import_group.release_imports is enabled (again, should not be an issue; releasing that module would make little sense -- arguably, releasing _any_ import module makes little sense).

The test for RDF/XML correctness is only about the main products, so it belongs to the section that creates those products.

Make another pass (hopefully the last one) to make the formatting more uniform. Notably, make sure to avoid using a mix of tabs and spaces for indenting. Only use tabs when they are required by Make's syntax (first character of every line within a rule), otherwise use spaces. Also always favour "post-block" space control over "pre-block" whenever possible, unless the use of post-block control happens to be more cumbersome than pre-block (this is sometimes the case).

gouttegd · 2026-08-22T22:00:17Z

@matentzn This one is likely going to be hard to review, sorry about that.

Reviewing individual commits (as for Uberon’s big Makefile refactor 3 years ago) might help, but even with that, the fact is that formatting changes are a pain in the neck (which is in fact why I wanted to do them all in a single PR, so that it’s done once and for all).

matentzn

Wonderful work @gouttegd, thanks a lot. Full support on everything. I noted a few minor issues I saw, and I am happy to give it another pass afterwards (it did take a bit of an afternoon I must say :P)

I wonder if we should add a .DELETE_ON_ERROR https://innolitics.com/articles/make-delete-on-error/ just to catch cases where for some reason a write with interrupted due to an OOM or some such?
We should probably create a detailed release notes with all the commands and vars removed, and a quick script to check "if someone happened to use them in project.Makefile"?

matentzn · 2026-08-23T13:51:49Z

+                          {%- endfor %}
+                          {%- endif %}
+
+EXTERNAL_PATTERN_URLS   = $(strip $(file <$(PATTERNDIR)/dosdp-patterns/external.txt))


Never seen $(file < before..

https://www.gnu.org/software/make/manual/html_node/File-Function.html

So that the output can be seen from the log of a GitHub Action run.

Users might want to re-serialize the edit file after they have edited it, but _before_ they have committed their changes to the repository (so as to avoid having to commit it again if it turned out that the file was indeed in need of normalization). This means they can't rely simply on `git restore` to recover from a botched write operation (if the normalization command is interrupted right at the wrong moment).

gouttegd · 2026-08-23T17:44:39Z

We should probably create a detailed release notes with all the commands and vars removed, and a quick script to check "if someone happened to use them in project.Makefile"?

There isn’t that many changes, in fact.

The following targets have been removed:

$(ONTOLOGYTERMS);
$(PATTERNDIR)/dospd-patterns/%.yml (replaced by $(PATTERNDIR)/dospd-patterns/%.yaml; the .yml extension seems to have been an error – everywhere else, it has always been assumed that all pattern files in $(PATTERNDIR)/dosdp-patterns/ have a .yaml extension);
$(REPORTDIR)/$(SRC)-obo-report.tsv and $(REPORTDIR)/$(SRC)-align-report.tsv (covered by the corresponding generic rules $(REPORTDIR)/%-obo-report.tsv and $(REPORTDIR)/%-align-report.tsv);
$(TMPDIR)/pattern_schema_checks (renamed to $(TMPDIR)/stamp-pattern-schema-checks, for consistency with components);
check_rdfxml_assets (renamed to check_rdfxml_products).

As well as the following variables:

FORMATS_INCL_TSV;
IMPORT_OBO_FILES and IMPORT_OWL_FILES;
MAIN_GZIPPED;
MAPPING_RELEASE_FILES (renamed to RELEASED_MAPPING_FILES);
ODK_VERSION_MAKEFILE;
ONTOLOGYTERMS;
RELEASE_ASSETS_AFTER_RELEASE (renamed to RELEASED_ASSETS);
ROBOT_PROFILE;
SPARQL_EXPORTS_ARGS;
SPARQL_VALIDATION_QUERIES;
SUBSET_ROOTS;
UPDATEREPODIR.

matentzn

Round 2. :)

The rule that produces both the `$(IMPORT_MODULE_SIGNATURE)` and `$(IMPORT_MODULE)` files is a perfect candidate for a "grouped target", as the same command produces both files at the same time.

matentzn

I checked once more carefully and could not see anything else. Nice work!

gouttegd added 4 commits July 19, 2026 13:35

Remove the UPDATEREPODIR variable.

1c6e01b

Since ODK 1.6, the mechanism to update a ODK-managed repository no longer involves a temporary `target` directory. The repo is updated "in place".

Move and reformat the QC section.

e48740d

Move the "Quality control" section near the beginning of the Makefile and reformat it.

gouttegd self-assigned this Aug 9, 2026

gouttegd force-pushed the refactor-makefile-template branch from 152fdab to fff94fa Compare August 14, 2026 21:50

gouttegd and others added 4 commits August 15, 2026 14:11

Move components section.

74b38db

Components are pretty similar to import modules, so we move their section just after the imports section.

Move the ORCIDIO section.

4551b5a

The ORCIDIO module is akin to a component, so we move it to just after the components section.

gouttegd force-pushed the refactor-makefile-template branch from 1988187 to 95aadce Compare August 15, 2026 20:40

gouttegd added 3 commits August 15, 2026 22:51

gouttegd force-pushed the refactor-makefile-template branch 2 times, most recently from a6e29c0 to 4ae16e0 Compare August 16, 2026 14:29

Move and re-format the release artefacts section.

d8b29f0

Re-format all the rules to produce the release artefacts and move them before the release section. The re-formatting makes intensive use of Jinja's whitespace control to try making the rules as readable as possible.

gouttegd force-pushed the refactor-makefile-template branch 2 times, most recently from 3aba48a to cccca0e Compare August 17, 2026 20:00

gouttegd force-pushed the refactor-makefile-template branch from cccca0e to 8826846 Compare August 17, 2026 20:23

gouttegd added 5 commits August 17, 2026 21:49

Apply linting rule.

7de56f1

Re-arrange and re-format the last section.

6a955ff

Move the rdfxml test to the main artefacts section.

33bbcba

The test for RDF/XML correctness is only about the main products, so it belongs to the section that creates those products.

gouttegd force-pushed the refactor-makefile-template branch from 95b65c7 to d69be57 Compare August 19, 2026 23:09

gouttegd mentioned this pull request Aug 20, 2026

Must “export” release artefacts use a format-dependent ontology IRI? INCATools/ontology-development-kit#1366

Open

gouttegd force-pushed the refactor-makefile-template branch from d69be57 to fff8c5f Compare August 22, 2026 13:18

gouttegd force-pushed the refactor-makefile-template branch from fff8c5f to 7ddb3ae Compare August 22, 2026 19:30

gouttegd force-pushed the refactor-makefile-template branch from 7ddb3ae to 2f324ab Compare August 22, 2026 21:29

gouttegd marked this pull request as ready for review August 22, 2026 21:55

gouttegd requested a review from matentzn August 22, 2026 21:55

matentzn reviewed Aug 23, 2026

View reviewed changes

gouttegd added 5 commits August 23, 2026 16:20

Typo fixes.

4350eb4

Restore printing to stdout when validating profile.

ca59ee3

So that the output can be seen from the log of a GitHub Action run.

Do not include help for non-existing commands.

a4ea06f

Include RDF/XML check in the main test target.

5d68933

gouttegd requested a review from matentzn August 23, 2026 17:45

matentzn requested changes Aug 23, 2026

View reviewed changes

gouttegd added 2 commits August 23, 2026 23:42

More typo fixes.

254b594

Make the IMPORT_MODULE_SIGNATURE rule a "grouped target".

01cafc1

The rule that produces both the `$(IMPORT_MODULE_SIGNATURE)` and `$(IMPORT_MODULE)` files is a perfect candidate for a "grouped target", as the same command produces both files at the same time.

matentzn approved these changes Aug 24, 2026

View reviewed changes

gouttegd merged commit 8b9d408 into main Aug 24, 2026
6 checks passed

gouttegd deleted the refactor-makefile-template branch August 24, 2026 14:15

Uh oh!

Conversation

gouttegd commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gouttegd commented Aug 22, 2026

Uh oh!

matentzn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matentzn Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

gouttegd Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

gouttegd commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matentzn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matentzn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gouttegd commented Aug 9, 2026 •

edited

Loading

gouttegd commented Aug 23, 2026 •

edited

Loading