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): CMake's Package Version file is installed in the wrong place #140

Closed
Tachi107 opened this issue Jan 31, 2022 · 7 comments · Fixed by #141
Closed

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

Tachi107 opened this issue Jan 31, 2022 · 7 comments · Fixed by #141
Assignees
Labels
bug Something isn't working

Comments

@Tachi107
Copy link
Contributor

Tachi107 commented Jan 31, 2022

Environment

toml++ version and/or commit hash: 248e603 to present

Target arch: all

Describe the bug

Quoting 248e603#commitcomment-65698096

  1. The architecture-independent directory of CMake Config and Version files is datadir/cmake
  2. CMake Version files are not architecture independent unless created with the ARCHITECTURE_INDEPENDENT option, and that is currently not supported by Meson (from the CMake docs : "an architecture check will be performed, and the package will be considered compatible only if the architecture matches exactly. For example, if the package is built for a 32-bit architecture, the package is only considered compatible if it is used on a 32-bit architecture, unless ARCH_INDEPENDENT is given")

I don't use personally use the CMake integration, but as a (newbie) Debian package maintainer I'm starting to care about architecture compatibilities. The most appropriate fix would be to set ARCH_INDEPENDENT, but that would require a patch in upstream Meson, so we currently have three options:

  1. Keep using Meson's write_basic_package_version_file(), but move the output to the arch-dependent libdir/cmake - might require to also move the Config file, and it is not right
  2. Manually write an arch-independent CMake Version file, and install that in datadir/cmake - I don't know if you ever looked at one, but they are not really human-friendly
  3. Drop the CMake Version file and keep only the Config file, and wait for a fix in Meson (I'm working on it) - users would still be able to run find_package(tomlplusplus), but without specifying the desired version

I haven't used Toml++ in quite some time, but I believe that this library can be consumed as both an header-only library and a compiled library, right? If so, neither the pkg-config file nor the CMake ones should unconditionally get installed in the architecture-independent directory, but that should depend on the current configuration (libdir when compiling the library, datadir when header-only)

@Tachi107 Tachi107 added the bug Something isn't working label Jan 31, 2022
Tachi107 referenced this issue Jan 31, 2022
#60)

toml++ is header-only so these should be installed into architecture-independent directories to allow them to be found for crosscompiling regardless of the architecture they were installed on.

Fixes #59.
@marzer
Copy link
Owner

marzer commented Jan 31, 2022

but I believe that this library can be consumed as both an header-only library and a compiled library, right?

The library is always header-only. The user can choose to 'compile' it by disabling header-only mode and marking a specific TU as the 'implementation' one, but that's got nothing to do with the build system in the traditional sense.

@Tachi107
Copy link
Contributor Author

Unrelated to this issue, but would you accept a PR enabling support for building and installing the library implemented in Meson? I ask because I'd like to package this library for Debian (and derivatives), and traditional libraries are generally preferred to header-only ones (thanks to shared linking- packaging a header-only library doesn't make users' lives better). I'm thinking to something similar to how cpp-httplib does it (but without the split.py part), you can take a look here: https://github.com/yhirose/cpp-httplib/blob/master/meson.build

@marzer
Copy link
Owner

marzer commented Jan 31, 2022

Unrelated to this issue, but would you accept a PR enabling support for building and installing the library implemented in Meson?

As long as it's not overly disruptive to current workflows and isn't cumbersome to maintain, sure. It being header-only is mostly for my benefit.

Unrelated to this issue,

What is the path forward for this this issue? I'm not going to do anything about CMake-related issues myself, since I value my blood un-boiled - I'd sooner just close it. From your description above, it sounds like "do nothing until meson gets a fix" is the right move, but I don't know enough about the interop to make any real suggestions here.

@Tachi107
Copy link
Contributor Author

The fix to Meson is pending in mesonbuild/meson#9916, but will require toml++ to bump its Meson requirement to version 0.62.0 (when released).

Doing nothing about this would prevent the library from getting packaged in Linux distributions and basically any other package manager (or at least that's what I would do), since it causes issues for things like cross-architecture compatibility.

In my opinion, the "best" solution to this is to drop Meson's write_basic_package_version_file() for the time being and ship a fixed Version file stored in cmake/, and when Meson 0.62.0 will be released the usage of the generator could be restored.

If you like this solution, I could prepare a PR :)

@marzer
Copy link
Owner

marzer commented Jan 31, 2022

If you like this solution, I could prepare a PR :)

Sounds good to me, fire away

@eli-schwartz
Copy link

Doing nothing about this would prevent the library from getting packaged in Linux distributions and basically any other package manager (or at least that's what I would do), since it causes issues for things like cross-architecture compatibility.

They would also have the option to configure the project with -Dgenerate_cmake_config=false. :D

but will require toml++ to bump its Meson requirement to version 0.62.0 (when released).

Not true. Given:

tomlplusplus/meson.build

Lines 534 to 538 in 8e669aa

cmake.write_basic_package_version_file(
name: meson.project_name(),
version: meson.project_version(),
install_dir: 'lib'/'cmake'/meson.project_name(),
)

This could be changed to:

	# cmake_installdir = get_option('build_libary') ? 'lib' : get_option('datadir')
	# cmake_installdir = cmake_installdir / 'cmake' / meson.project_name()
	cmake_installdir = 'lib/cmake' / meson.project_name()
	if meson.version().version_compare('>=0.62.0')
		cmake.write_basic_package_version_file(
			name: meson.project_name(),
			version: meson.project_version(),
			arch_independent: true, # get_option('build_library')
			install_dir: cmake_installdir,
		)
	else
		cmake.write_basic_package_version_file(
			name: meson.project_name(),
			version: meson.project_version(),
			install_dir: cmake_installdir,
		)
	endif

It would then generate an architecture-dependent file on older versions of meson, and an architecture-independent file on newer versions of meson. People using old versions of meson would "pay the price" by having worse versions of the file, which they may or may not care about (Debian certainly does).

You could also use:

	if meson.version().version_compare('>=0.62.0')
		cmake_kwargs = {'arch_independent': true}
	else
		cmake_kwargs = {}
	endif
	cmake.write_basic_package_version_file(
		name: meson.project_name(),
		version: meson.project_version(),
		install_dir: 'lib'/'cmake'/meson.project_name(),
		kwargs: cmake_kwargs,
	)

but that would emit a FeatureNew warning on 0.62.

@Tachi107
Copy link
Contributor Author

Tachi107 commented Feb 1, 2022

They would also have the option to configure the project with -Dgenerate_cmake_config=false. :D

Right

but will require toml++ to bump its Meson requirement to version 0.62.0 (when released).

Not true. Given:

tomlplusplus/meson.build

Lines 534 to 538 in 8e669aa

cmake.write_basic_package_version_file(
name: meson.project_name(),
version: meson.project_version(),
install_dir: 'lib'/'cmake'/meson.project_name(),
)

This could be changed to:

	# cmake_installdir = get_option('build_libary') ? 'lib' : get_option('datadir')
	# cmake_installdir = cmake_installdir / 'cmake' / meson.project_name()
	cmake_installdir = 'lib/cmake' / meson.project_name()
	if meson.version().version_compare('>=0.62.0')
		cmake.write_basic_package_version_file(
			name: meson.project_name(),
			version: meson.project_version(),
			arch_independent: true, # get_option('build_library')
			install_dir: cmake_installdir,
		)
	else
		cmake.write_basic_package_version_file(
			name: meson.project_name(),
			version: meson.project_version(),
			install_dir: cmake_installdir,
		)
	endif

Oh right, ifs... I always forget about them :/

Tachi107 added a commit to Tachi107/tomlplusplus that referenced this issue Feb 1, 2022
Since Meson doesn't yet support CMake's ARCH_INDEPENDENT option, a
pre-generated Package Version file in installed instead of generating it
at configure time.

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

Fixes marzer#140
Tachi107 added a commit to Tachi107/tomlplusplus that referenced this issue 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 marzer#140
Tachi107 added a commit to Tachi107/tomlplusplus that referenced this issue Feb 1, 2022
Tachi107 added a commit to Tachi107/tomlplusplus that referenced this issue Feb 1, 2022
Tachi107 added a commit to Tachi107/tomlplusplus that referenced this issue Feb 1, 2022
Tachi107 added a commit to Tachi107/tomlplusplus that referenced this issue Feb 2, 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 marzer#140
marzer pushed a commit that referenced this issue Feb 2, 2022
* build(meson): install CMake Config files to datadir

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

* build(meson): don't hardcode include in CMake Config
Tachi107 added a commit to Tachi107/tomlplusplus that referenced this issue Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants