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 support for the Meson build system #495

Merged
merged 15 commits into from Jul 9, 2020

Conversation

nirbheek
Copy link
Contributor

@nirbheek nirbheek commented Jun 9, 2020

Hi! I'm a GStreamer developer, and for the same reasons as OpenH264, we've written Meson build files for libsrtp. We use libsrtp in gstreamer for WebRTC DTLS.

The Meson build system is being adopted by projects such as GNOME, GTK, Mesa, GStreamer, etc. I am not sure if such a pull request is welcome, but it would be amazing if you could consider merging this :)


These Meson build files have feature-parity with the Autoconf/Make build files, the Visual Studio solution, and the CMake build files. They are also simpler, faster, and support all target platforms supported by libsrtp (that I know of). The following have been tested:

Linux, macOS, Windows (MinGW), Windows (MSVC), Windows (UWP/WinRT), Cross-Android (all arches), Cross-iOS (all arches), Cross-Linux, Cross-Windows (MinGW).

Meson supports a superset of the currently-supported target platforms, and no platform-specific code should be needed for those, so this also improves cross-platform support.

The build outputs should be ABI-compatible with the outputs by Autoconf, but this has not been verified in detail.

I've also updated the README, and (pasted from there) the steps to try this out are:

# Setup the build subdirectory
meson setup --prefix=/path/to/prefix builddir
# Build the project
meson compile -C builddir
# Run tests
meson test -C builddir
# Optionally, install
meson install -C builddir

tp-m and others added 3 commits Jun 9, 2020
It's either defined or undefined, don't rely on implicit
0 value if it's not defined, this will cause compiler
warnings and is a bit crufty.
Modify aes_calc to optionally accept the expected ciphertext as
a third argument, so it can error with a non-0 exit code if the
calculated ciphertext doesn't match the expected ciphertext. This
makes our test setup with meson much easier, as we don't need to parse
and compare the output of the test binary.

The test is now self-sufficient.
Everywhere else we use `#ifdef`, but here we use `#if`. This causes
a build error when using `#define CPU_RISC` like the meson build files
do.
@nirbheek nirbheek force-pushed the add-meson-build-system branch from d42306b to 717e33d Compare Jun 9, 2020
@pabuhler
Copy link
Member

pabuhler commented Jun 9, 2020

Hi, I have nothing really against including meson besides the potential maintenance over head. There has been no real discussion on replacing autotools and the current cmake support was seen both as a way to replace hard coded windows project files and to provide a "cross platform" solution to reduce maintenance. If this was to be accepted and you would like to maintain some hope that it does not get broken then I would suggest adding at least a standard linux build to the travis build file.
If meson is gaining traction and it helps (more than one) other projects to have it here then I am ok with it. Lets see what others think?

@nirbheek
Copy link
Contributor Author

nirbheek commented Jun 10, 2020

Thanks for looking at this @pabuhler :)

Sorry for not being clear, my intention is not to advocate for Meson to replace any of the build systems used by libsrtp. I'm an outsider to the project after all.

As for maintenance overhead, I'd be happy to maintain these build files. I can add a CODEOWNERS file with myself in it and I'll get email about all PRs that touch the build files. We're going to be using this in GStreamer, so we will be doing active maintenance too.

I'll also add some Travis CI builds, thanks for the suggestion!

@nirbheek
Copy link
Contributor Author

nirbheek commented Jun 10, 2020

A datapoint in favour of the idea that this would help more than one project is that there's also a set of meson build files in Meson's WrapDB (code: https://github.com/mesonbuild/libsrtp2/tree/2.2.0) written independently by @xhaakon.

@xhaakon would you also like to be in CODEOWNERS for the meson build files in libsrtp if this is merged?

@nirbheek nirbheek force-pushed the add-meson-build-system branch 13 times, most recently from bb101e2 to e149229 Compare Jun 10, 2020
fdo-mirror pushed a commit to freedesktop/gstreamer-cerbero that referenced this pull request Jun 10, 2020
@nirbheek nirbheek force-pushed the add-meson-build-system branch 9 times, most recently from d7e5df9 to e8b24b2 Compare Jun 10, 2020
test/rtpw.c(561): warning C4244: 'function': conversion from 'double' to 'DWORD', possible loss of data
@nirbheek nirbheek force-pushed the add-meson-build-system branch from c2e1adf to ada6e8f Compare Jun 10, 2020
@nirbheek
Copy link
Contributor Author

nirbheek commented Jun 10, 2020

I'll also add some Travis CI builds, thanks for the suggestion!

I've implemented Meson counterparts for all CI build jobs, and I added a new one specifically for testing the build on WinRT/UWP too. Tests seem to be passing everywhere they could be run.

CI now takes ~1-2min more than it used to because the big-endian mips test is the bottleneck and we run tests using Meson there too now.

@havardgraff
Copy link

havardgraff commented Jun 10, 2020

@pabuhler I can warmly recommend Meson! It makes everything hard and clunky about cross-platform development easy and lean.

@pabuhler
Copy link
Member

pabuhler commented Jun 11, 2020

@nirbheek thanks for adding the builds to travis, that would have taken me a bit of time to figure out. I think my position has not changed, this is fine for me as long as there is not to much maintenance and if that becomes an issue we can just bin it later. Will wait abit to see if any other people have comments.
@havardgraff I'll take your advice in to consideration ;-)

@nirbheek
Copy link
Contributor Author

nirbheek commented Jun 11, 2020

@tp-m pointed out to me that it would be good to add a make distcheck target that checks whether the project version in the meson build files is correct. I'm not sure if I did it correctly, but I've added a rough check to the make distribution target with instructions on how to update it. It looks like this:

===================================================
Meson project version is 2.3.0 which is incorrect.
Please edit meson.build and change the 'version:'
field in the project() call to 2.4.0
===================================================

The project() call is the first line in the build file, so should be easy to find.

@xhaakon
Copy link
Contributor

xhaakon commented Jun 12, 2020

@xhaakon would you also like to be in CODEOWNERS for the meson build files in libsrtp if this is merged?

Nothing against that, feel free to add me.

nirbheek added 8 commits Jun 12, 2020
To get output when the check is run:

Checking if "inline keyword check" compiles: YES
Ensure that the meson project version matches the release project
version when `make distribution` is run to make a release.

Also update the meson version to the correct one: 2.4.0
Run tests under valgrind by calling:

meson test --setup=valgrind -C builddir

Pass -v / --verbose to see test output while tests are running
We need to include crtdefs.h for the definition of size_t. See:

https://docs.microsoft.com/en-us/cpp/c-runtime-library/standard-types

Add CI so this doesn't break again.
@nirbheek nirbheek force-pushed the add-meson-build-system branch from fcf3cf6 to e09fbfa Compare Jun 12, 2020
@nirbheek
Copy link
Contributor Author

nirbheek commented Jun 12, 2020

Nothing against that, feel free to add me.

Done, thanks!

Also fixed a build failure on msvc-32-bit and added CI for windows UWP arm64.

Edit: CI has passed, but it's not showing status on the PR for some reason.

@nirbheek nirbheek force-pushed the add-meson-build-system branch from 8cf9b69 to 32d4d31 Compare Jun 12, 2020
For some reason, pip3 is now trying to build ninja instead of
installing the prebuilt binary. Try choco instead.
@nirbheek nirbheek force-pushed the add-meson-build-system branch from 32d4d31 to bfde963 Compare Jun 30, 2020
root: deprecated key sudo (The key `sudo` has no effect anymore.)
jobs.include: deprecated key sudo (The key `sudo` has no effect anymore.)
@nirbheek
Copy link
Contributor Author

nirbheek commented Jul 1, 2020

Gentle poke :)

@pabuhler
Copy link
Member

pabuhler commented Jul 6, 2020

@nirbheek , sorry for the delay, have been out of the office for a few weeks. Will follow up now.

@pabuhler pabuhler merged commit 0da0fb1 into cisco:master Jul 9, 2020
1 check passed
@pabuhler
Copy link
Member

pabuhler commented Jul 9, 2020

@nirbheek for me supporting meson seams non trivial as with all build systems there is lots to learn and when adapting to an existing projects there are always special case handling that require even more work. So we can see how this goes and if it does not cause to much work then great but if it becomes a problem we will have to review it.

@tp-m
Copy link
Contributor

tp-m commented Jul 9, 2020

Thanks a lot for merging this!

I'm also happy to help out if there are any issues related to the Meson build, or with keeping it up to date.

@nirbheek nirbheek deleted the add-meson-build-system branch Jul 9, 2020
@nirbheek
Copy link
Contributor Author

nirbheek commented Jul 9, 2020

Many thanks for merging this 😄 I hope it works out for all of us. Please poke me when you have questions. Either on GitHub, or on FreeNode (nirbheek). I am on #gstreamer and #mesonbuild. And of course, since we're all in the CODEOWNERS file, we will get email for PRs to the meson build files.

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.

None yet

5 participants