Add meson build configuration #26
Conversation
|
Thanks for taking the time to contribute. Even though I have no issues using autotools (years of pain mean I'm pretty comfortable with it) and will keep that as the canonical default for now, having more than one build system available probably make sense for this project. It might even make sense for SILE too, something mentioned as recently as comments in this issue from last week. |
There was a problem hiding this comment.
Before we merge this, I would like to see some CI tests that prove the build system(s) are doing at least roughly what is expected of them.
For this PR I think that will mean a new GH Actions workflow that does some sort of test build. What packaging test mechanisms meson exposes I don't know, but something equivalent to a make distcheck from autotools would do the trick.
|
EDIT: This was written when I misread the repository this was filed against as being sile-typesetter/sile. It does not apply to this PR against the libtexpdf library. I'm sorry, I just don't know that we want to go down this route. I've seen it happen in other projects, where we start getting a multiplicity of build system scripts, and none work well, but all are better at one thing or another. I'm not against switching to And I'm not really that good at GNU Autotools either, I find its system of ins and outs perplexing and its manual generally unhelpful. All of its shouty macros like But that's not the point I'm trying to make. I'm wondering if it's not better for us to all be building SILE the same way, so our problems are all shared. If we all just start building however we want, then I don't understand how we're supposed to keep coherence in user experience between us. And new users find it perplexing to see a CMakeLists, a meson file, and a configure script all side by side. I think we need more meta discussion on which solution we are going with. |
franko
force-pushed
the
meson-build
branch
2 times, most recently
from
694c776
to
a8e8c54
Compare
Of course, that makes sense. I have just added a new job in the github workflow to test the meson build. I am using the "ninja dist" command that is roughly equivalent to "make distcheck". Everything works fine but I had to fix a problem with a header check that I forgot in the meson build. Otherwise the Meson build comes out pretty nicely and it works smoothly. It is also a low-maintenance solution as the build script is very simple and straightforward. |
Meson is a modern system that does pretty well all around and it has a sane, simple syntax. Meson itself is pretty reliable and already used by many high profile open source projects. Its main weak point, as far as I know, is its support for Visual Studio solutions but for a free open source project this not a significant issue. Another issue is that it requires python and ninja to work so it's not "minimalist" but both are readily available on any modern system. Another criticism I met in the past is that Meson was rapidly evolving but it recently hit the 1.0 release so things should be pretty stable now. In the past it was common to enforce the usage of a "pretty recent" version of Meson.
I propose to introduce Meson alongside with the autotools build system as they are currently used. Switching to Meson exclusively may be done later, only if all the contributors are comfortable with that.
Having a few build systems side-by-side is not a problem in my humble opinion and based on my experience with other open source project. |
Well, ninja works pretty well on Windows too. :D But there is a Visual Studio backend and lots of people make good use of it.
And for those that still have a concern, there is https://sr.ht/~lattis/muon and https://github.com/michaelforney/samurai for, respectively, Meson and Ninja, both written in c99.
Well, it depends on the project TBH. This project is a library, not an application. It's a lot more common for libraries to support multiple build systems, because that makes it easier to embed inside another project using the same build system:
In fact, sile makes use of libtexpdf via the autotools approach (while also supporting an option to use a system one, which is a bit messy since autotools needs to resolve the full embedded copy in order to get a sensibly working ./configure script, whereas meson and cmake both download it on-demand and meson even allows you to trivially choose whether to use the embedded or system copy). |
You're right. For some reason I thought this was filed against sile-typesetter/sile. My thoughts on the matter are withdrawn, I'm fine with this. |
There was a problem hiding this comment.
A note on library exports. Makefile.am uses -export-symbols-regex '^texpdf_' to ensure that libtool produces a GNU ld version script which looks like this:
{
global: texpdf_*;
local: *;
};
although actually it contains all 283 symbols explicitly spelled out instead of using wildcard support in the global section, because theoretically a regex could do something more complex than globs. 😆
| 'tfm.c', | ||
| ] | ||
|
|
||
| libtexpdf = library('texpdf', libtexpdf_sources, |
There was a problem hiding this comment.
The Makefile build implicitly sets the version to 0.
There was a problem hiding this comment.
Note this is kind of weird but it is likely a good idea to do for compatibility reasons.
libtool by default sets the version to 0 if you don't explicitly specify one, and to suppress that you need to opt into libtexpdf_la_LDFLAGS += -avoid-version. Having a soname version "0" doesn't really say anything meaningful, but at this point it's baked into the ABI so 🤷.
There was a problem hiding this comment.
Note this is kind of weird but it is likely a good idea to do for compatibility reasons.
libtool by default sets the version to 0 if you don't explicitly specify one, and to suppress that you need to opt into
libtexpdf_la_LDFLAGS += -avoid-version. Having a soname version "0" doesn't really say anything meaningful, but at this point it's baked into the ABI so 🤷.
Thanks for bringing this up but I am confused. Either we declare the project version to zero which seems unorthodox and confusing or otherwise we use some obscure linker flag to set the version to zero. In the latter case I feel this can be confusing and difficult to support across all platforms. I don't even know how this can be done with Meson.
There was a problem hiding this comment.
In meson, you just set the version: '0' kwarg to the library.
This isn't related to the project version.
There was a problem hiding this comment.
In meson, you just set the
version: '0'kwarg to the library.
Ok, thank you, I didn't know about that option. That makes sense but before making this change I would prefer to have the advice of the other contributors. If it is up to me I would just omit this change.
meson.build
Outdated
| check_headers = ['dlfcn.h', 'inttypes.h', 'stdint.h', 'stdlib.h', 'string.h', 'sys/stat.h', 'sys/types.h', 'sys/wait.h', 'unistd.h'] | ||
| foreach header_name : check_headers | ||
| if compiler.has_header(header_name) | ||
| conf.set('HAVE_' + header_name.underscorify().to_upper(), 1) |
There was a problem hiding this comment.
This is a port from the existing code, but it's not really necessary -- autotools sometimes checks for things without you asking ;) and the real question is, what does the code itself check for?
- sys/types.h
- sys/wait.h
- sys/stat.h
- stdlib.h
- stdbool.h
EDIT:
- inttypes.h
- stdint.h
- string.h
unistd.h and dlfcn.h aren't needed. stdbool.h is.
There was a problem hiding this comment.
Only sometimes? ;-) I've never been able to figure out how to stop it from doing hundreds of useless checks. You would think it'd detect a modern Linux kernel and then just give up on all that but nope.
There was a problem hiding this comment.
This is a port from the existing code, but it's not really necessary -- autotools sometimes checks for things without you asking ;) and the real question is, what does the code itself check for?
I think too all these checks are pretty useless and I feel these are a legacy from the the autotools. The autotools go to a great length to support obscure Unix systems most of which are no longer in use. To these purpose autotools makes a lot of checks and tends to introduce a lot of accidental engineering complexity into the project itself.
My idea was to match at first the autotools using a Meson config and later, if we want to, remove some of these unneeded cruft. I just omitted from Meson all the checks that are done by the configure script but are not actually used in the code.
There was a problem hiding this comment.
Sorry, I should clarify. When I say these are the only used ones, I mean that I exhaustively grepped the source files to figure out which HAVE_ defines actually get used in the source code.
Cleaning up those ifdef/else blocks to make the headers mandatory can be done later for all build systems, if desired, but I don't really care about that.
The focus of my review was simply that it's true for meson that you don't need to run a configure check in order to detect whether to set a macro that isn't used.
There was a problem hiding this comment.
I agree with you but, for example, HAVE_SYS_TYPES_H is actually used in the code so I added it in the meson configuration. The same goes for the others.
There was a problem hiding this comment.
There are two that you're checking for which I cannot find in the code -- and there's one you're not checking for that is used.
There was a problem hiding this comment.
@eli-schwartz, ok sorry, I did not clearly understood at the beginning. I have normally fixed this problem, please have a look.
|
Thanks for the PR review and Meson feedback @eli-schwartz, that's supper helpful. |
I am wondering if we really need to filter the exported symbols and for what reason. I would prefer to omit this if that's not really needed. Supporting this option across all platforms may be difficult. |
This reminds me it's still on my to-do list to implement cross-platform symbol lists in meson... Filtering the exported symbols avoids ABI leakage which could result in programs using something that breaks in a micro release, but that's more important in libraries that actually track soname. However, it's also required on Windows because MSVC shared DLLs operate in the reverse of Unix -- Unix exports all symbols unless you hide some, MSVC exports none unless you make some public. So shared DLLs are broken if you don't use a def file. (Alternatively you can use declspec, and then have fun with setting a different declspec depending on whether you're building shared or static, and a third one if you are linking to it from another application.) Of course it's also possible to just say "on Windows, only use static_library". You can do that with meson's |
The text does not appear in the document. The page size should be set to some more sensible values. We should probably check the output file, at least that it is not empty and check that the header correspond to a valid PDF file.
Normally the test program should not use the private config variables of the library but we are temporarily adding them to make the tests succed. The problem was reported in the github issue: sile-typesetter#27
|
I have added a stub of a simple unit test in the meson config, you may have a look if you want. I wanted to create an "Hello world" PDF for the test but it just creates an empty PDF, I need some help to fix this and make the test more pertinent. Please note that I have noticed a problem with private build variables exposed in public header and the test was initially failing. I forced the test to succeed by adding the private build defines into the test file but I don't think this is the right solution. |
I agree, that's a well known problem. It seems that support for visibility list is not yet available in Meson because it's difficult to make it work in all platforms and across different compilers. I found this thread: Otherwise it is possible to use the GCC visibility attribute but it requires some platform-dependent macro-definition: https://gcc.gnu.org/wiki/Visibility If we insist on filtering the exported symbols we don't have a Meson solution AFAIK so that could be a blocking point. On the other side I feel that this is a very subtle aspect, hardly a problem in practice. If we really want to we may adapt the GCC visibility attributes but this seems overkill for such a small, simple library. |
|
Like I said, the simple solution is to just skip it and force windows to only build static libraries. |
Hi,
I've created a pull request that adds a Meson build configuration to the project. Meson is a modern build system that is used by high-profile open source projects such as GNOME, GTK, and Mesa3D. It is significantly simpler to use and maintain than autotools, and it has the added benefit of being cross-platform, working on macOS and Windows using MinGW. This configuration also includes a pkg-config file. I believe it would be beneficial to have an alternative build system available.
I understand that adding a new build system may require additional maintenance, and I'm happy to help with any questions or issues that may arise. Let me know if there's anything else I can do to assist with the review and merging of this pull request.
Thank you for considering this contribution.