adobe-type-tools / afdko

Adobe Font Development Kit for OpenType
https://adobe-type-tools.github.io/afdko/
Other
1.06k stars 167 forks source link

support newer antlr4? #1748

Closed juhp closed 2 months ago

juhp commented 5 months ago

Currently in Fedora we are unable to build afdko 4 because our antlr4 package has version 4.10.1, which seems too new apparently.

https://bugzilla.redhat.com/show_bug.cgi?id=2219176 is the related downstream issue.

Are there any plans to support newer antlr4 in afdko ?

skef commented 5 months ago

The current python packaging doesn't rebuild the parser source files (.g4) and downloads a specific Cpp runtime to compile and statically link with. There should be no interaction with any installed version of antlr4. If there is it's probably because someone has modified our build.

We will periodically upgrade the version of antlr4 we build against, generally when we updated the feature file grammar, but do not intend to sync our use of antlr4 with any given system's installed version. There is too little stability across releases of the runtime.

skef commented 5 months ago

Another way of saying some of this is that the antlr4 Cpp "runtime" is not really a "library" in the traditional sense of something with a backward-compatible API. The runtime for 3.6 (for example) won't necessarily be compatible with 3.5 or 3.7. And if you rebuild the parser input files, the interface for (e.g.) the visitor API may change. So if Fedora wanted us on 4.10.1 because that's what they ship and Arch wanted us on 4.13.1 because that's what they ship we would just be screwed, having to supply as many source repositories as there are active versions or have a sea of ifdefs and constantly having to upgrade when a new version of antlr4 comes out.

And keep in mind that all of this is packaged as a python module, and needs to be built via pip or other python packaging systems.

The current arrangement is quite reasonable: just statically link against the relevant version of the runtime, which only changes when there is a reason to change the parser. The main, and really only, resistance we get to this convention is from various Linux distro maintainers, and so far that resistance has only been ideological. Basically "we don't think libraries should be statically linked because of the potential cache performance reduction and file size". But no one is picking what they install based on installed size (unless the size is really huge), and you only get better cache performance if some other program uses the same version of the library.

So the complaint boils down to "we think your build system should rebuild the parser source files for our particular installed version of antlr4 and your implementation should be ifdefed to support that version." But that doesn't make sense because the c++ antlr implementation + runtime can't reasonably be expressed as a python requirement, therefore basically no one else would build that way. So then it comes down to "we think your build system should provide that option", but we're not going to ifdef in support for every version of the runtime that happens to correspond to what any given distro is currently shipping just because of an ideological preference against static linking.

juhp commented 5 months ago

Thank you for the detailed explanation.

I think we will try to bundle the appropriate version of antlr4 with afdko then in Fedora.

cc @Manish7093

skef commented 5 months ago

I assume that Fedora is still producing "binary"/compiled packages, so it's worth noting that if you just build our package in the right environment and then ship the results the right thing happens: you get a copy of makeotfexe that's statically linked with the relevant antlr4 runtime because the CMake-based build system downloads that and makes a static library to link against. So it's not clear that anything special needs to happen for Fedora packaging.

juhp commented 5 months ago

We can't download anything during the Fedora build: there is no network available in the buildsystem. But okay sounds like you already support this bundling.

skef commented 5 months ago

There are some commented-out directives in the top-level CMakeLists.txt file (with explanatory comments) indicating how you can supply a zip file of the source to override the default network load. I haven't tested that in a while but it worked the last time I tried it. If that is a good solution for this build, but you run into problems using it, I'm happy to investigate and get it working again.

skef commented 2 months ago

This Fedora issue is now resolved. We'll continue to upgrade to new versions of the antlr4 runtime as it makes sense for our development processes.