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

Add meson build file #106

Closed
wants to merge 1 commit into from
Closed

Add meson build file #106

wants to merge 1 commit into from

Conversation

v1993
Copy link

@v1993 v1993 commented Apr 7, 2023

Pull request policy (please read)

Contributions are always welcome. However, I release my projects in cumulative updates after editing and testing them locally on my system, so my policy is not to accept any pull requests. If you open a pull request, and I decide to incorporate your suggestion into the project, I will first modify your code to comply with the project's coding conventions (formatting, syntax, naming, comments, programming practices, etc.), and perform some tests to ensure that the change doesn't break anything. I will then merge it into the next release of the project, possibly together with some other changes. The new release will also include a note in CHANGELOG.md with a link to your pull request, and modifications to the documentation in README.md as needed.

Describe the changes

Add file for meson build system. It makes running tests easier, i.e.

meson build
cd build
meson test

will run tests both for normal and light versions. It's also possible to use meson's coverage reporting support to get detailed information about test coverage.

Finally, this makes including this project into meson's warpdb trivial. The only maintenance burden is having to update version when new release is made.

Includes support for installing headers and running tests
@bshoshany
Copy link
Owner

Thanks for this pull request, @v1993! I'm not familiar with Meson, so please give me some time to get acquainted with it, and I will then incorporate your build file into the project.

@bshoshany
Copy link
Owner

Update: Unfortunately I did not yet have time to incorporate this PR in v3.5.0, which was released today. However, compatibility with the meson build system is now on the top of my TODO list for the next release. I just need to find some time to familiarize myself with the system first.

Meanwhile, I am closing this PR, since it is on the fast track to be incorporated in the next release - but please keep the branch open. Thanks again!

@bshoshany bshoshany closed this May 26, 2023
@v1993
Copy link
Author

v1993 commented May 26, 2023

Sure! In the meantime, I'll be maintaining build files over at meson wrapdb. They are slightly newer than what this PR contains anyways, including an option to disable building tests and do so automatically on macos due to missing quick_exit in its libc. Those can be incorporated into upstream project with no to minimal tweaking.

@bshoshany
Copy link
Owner

bshoshany commented May 26, 2023

Sounds good, thanks!

I'm surprised macOS doesn't implement std::quick_exit, as it is part of the C++ standard library since C++11! I don't have access to macOS, so I can't test this, but if this is an issue I can remove std::quick_exit from the test, or modify the test to only use it on other operating systems. std::quick_exit is important because otherwise the test won't terminate if a deadlock occurs, but if this function is not implemented, I don't know if there's any other option. Let me know what you think.

@v1993
Copy link
Author

v1993 commented May 26, 2023

I'd personally go for not using std::quick_exit exclusively on MacOS, probably with #define-selected path in code and -DNO_QUICK_EXIT passed by build system if it detects unsuitable environment (std::terminate should work in such case). Should be simple enough.

P.S.: I'd appreciate if constexpred variables in tests, especially enable_tests/enable_benchmarks were controllable with defines to allow only running tests/benchmarks when invoking meson test/meson benchmark. Although, ideally, this also should apply to output_log too, since meson takes care to write down output of tests (and enable_long_deadlock_tests for consistency at this point).

@bshoshany
Copy link
Owner

Yes, I'll probably use #ifdef __APPLE__ or something like that (I'll try to get a MacBook from someone to check what works). Of course, macOS users will have to terminate the test manually if a deadlock occurs, but I'm not sure how to avoid that without std::quick_exit.

In the next release I plan to change all the enable/disable flags to command line arguments, so that should take care of your second request :)

@v1993
Copy link
Author

v1993 commented May 26, 2023

I'm pretty sure std::terminate bypasses normal destructor calls? One of situations when it's called is throwing from destructor, after all.

In the next release I plan to change all the enable/disable flags to command line arguments, so that should take care of your second request :)

Ah, that also works. Thanks!

@bshoshany
Copy link
Owner

std::terminate would certainly terminate the program, but it is basically equivalent to terminating manually, and it doesn't allow me to specify an exit code. std::quick_exit, to my understanding, is a "cleaner" exit, just not as clean as std::exit, and it lets me return an exit code, which is important for scripting purposes.

@v1993
Copy link
Author

v1993 commented May 27, 2023

Well, most importantly, std::terminate allows to signal an abnormal termination, possibly with core dump. While it doesn't let specify the exact exit code, it still gets across that program failed.

For scripting purposes and generally machine-interpreting test results I'd personally look into using TAP, which is pretty trivial to implement without relying on third-party libraries and has a plenty of harnesses, including one built into meson. It's also rather human-readable.

@v1993
Copy link
Author

v1993 commented May 27, 2023

Here are build files updated for version 3.5.0: mesonbuild/wrapdb#1032 (that PR doesn't touch meson_options.txt, but still uses it). I don't plan to maintain branch for PR we're having this conversation in.

@bshoshany
Copy link
Owner

Thanks! So since I don't know yet how meson works, it is possible to just update mesonbuild/wrapdb without adding any files to my repository (which is how I maintain the vcpkg and conan integration, and my preferred option since I don't like adding too many files to the repository), or do I have to add the files to my repository otherwise it won't work?

@v1993
Copy link
Author

v1993 commented May 28, 2023

Sure, that already works (meson's wrap system literally copies files into project's folder during build), although IMO it would still be beneficial to have it upstream for ease of building and running tests (once project is configured, meson test is sufficient to rebuild if needed and test) as well as potentially taking advantage of coverage reporting and similar features, like I've mentioned in PR description. But, of course, that's just my five cents and deciding for or against inclusion of build files upstream is up to you.

@v1993
Copy link
Author

v1993 commented May 28, 2023

Finally, it would probably be a good idea to include a note about being available as meson wrap. Something like:

Installing using Meson

If you're using Meson build system, this package can be used as a wrap available from WrapDB. To do so, run

meson wrap install bshoshany-thread-pool

Afterwards, calling dependency('bshoshany-thread-pool') will let you use this library like any other. If you need to update to new version, run

meson wrap update bshoshany-thread-pool

@bshoshany
Copy link
Owner

Thanks for the information. I have my own test program, and it's good enough for my purposes, so for now I don't see a reason to include the meson files in the GitHub repository (just like I don't include any files for vcpkg or conan).

For the next release, I will first spend some time learning how to use meson myself, and will then add information about how to use the package with meson to README.md as you suggested, as with the other two package managers.

I can also take over updating wrapdb whenever a new version of the thread pool is release, like I do with the other package managers. That will ensure it's always updated as soon as possible.

Thanks again! :)

@eli-schwartz
Copy link

eli-schwartz commented May 29, 2023

FWIW, meson isn't comparable to vcpkg/conan, but rather comparable to GNU autotools, or cmake.

All 3 tools allow you to:

  • build and install the project, in this case by installing BS_thread_pool.hpp to e.g. a Unix system at /usr/include/BS_thread_pool.hpp
  • create a pkg-config file so that other projects can run pkg-config --cflags --libs bshoshany-thread-pool and check that it's available, and which -I flags to add

vcpkg and conan and similar tools usually run meson/cmake/autotools to install the headers and pkg-config interface descriptions into their own managed library directories. They add a layer on top of meson/cmake/autotools.

And meson and cmake, but NOT autotools, allow other projects to:

  • copy this repository into their repository, and build both projects at the same time while merging definitions, which similarly to pkg-config files means that they can "link" to a named dependency interface that then handles -I flags automatically

This blurs the line between a build tool and a package manager, because meson/cmake are intrinsically the former but can do a lightweight form of the latter.

...

Projects that provide header-only libraries often feel much less motivation than projects that produce compiled static/shared libraries, to do without build systems -- "simply copy the header into your own sources".

But it really is a separate issue from the topic of package managers.

Also IMHO that misses the topic of pkg-config files for providing named dependency interfaces.

@bshoshany
Copy link
Owner

I see, thank you for the detailed explanation. Like I said, I'm not familiar with meson, so I'll have to spend some time learning what it does and how to use it before the next release of this project.

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.

3 participants