Skip to content

Refactor the Makefile template - #20

Open
gouttegd wants to merge 20 commits into
mainfrom
refactor-makefile-template
Open

Refactor the Makefile template#20
gouttegd wants to merge 20 commits into
mainfrom
refactor-makefile-template

Conversation

@gouttegd

@gouttegd gouttegd commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

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.

gouttegd added 4 commits July 19, 2026 13:35
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.
@gouttegd gouttegd self-assigned this Aug 9, 2026
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
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
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
gouttegd force-pushed the refactor-makefile-template branch from 1988187 to 95aadce Compare August 15, 2026 20:40
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
gouttegd force-pushed the refactor-makefile-template branch 2 times, most recently from a6e29c0 to 4ae16e0 Compare August 16, 2026 14:29
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
gouttegd force-pushed the refactor-makefile-template branch 2 times, most recently from 3aba48a to cccca0e Compare August 17, 2026 20:00
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
gouttegd force-pushed the refactor-makefile-template branch from cccca0e to 8826846 Compare August 17, 2026 20:23
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
gouttegd force-pushed the refactor-makefile-template branch from fff8c5f to 7ddb3ae Compare August 22, 2026 19:30
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
gouttegd force-pushed the refactor-makefile-template branch from 7ddb3ae to 2f324ab Compare August 22, 2026 21:29
@gouttegd
gouttegd marked this pull request as ready for review August 22, 2026 21:55
@gouttegd
gouttegd requested a review from matentzn August 22, 2026 21:55
@gouttegd

Copy link
Copy Markdown
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).

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

Successfully merging this pull request may close these issues.

1 participant