kosma / minmea

a lightweight GPS NMEA 0183 parser library in pure C
Do What The F*ck You Want To Public License
756 stars 246 forks source link

Add ability to build minmea with meson #28

Open ssloboda opened 6 years ago

ssloboda commented 6 years ago

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.

mtp401 commented 6 years ago

@kosma can you take a peek when you get a chance? thanks!

kosma commented 6 years ago

I'm gonna need some rationale behind making a minmea directory. This is a somewhat breaking change for the users. Why does Meson require a subdirectory?

kosma commented 6 years ago

@ssloboda @mtp401

ssloboda commented 6 years ago

When setting up a Meson project to be usable as a subproject, you have to specify its include directories. A minmea directory was added so that client code could #include "minmea/minmea.h".

The reason why one would want to #include "minmea/minmea.h" rather than #include "minmea.h" is to avoid potential name conflicts (somewhat similar reasoning to prefixing all the functions with minmea_). While in this case, it is only one header with an uncommon name that needs to be included, not using subdirectories for your includes, to my knowledge, is generally seen as bad practice. Keeping project header files in their own directory is especially important when building and installing libraries. Just imagine how many problems would come up if the headers for every library you installed got dumped, without subdirectories, into /usr/local/include/. Adding the minmea subdirectory helps prevent this sort of include directory pollution and file name conflict. Also, this helps future-proof minmea because if at some point a new header file with a common name needs to be added, e.g., utils.h, doing so will not cause any of the problems described above.

If adding the minmea directory is a no-go, I'd still like to incorporate the build with meson changes (I've got another commit to add for library installation in a different branch) because that will be useful for developers using Meson and is a non-breaking feature addition.

ssloboda commented 6 years ago

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 minmea directory for subproject includes. It's easy to optionally add a prefix when installing as a library to the system.

cmorganBE commented 2 years ago

@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.

mtp401 commented 2 years ago

@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.

eli-schwartz commented 2 years ago

The reason why one would want to #include "minmea/minmea.h" rather than #include "minmea.h" is to avoid potential name conflicts (somewhat similar reasoning to prefixing all the functions with minmea_). While in this case, it is only one header with an uncommon name that needs to be included, not using subdirectories for your includes, to my knowledge, is generally seen as bad practice. Keeping project header files in their own directory is especially important when building and installing libraries. Just imagine how many problems would come up if the headers for every library you installed got dumped, without subdirectories, into /usr/local/include/. Adding the minmea subdirectory helps prevent this sort of include directory pollution and file name conflict. Also, this helps future-proof minmea because if at some point a new header file with a common name needs to be added, e.g., utils.h, doing so will not cause any of the problems described above.

Actually, many libraries install headers directly to /usr/local/include/ and expect users to use #include <foolib.h> -- the difference is that these libraries carefully maintain a public header API in a single file with a distinctive name.

Or sometimes they do it in multiple files with distinctive names. <foolib.h> and <foolib_utils.h>.

Many projects also install everything to a subdir #include <foolib/foolib.h>.

A third choice, yes that's right there's three of them, is that the files get installed to /usr/local/include/foolib/ but that gets added as the includedir on the compiler command line, and users still use #include <foolib.h>.

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. #include <foolib.h> or #include <foolib/foolib.h>, needs to be an on-disk file structure in the repository so that the in-tree version can be passed as a compiler include directory. This is an actual good practice.

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 #include <minmea.h> then no changes need be made to the file structure here.

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.

chmorgan commented 2 years ago

@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.

eli-schwartz commented 2 years ago

@chmorgan,

if someone uses minmea in their own project, what would you recommend they use as a header #include?

cmorganBE commented 2 years ago

@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.

eli-schwartz commented 2 years ago

The question is what should be the canonical way, not what is technologically possible. :)

kosma commented 2 years ago

The include for minmea is #include <minmea.h>. It's a single-file library by design, so subfolders are not needed, not necessary, and not wanted. This is a design decision.

eli-schwartz commented 2 years ago

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 #include <minmea/minmea.h>, and that's massively out of scope for "support building via Meson" because it requires source code and documentation changes to all external users.

kosma commented 2 years ago

@eli-schwartz You are an absolute blessing. ❤️

kosma commented 2 years ago

Okay. I think I am ready to proceed with this... but I need the following:

  1. Clean, minimal build.meson. If we can have no options, just single build.meson, that would be perfect.
  2. Instructions how to use it - preferably in the form of a shell script, as this will need to be integrated into our CI setup.