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/bt as alternative build and packaging system; make enum deserialisation more robust #722

Merged
merged 12 commits into from
Mar 4, 2023

Conversation

matty0ung
Copy link
Contributor

The enum deserialisation is just adding a fallback for case-insensitive matching.

The Meson build stuff is mostly in two new files at the top-level directory: bt and meson.build. The former is a Python script that can:

  • install almost all the necessary dependencies for building the software
  • set up the Meson build environment (in the mbuild subdirectory)
  • after a Meson build is done (via cd mbuild; meson compile), package up the software (into mbuild/packages subtree)

Note that the current CMake/CPack build is pretty much unaffected by this new work. You can still run all the regular CMake stuff in the build subdirectory. CMake/CPack builds should continue to function as correctly as they do now (ie just fine on Windows and Linux, and fine except for package building on Mac).

@penguinpee
Copy link
Contributor

Nice! I'll give that a try as the build backend for the Fedora package. Are there plans for switching to Meson completely?

@matty0ung
Copy link
Contributor Author

Nice! I'll give that a try as the build backend for the Fedora package. Are there plans for switching to Meson completely?

There are still a few glitches to iron out, but once things seem like they are working, we'll see what folks think.

Personally I think using Meson simplifies the build, partly because Meson doesn't try to do all the things that CMake does, so you end up doing all the "not-build" things in a regular scripting language. And, for non-trivial scripting, I think Python is nicer than bash. Of course moving away from CPack it makes packaging less automagical, but CPack has been far from trouble-free, and the upside of doing things more explicitly is that we then have more control over library dependency and other packaging joys. Certainly doing the scripting has helped me understand a lot better how packaging works on Windows and Mac!

bt
Comment on lines +948 to +957
# and there is generally not a lot of info out there about how to do packaging in Meson. In fact, it seems unlikely
# that packaging will ever come within it scope. (Movement is rather in the other direction - eg there _used_ to be
# a Meson module for creating RPMs, but it was discontinued per
# https://mesonbuild.com/Release-notes-for-0-62-0.html#removal-of-the-rpm-module because it was broken and badly
# designed etc.)
#
# At first, this seemed disappointing, but I've rather come around to thinking a different way about it. Although
# CPack has lots of features it is also very painful to use. Some of the things you can do are undocumented; some of
# the things you want to be able to do seem nigh on impossible. So perhaps taking a completely different approach,
# eg using a scripting language rather than a build tool to do packaging, is ultimately a good thing.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I too am not all that convinced that Meson will gain this ability... but...

The rpm module, specifically, was not much of a packaging tool anyway. What it did was create an rpm spec file which then got copied to some directory which also had a meson dist source tarball, and run in order to install another copy of meson using rpm, build the project again, etc.

I suspect if it had been designed to package up the currently built version of the project, people might have actually used it, and then it wouldn't have been unmaintained, would not have broken, and wouldn't have ended up being deleted. :D

bt Outdated
Comment on lines 1042 to 1044
# --allow-dirty = allow uncommitted changes, which is needed more often than you think (eg for the automatic
# builds done when a commit is pushed up to github). HOWEVER, note this option is only available
# in Meson 0.62 and later.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this option is needed to restore previous behavior of warning that uncommitted changes will be ignored, which was difficult to notice in logs. In meson 0.62.0, the warning simply became fatal for better visibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I had slightly misunderstood that. Will fix the comment.

meson.build Outdated
# bindings, and C++11 or later for lambdas.
#
# - warning_level 3 is the highest level ie warns about the most things. For gcc it translates to
# -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Wpedantic

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed -Wnon-virtual-dtor for the record (in meson 1.0.0). It didn't have a good explanation for being there, and GCC devs think it's a bad idea to force projects to use it. It belongs in -Weffc++ rather than being lumped in with -Wall.

meson.build Outdated
Comment on lines 199 to 202
message('⭐ Building', projectName, 'version', meson.project_version(), 'for', host_machine.system(),
'on', build_machine.system(), 'in', meson.project_source_root(), '⭐')
compiler = meson.get_compiler('cpp')
message('Using Meson', meson.version(), ', ', compiler.get_id(), 'compiler and', compiler.get_linker_id(), 'linker')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much of this is necessary? Meson already has all this information logged, although in a different format.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's useful to have it in the main log for the github actions. I'll add a comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is that the console log looks something like this:

The Meson build system
Version: 1.0.1
Source dir: /home/eschwartz/git/mesonbuild/wrapdb/subprojects/zlib-1.2.13
Build dir: /home/eschwartz/git/mesonbuild/wrapdb/subprojects/zlib-1.2.13/builddir
Build type: native build
Project name: zlib
Project version: 1.2.13
C compiler for the host machine: ccache cc (gcc 11.1.0 "cc (GCC) 11.1.0")
C linker for the host machine: cc ld.bfd 2.36.1
Host machine cpu family: x86_64
Host machine cpu: x86_64

It's not hidden in the debug log.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, I see what you mean. You're right - no need to log it again manually.

meson.build Outdated
Comment on lines 205 to 210
# We need two versions of the main executable name because they are different on Windows:
# - We need a "target" name _without_ the '.exe' suffix for the executable() command because, on Windows, Meson will
# always add its own '.exe' suffix and we don't want to end up with '.exe.exe' as the suffix!
# - Everywhere else, we want the actual name of the file, including the '.exe' suffix.
#
# The default (on Windows and Linux) is to use Unix-style executable names, ie all lower case

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the basename or is the full filename okay? You can calculate the target name, then take the returned executable() object and use its .full_path() method to acquire the string value of the output filename, prefixed by the full builddir path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know about .full_path(). Will update comment.

# which we depend. However, I have yet to find a universal pain-free solution.
#
# Meson has its own dependency management system, Wrap, but the list of pre-provided projects at
# https://mesonbuild.com/Wrapdb-projects.html does not include Qt, Boost, Xerces, Xalan or Valijson.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qt at least would likely be far, far too painful to reimplement.

Anything else is probably feasible, and you can submit a request or a PR to add it to the WrapDB. We'll be happy to accept it. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One for the future! :)

meson.build Outdated
dlDependency = dependency('dl', required : needLibdl, static : true)
#
# On newer versions of Meson (0.62.0 and newer), the dependency has special-case code for dealing with libdl being built
# into the compiler. This means on Mac, for instance, we can have dlDependency.found() is true without there being any

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually in large part why Meson has a dl dependency. You shouldn't need to mess with ldd -- because dependency('dl') will check for the dl functionality with two possible "dependency providers":

  • check to see if dlopen() is available in libc
  • check to see if an external dl library is available

But on versions of Meson before 0.62.0, it will simply not find anything at all, even if needLibdl is true. So Meson will just fail in that case.

Granted, if a case-sensitive cmake Finddl.cmake exists then meson will fall back on that for meson versions <0.62.0, but I am not aware of cmake itself shipping with such a thing.

If you need to support meson 0.60.0, then I'd suggest not bothering with ldd at all, and just using this:

if cc.has_function('dlopen', prefix: '#include <dlfcn.h>')
  dl_dep = declare_dependency()
else
  dl_dep = cc.find_library('dl', has_headers: 'dlfcn.h')
endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's much more elegant. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that it doesn't work on Windows. 😿
Well, we'll do the new way on Mac and Linux at least.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does dl do on Windows anyway? Is that just when building using a mingw-based compiler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good question. Maybe I'll try not having it as a dependency at all on Windows.

meson.build Outdated
Comment on lines 445 to 448
# For BeerXML processing we need Xerces-C++ and Xalan-C++. Meson can find both of these automatically using CMake's
# find_package(), as long as (a) CMake is installed(!) and (b) we provide the right library names ('XercesC' per
# https://cmake.org/cmake/help/latest/module/FindXercesC.html and 'XalanC' per
# https://cmake.org/cmake/help/latest/module/FindXalanC.html)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the official way to search for both is with dependency('xerces-c') and dependency('xalan-c'). xalan-c provides a cmake package, but xerces-c doesn't.

Both provide pkg-config files which are lowercased and have a hyphen, so you need to use the meson 0.60.0 support for dependency('xerces-c', 'XercesC') with multiple possible names for the same dependency call, if you want to be able to find either one. If you can find it with pkg-config, you don't need cmake installed. You can get the 'libdir' variable from the dependency if its .type_name() == 'pkgconfig'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, OK, that's interesting.
AFAICT, the downside of using dependency('xerces-c', 'XercesC') is that, if the dependency is found natively by Meson (rather than calling out to CMake's find_package()), then we don't have a way to know what directory the library was found in, which is rather nice to have for passing out to the packaging scripts. But very happy to be corrected about this!

meson.build Outdated
# on.
#
# We ultimately want the generated files inside the 'packaging' subtree of the the build directory, but that is not
# permitted by configure_file, so, that gets done by the bt script.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can, if you use subdir('packaging') and then define the configure_file() inside packaging/meson.build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, neat. I didn't know about that. Thanks!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can also be useful for organizational purposes and to avoid unbounded growth of the root meson.build file. :)

For example, some people prefer to subdir into the src/ directory and append to a global list of sources to keep long lists of filenames separate from top-level logic about how to use them.

meson.build Outdated
#
pandocResult = run_command(
'pandoc',
join_paths(meson.current_build_dir(), 'manpage.1.md'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you save the configure_file() returned value from above, you can pass it into command lines here.

Any particular reason you used run_command() instead of something like:

man_file = configure_file(
    input: configured_md_file,
    output: projectName + '.1',
    command: ['pandoc', ......],
)
install_man(man_file)

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, OK, so I can use configure_file again, but this time with a custom command. That hadn't occurred to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't noticed that configure_file returns a file object!

meson.build Outdated
capture : true,
check : true
command: ['pandoc', manPageMarkdown, '--verbose', '-t', 'man', '-s'],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
command: ['pandoc', manPageMarkdown, '--verbose', '-t', 'man', '-s'],
command: ['pandoc', '@INPUT@', '--verbose', '-t', 'man', '-s'],

Instead of repeating manPageMarkdown, you can have Meson format the input location into the command.

@matty0ung matty0ung merged commit e234270 into Brewtarget:develop Mar 4, 2023
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

3 participants