Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(meson): install CMake Config files to datadir #141

Merged
merged 2 commits into from
Feb 2, 2022

Conversation

Tachi107
Copy link
Contributor

@Tachi107 Tachi107 commented Feb 1, 2022

Since Meson doesn't yet support CMake's ARCH_INDEPENDENT option, a pre-generated Package Version file is installed instead of generating it at configure time.

I've also cleaned up a bit the nearby lines of code.

Fixes #140

Pre-merge checklist

  • I've read CONTRIBUTING.md
  • I've rebased my changes against the current HEAD of origin/master (if necessary)
  • I've added new test cases to verify my change
  • I've regenerated toml.hpp (how-to)
  • I've updated any affected documentation
  • I've rebuilt and run the tests with at least one of:
    • Clang 6 or higher
    • GCC 7 or higher
    • MSVC 19.20 (Visual Studio 2019) or higher
  • I've added my name to the list of contributors in README.md

@Tachi107
Copy link
Contributor Author

Tachi107 commented Feb 2, 2022

Uhm, this hardcodes the version number in yet another place. I can easily tell Meson to insert the version, so I'll do just that.
Converting to draft.

@Tachi107 Tachi107 marked this pull request as draft February 2, 2022 07:05
Since Meson doesn't yet support CMake's ARCH_INDEPENDENT option, a
pre-generated Package Version file is installed instead of generating it
at configure time.

I've also cleaned up a bit the nearby lines of code.

Fixes marzer#140
@Tachi107 Tachi107 force-pushed the meson-cmake-config-path branch from 0343c19 to 228dc33 Compare February 2, 2022 07:45
@Tachi107
Copy link
Contributor Author

Tachi107 commented Feb 2, 2022

@marzer since I'm working on the feature I described in #140 (comment), could you please explain a bit to me how TOML_HEADER_ONLY=0 and TOML_IMPLEMENTATION work? The docs don't go much into the details.

@Tachi107 Tachi107 marked this pull request as ready for review February 2, 2022 07:55
@marzer
Copy link
Owner

marzer commented Feb 2, 2022

The Speeding up compilation section has what you need, I suspect.

@Tachi107
Copy link
Contributor Author

Tachi107 commented Feb 2, 2022

I've read it, but it doesn't explain much.

Let's say I have a toml.cpp file with the following content

#define TOML_HEADER_ONLY 0
#define TOML_IMPLEMENTATION
#include <toml++/toml.h>

And then I compile that single file as it was a regular library. In meson terms:

project('toml_lib', 'cpp')
lib = library('toml_lib', 'toml.cpp')

Can I then link against that library from external projects? Are shared symbols generated? How much of toml++ gets compiled in there, and how much of the implementation is kept into the headers?

@marzer
Copy link
Owner

marzer commented Feb 2, 2022

but it doesn't explain much.

Well, given that it's a header-only library, it explains exactly what it needs to explain to support that use-case.

How much of toml++ gets compiled in there, and how much of the implementation is kept into the headers?

The .inl files contain the implementation that is guarded behind TOML_IMPLEMENTATION. Give toml.h a look to see how this works (it's a short file; just includes other files). The single-header toml.hpp is the same as that file just with the headers preprocessed inline.

What's not obvious from that file is how much code is in the public .h files vs. the private .inl files, but the breakdown is approximately:

  • Public code: ~7000 lines of mostly lightweight classes and templates
  • Private code: ~9000 lines of actual code, ~4000 of which is the parser

Let's say I have a toml.cpp file with the following content

In that particular example (having the library included in a single cpp file) there is no difference between TOML_HEADER_ONLY being 0 or 1 - the amount of code seen by the compiler is the same. Disabling header-only mode only makes sense when toml++ is seen in more than one translation unit.

Are shared symbols generated?

That can be controlled by setting TOML_API. See Library configuration.

Can I then link against that library from external projects?

Yup, if you configure the exports. See above. This is how we use the library at my workplace; it is exported from inside a larger DLL.

Worth noting that is all pretty much the same as any other header-only library with an optional implementation target, like the stb libs, and a number of other popular ones. I'm not doing anything novel or tricky here.

@Tachi107
Copy link
Contributor Author

Tachi107 commented Feb 2, 2022

Are shared symbols generated?

That can be controlled by setting TOML_API. See Library configuration.

Thanks, visibility support is really useful when working with ABI compatibility

Worth noting that is all pretty much the same as any other header-only library with an optional implementation target, like the stb libs, and a number of other popular ones. I'm not doing anything novel or tricky here.

I've been in the coding world for just a few years (my first GitHub issue was opened in 2020), and I've never encountered header-only libraries that offer this kind of feature. So it might not be tricky to you, but is almost mind blowing to me :D

Now everything is super clear, I'll submit a PR when ready. Thanks!

@marzer
Copy link
Owner

marzer commented Feb 2, 2022

So it might not be tricky to you, but is almost mind blowing to me :D

Hah, interesting. I almost wish it wasn't that common, to be honest; the only reason header-only libs like mine have become popular is as a workaround for the absolute clusterfuck that is the C++ ecosystem, lmao

@Tachi107 Tachi107 force-pushed the meson-cmake-config-path branch from b7a7ab6 to f0feab9 Compare February 2, 2022 13:45
@marzer
Copy link
Owner

marzer commented Feb 2, 2022

As a vaguely-related aside: here's a big list of libs with similar designs to this one: https://github.com/nothings/single_file_libs - lots of good stuff in there.

@Tachi107
Copy link
Contributor Author

Tachi107 commented Feb 2, 2022

I almost wish it wasn't that common, to be honest; the only reason header-only libs like mine have become popular is as a workaround for the absolute clusterfuck that is the C++ ecosystem, lmao

Yeah that's true lol, I wonder why. Anyway, could you please merge this PR so that I can branch off these changes to work on the other feature? I've also added a commit that avoids hardcoding include as the name of the include directory in the CMake Config file, so that it would work also when defining the includedir Meson option to a custom value

@marzer
Copy link
Owner

marzer commented Feb 2, 2022

Anyway, could you please merge this PR so that I can branch off these changes to work on the other feature?

Sure thing. Sorry, I assumed you were just gonna bundle the other work into this one.

@marzer marzer merged commit 4bd9bda into marzer:master Feb 2, 2022
@Tachi107 Tachi107 deleted the meson-cmake-config-path branch February 2, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build(meson): CMake's Package Version file is installed in the wrong place
2 participants