mkdir -p sharedir before touch

[kba]

Otherwise I get a build failure for make ocrd-tesserocr-binarize with a newly created venv.

[bertsky]

Sorry, didn't see that one. But better do it as an OO dependency. (And maybe the others need that as well?)

BTW, GNU make 4.2 now has syntax for multiple (parallel) targets in one rule (:& instead of :), which would make solutions like this obsolete. But of course, we cannot rely on such features (yet).

[stweil]

I'd prefer a more direct solution: mkdir -p sharedir before touch (as the subject already says).

@mkdir -p $(SHARE) && touch $@

[stweil]

And while changing that issue, I also suggest to change

SHARE := $(VIRTUAL_ENV)/share/ocrd_all

Then there are no stray empty files on the share level which typically only contains directories, and it is also clear were those files belong to.

[bertsky]

I'd prefer a more direct solution: mkdir -p sharedir before touch (as the subject already says).

@mkdir -p $(SHARE) && touch $@

But this is actually the indirect solution! And it's exactly equivalent to the first version of this PR. I specifically requested above changes (doing mkdir via a order-only prerequisite) because:

1. SHARE directory is needed by other rules, too
2. shorter than the extra mkdir in every such rule
3. slightly faster

[bertsky]

And while changing that issue, I also suggest to change

SHARE := $(VIRTUAL_ENV)/share/ocrd_all

Then there are no stray empty files on the share level which typically only contains directories, and it is also clear were those files belong to.

I don't quite see the point because it's our local venv only (or our Docker image), and we=ocrd_all – except if you want to set VIRTUAL_ENV=/usr/local in the host system. But I have no objection either.

[bertsky]

Thanks!

[stweil]

It's direct because it is easy for me to see where it is used. Which other rules need SHARE?

[stweil]

slightly faster

Then put it in $(VIRTUAL_ENV). That already exists, there is no subdirectory needed, no extra code is needed, and it's our local venv, so we can do anything. :-)

[bertsky]

Apart from the pattern rule:

• the numpy rule
• the clstm rule

@kba, you need to add | $(SHARE) there as well – but as I said, I cannot make suggestions outside of the hunk context in GH.

[bertsky]

Then put it in $(VIRTUAL_ENV). No subdirectory needed, no extra code needed, and it's our local venv, so we can do anything. :-)

Sure, why not!

[stweil]

numpy, clstm

Sure, that's the same kind of code, so it's not really new. Automatically fine when putting the touched files in $(VIRTUAL_ENV).

[stweil]

That makes the patch too trivial. What about adding _timestamp to the touched filenames?

[bertsky]

That makes the patch too trivial.

Less is more. Took 3 devs to arrive at! And it's not even done yet.

What about adding _timestamp to the touched filenames?

What for? OO deps don't ever update their dependents due to being older (by design).

The only reason for these pseudo-targets is to avoid installing the same pkg 11 times if the module has 11 CLIs, just because they are all mentioned in the same rule. There used to be no way to tell make that these targets are all built at once by the same recipe, except by formulating it as a pseudo-pattern rule (which is highly unreadable and next to incomprehensible). But no one wants to wait for or require GNU make 4.2.

[bertsky]

There used to be no way to tell make that these targets are all built at once by the same recipe, except by formulating it as a pseudo-pattern rule (which is highly unreadable and next to incomprehensible). But no one wants to wait for or require GNU make 4.2.

Or maybe we should in fact use pseudo-pattern rules to get the single-rule-multiple-output behaviour. That would not only eliminate the need for the above discussed pseudo-targets for numpy names, but also avoid running make install as many times as there are executables per module. I will make a PR for that so we can better discuss this option.

[bertsky]

Apart from the pattern rule:

* the [numpy rule](https://github.com/OCR-D/ocrd_all/blob/097948b6807385d3c7afe03765e9b432915f0a63/Makefile#L84)

* the [clstm rule](https://github.com/OCR-D/ocrd_all/blob/097948b6807385d3c7afe03765e9b432915f0a63/Makefile#L115)

@kba, you need to add | $(SHARE) there as well – but as I said, I cannot make suggestions outside of the hunk context in GH.

Done (along with rebase).

[bertsky]

There used to be no way to tell make that these targets are all built at once by the same recipe, except by formulating it as a pseudo-pattern rule (which is highly unreadable and next to incomprehensible). But no one wants to wait for or require GNU make 4.2.

Or maybe we should in fact use pseudo-pattern rules to get the single-rule-multiple-output behaviour. That would not only eliminate the need for the above discussed pseudo-targets for numpy names, but also avoid running make install as many times as there are executables per module. I will make a PR for that so we can better discuss this option.

Sorry, this was completely unrelated, as I can see now. The above is now in #25.

[bertsky]

@stweil are you ok with the current state of this PR, or do you insist we should change the make tokens of the pip targets for numpy, clstm and tesserocr from files in $(SHARE) to files in $(VIRTUAL_ENV)?

[kba]

That makes the patch too trivial.

I think trivial is good. I like trivial. Can we merge this? It fixes the initial problem without complicating the Makefile further IMHO.