Add a `apply-patches` helper workflow. by gouttegd · Pull Request #27 · INCATools/odkcore

gouttegd · 2026-09-07T21:05:12Z

This PR adds a apply-patches target to the standard Makefile. When triggered, this workflow will find all .obo, .tsv, and .kgcl files into the src/ontology/tmp/patches directory (if such a directory exists) and:

for OBO files, they will be simply merged into the -edit file;
for TSV files, they are assumed to be ROBOT templates, they will be instantiated and the result will be merged into the -edit file;
for KGCL files, they are assumed to contain KGCL instructions which will be applied to the -edit file.

Patches that have been successfully applied will be deleted from the patches directory afterwards. If a patch could not be applied and ROBOT gave an error message, that message will be written in a file with the same name as the patch file plus a .ERRORS extension.

closes INCATools/ontology-development-kit#1283

Add a `apply-patches` target to the standard Makefile. When triggered, this workflow will find all `.obo`, `.tsv`, and `.kgcl` files into the `src/ontology/tmp/patches` directory (if such a directory exists) and: * for OBO files, they will be simply merged into the `-edit` file; * for TSV files, they are assumed to be ROBOT templates, they will be instantiated and the result will be merged into the `-edit` file; * for KGCL files, they are assumed to contain KGCL instructions which will be applied to the `-edit` file. Patches that have been successfully applied will be deleted from the patches directory afterwards. If a patch could not be applied and ROBOT gave an error message, that message will be written in a file with the same name as the patch file plus a `.ERRORS` extension. closes INCATools/ontology-development-kit#1283

matentzn · 2026-09-08T12:45:12Z

+
+        p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        (out, err) = p.communicate()
+        if p.returncode != 0:


Does kgcl:apply really ever returns st. non-zero? Best check. I tried to submit a broken patch and and got an exit = 0.

It throws an exception, but I believe ROBOT’s main method catches all exception without returning an error code.

I will need to add a --fail-on-error or something like that.

Err, no, ROBOT does error out (with a non-zero return code) when the exception is thrown for invalid KGCL input…

What do you mean by “broken patch”? There are two different error scenarios here:

(a) Invalid KGCL syntax. This should already yield a non-zero return code.

(b) Valid KGCL syntax, but the change described cannot be applied to the ontology (e.g. you’re trying to change a term that does not exist). kgcl:apply currently does not error out in this case.

matentzn · 2026-09-08T12:51:19Z

+	                                 --context $(CONTEXT_FILE) \
+	                                 {% endif -%}
+	                                 --format {% if project.edit_format == "owl" %}ofn{% else %}{{ project.edit_format }}{% endif %} \
+	                                 $(SRC) $(PATCHESDIR) ; \


Could a mid-write failure leave SRC in a bad state? If so, maybe best to write to a temp file and move.

No more risk in this particular command than in any other ROBOT command.

matentzn · 2026-09-08T12:53:01Z

+            logging.info(f"Applying KGCL patch {path}")
+            cmd.extend(["kgcl:apply", "--input", ontology, "--kgcl-file", path])
+
+        cmd.extend(["convert", "--format", format, "--output", ontology])


Maybe same concern as https://github.com/INCATools/odkcore/pull/20/changes#diff-d42955d839ab621f249693af2c4a252513337e76dd72ef098b6cdd3149e50e8cR2048? (a case for using the write to tmpfile then move pattern)?

I am not concerned, just wanted to note it and leave it to your judgement.

The situation is different. For the normalize_* targets, the expectation was that people may modify the -edit file and then call the normalization target without committing their changes first (because they don’t want to commit a non-normalized file). Therefore, in the off-chance that something went wrong and the -edit file is written back in a corrupted form, users may loose their last modifications.

Here, I would not expect to call the apply_patches target on a “dirty” (non-committed) -edit file. So in the off-chance anything goes wrong, you can just revert the file to its last good state.

matentzn · 2026-09-08T12:56:37Z

 {% if project.use_translations -%}
 TRANSLATIONSDIR           = ../translations
 {% endif -%}
+PATCHESDIR                = $(TMPDIR)/patches


Note that placing this in tmp could cause some heartache of you are, like me, prone to running make clean - but I again have only a very weak stance here. If its our position that patches are ephemeral and applied at the same time as being generated then it is ok I guess.

I’d have no objection to using another location. $(TMPDIR) seems appropriate to me because patches are not intended to be kept around.

If you’re working on a set of patches and you’d like to keep them around until they are ready – while also making sure you won’t accidentally delete them when running an occasional make clean –, then you can always have a separate directory elsewhere (src/ontology/patches-in-progress), and move them to $(TMPDIR)/patches when they are ready.

matentzn · 2026-09-08T12:59:02Z

+
+        total += 1
+
+    if applied and total:


Suggested change

if applied and total:

if total:

Just so you get some sort of summary when the process fails? Weak opinion here.

If you don’t get the Successfully applied..., then you know none of the patches were successfully applied.

matentzn · 2026-09-08T12:59:43Z

+    Supported formats:
+    - .obo: OBO Flat File, merged as it is;
+    - .tsv: ROBOT template, instantiated and merged;
+    - .kgcl: KGCL patch file, applied.


Do we need kgcl.jar in odklite to stay true to our principle that we can run everything in the makefile with odklite?

🤦 Yes, absolutely!

matentzn · 2026-09-08T13:00:31Z

+    directory and applies them to the ontology.
+
+    Supported formats:
+    - .obo: OBO Flat File, merged as it is;


such a pity this wont work on ofn, but - we start somewhere!

Well, it could very well work on OFN! I didn’t do it because I would assume that, if you create a patch, you would rather do it in OBO (AFAIK that’s typically what Chris’ AI bots are doing) than in OFN, but adding OFN would be completely trivial.

Pass two more options to `robot kgcl:apply`: * `--no-partial-apply`, so that a patch is either fully applied or not at all; * `--fail-on-reject`, so that ROBOT errors out when a valid patch cannot be applied (so that we can detect the failure). Of note, the second option requires version 0.6.2 of the plugin, which has not been released yet.

Sort the list returned by `iterdir()`, so that patches are applied in a deterministic order dependent on the file names.

We don't bother to decode the output returned by the ROBOT process, so the file handle on the .ERRORS file should be open in binary mode so that the `write()` method expects bytes, rather than text.

gouttegd self-assigned this Sep 7, 2026

gouttegd requested a review from matentzn September 7, 2026 21:19

gouttegd force-pushed the update-from-files branch from 797dfb4 to bf9c842 Compare September 7, 2026 22:16

matentzn reviewed Sep 8, 2026

View reviewed changes

Comment thread src/incatools/odk/templates/src/ontology/Makefile.jinja2

gouttegd added 4 commits September 8, 2026 20:31

Apply patches in a deterministic order.

837c103

Sort the list returned by `iterdir()`, so that patches are applied in a deterministic order dependent on the file names.

Open the .ERRORS file in binary mode.

ecb1c18

We don't bother to decode the output returned by the ROBOT process, so the file handle on the .ERRORS file should be open in binary mode so that the `write()` method expects bytes, rather than text.

Mention the apply_patches target in the Makefile's help.

41c9303

Uh oh!

Conversation

gouttegd commented Sep 7, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants