Refactor the Makefile template - #20
Open
gouttegd wants to merge 20 commits into
Open
Conversation
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.
gouttegd
force-pushed
the
refactor-makefile-template
branch
from
August 14, 2026 21:50
152fdab to
fff94fa
Compare
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).
gouttegd
force-pushed
the
refactor-makefile-template
branch
from
August 15, 2026 20:40
1988187 to
95aadce
Compare
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.
gouttegd
force-pushed
the
refactor-makefile-template
branch
2 times, most recently
from
August 16, 2026 14:29
a6e29c0 to
4ae16e0
Compare
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
August 17, 2026 20:00
3aba48a to
cccca0e
Compare
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).
gouttegd
force-pushed
the
refactor-makefile-template
branch
from
August 17, 2026 20:23
cccca0e to
8826846
Compare
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.
gouttegd
force-pushed
the
refactor-makefile-template
branch
from
August 19, 2026 23:09
95b65c7 to
d69be57
Compare
gouttegd
force-pushed
the
refactor-makefile-template
branch
from
August 22, 2026 13:18
d69be57 to
fff8c5f
Compare
gouttegd
force-pushed
the
refactor-makefile-template
branch
from
August 22, 2026 19:30
fff8c5f to
7ddb3ae
Compare
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
force-pushed
the
refactor-makefile-template
branch
from
August 22, 2026 21:29
7ddb3ae to
2f324ab
Compare
gouttegd
marked this pull request as ready for review
August 22, 2026 21:55
Collaborator
Author
|
@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). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
curlorwget(such as remote SSSOM mapping sets or remote DOSDP templates) are now downloaded using the ODK’s ownodk-helper downloadcommand (already used for mirrored ontologies and remote components).Tested on FBbi and Uberon.