Add ability to build minmea with meson #28
Conversation
This function is useful when writing libraries that use minmea and support NMEA as well as other sentence types.
ssloboda
force-pushed
the
ssloboda/meson
branch
from
646d57d
to
77e2579
Compare
|
@kosma can you take a peek when you get a chance? thanks! |
|
I'm gonna need some rationale behind making a |
|
When setting up a Meson project to be usable as a subproject, you have to specify its include directories. A The reason why one would want to If adding the |
|
Also, it's been a while since I've read through the Meson docs, but I don't think it's possible to optionally fake a |
|
@ssloboda any reason you can't place minmea in a subdirectory on your own end to avoid having to move the files within this repository? I can't imagine everyone integrating libraries with meson is looking to move their location in upstream repos. |
This can be closed if the maintainers of minmea are not interested in first class meson support. We'll contribute a wrap to Meson's wrapdb instead. |
Actually, many libraries install headers directly to /usr/local/include/ and expect users to use Or sometimes they do it in multiple files with distinctive names. Many projects also install everything to a subdir A third choice, yes that's right there's three of them, is that the files get installed to These are valid decisions, but it should in fact be a decision of the project. In general, the subdir approach tends to be taken by projects that expect to have a bunch of header files, and the no-subdir approach tends to be taken by projects that expect to have one or two header files. Is there a serious concern that this project will grow to include many publicly installed header files? Keep in mind, if there comes to be a utils.h which is used internally for building the library, but isn't needed for third-party projects to build against the library API, then for the purposes of this discussion it's quite irrelevant and doesn't need to be factored in here. ... Either way, with my Meson core dev hat on... Meson does not care which approach is taken, it supports and is used for both. What Meson does care about, is that in order for use via the wrap subprojects system, whatever include name the project expects downstream consumers to use in their source code, i.e. Failure to do this means that Meson will still support building and installing the project, but will not support adding the project as a git submodule to another project and building it as a vendored copy as described in https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section ... If the project expects people to do This conversation should NOT focus on implementing moving files around the source tree, but rather focus on convincing the project maintainers of the relative merits of different styles of includes, and then work backwards from there to implement the desired approach. |
| *.exe | ||
| build |
There was a problem hiding this comment.
This is not necessary:
-
it may be named anything and a popular convention is
builddir/, orbuilddir-release/andbuilddir-debug/, so this gitignore entry likely won't take effect -
Meson knows at the time of creating
builddir/that it is a build directory, and that no files in there should be added to git, so it createsbuilddir/.gitignorewhich ignores*, so that you don't have to. This just magically works.Admittedly, this was implemented in December 2020 via mesonbuild/meson@7437d98 and released in Meson 0.57.0, so back when you originally submitted this PR that was not the case, but still... it is long past December 2020, so you can remove this now. :)
| ) | ||
|
|
||
| add_project_arguments( | ||
| '-Wall', '-Wextra', '-Werror', |
There was a problem hiding this comment.
Meson will warn you not to do this, and suggest instead that you set default_options to include ['warning_level=2', 'werror=true'].
This will guarantee that it works cross-platform and on any compiler, and it also means that when GCC adds new warnings to -Wextra, which it often does, users can still build the project by passing -Dwerror=false as a manual override.
| ) | ||
|
|
||
| if host_machine.system() == 'windows' | ||
| add_project_arguments('DMINMEA_INCLUDE_COMPAT') |
There was a problem hiding this comment.
I think this is missing a leading -
| c_args = [] | ||
|
|
||
| if get_option('buildtype') == 'debug' | ||
| c_args += '-ggdb' |
There was a problem hiding this comment.
The debug buildtype already sets -g, why do you need to enforce -ggdb (which the Makefile doesn't do anyway)?
| minmea_lib = library( | ||
| 'minmea', | ||
| minmea_sources, | ||
| include_directories: minmea_includes, |
There was a problem hiding this comment.
The current directory is an implicit include, you still need to use it for declare_dependency, but you don't need it for defining the library. Minus one line.
| 'minmea', | ||
| minmea_sources, | ||
| include_directories: minmea_includes, | ||
| dependencies: minmea_dependencies, |
There was a problem hiding this comment.
This is an empty array, is it just future proofing? The project should confirm whether it's worth the additional lines for something that may have a definite development policy to never utilize.
In fact this seems likely to be the case, the project goals state that it is lightweight/minimal, and mention the bullet point:
- One source file and one header - can't get any simpler.
So it sounds like they would not be willing to add dependencies in the future, which means you don't need to plan ahead for dependencies.
| link_with: minmea_lib, | ||
| ) | ||
|
|
||
| if get_option('example') |
There was a problem hiding this comment.
I would not bother with this option, just always define the example to be built. It's a single compile rule with no dependencies, it doesn't add any complexity to building, so there's no cost to always building it.
If you just don't want to run an extraneous build rule when using this as a wrap, set this executable to build_by_default: false.
| endif | ||
|
|
||
| if get_option('tests') | ||
| check = dependency('check') |
There was a problem hiding this comment.
I would use a feature option for this these days, you can do check = dependency('check', required: get_option('tests')) and it will be a tristate lookup. If enabled, the dependency is required. If disabled, the dependency is skipped and forced to not-found. If auto, it will try to find the dependency, but allow not finding it.
Then, you can check whether the dependency was found and only when it is found, define the tests.
|
@eli-schwartz @ssloboda is it ok if only the header file is moved into a minmea/ subdirectory? As the library (.a file) would remain in the parent folder during install but the header would go into the subdir? Would that let you use the meson build in your repo without pulling it in here? If we pulled it in here we'd have to maintain it etc... and there are a lot of build systems. |
|
if someone uses minmea in their own project, what would you recommend they use as a header |
|
@eli-schwartz it would depend where they put the header files. If you put minmea into a 'minmea' folder you could add the containing folder to your search path and #include "minmea/minmea.h", or add minmea to your include path and #include minmea.h". It feels like I'm misunderstanding the question though. |
|
The question is what should be the canonical way, not what is technologically possible. :) |
|
The include for minmea is |
|
Then my comment above still applies, in which I predicted that that would be the case. Nothing, not even Meson, needs the folder structure of this repository changed. Changing the folder structure would be a statement that you expect people to use that folder structure via |
|
@eli-schwartz You are an absolute blessing. ❤️ |
|
Okay. I think I am ready to proceed with this... but I need the following:
|
Hi, is this something that you would consider merging? I'm currently writing a library that pulls in minmea as a dependency and I needed to build it with meson to do so. I thought the work done here might be useful for others who are also using the meson build system.