mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.51k stars 1.6k forks source link

has_header should accept internal dependencies from subprojects #13553

Open whs opened 1 month ago

whs commented 1 month ago

Describe the bug I'm getting #1430 but in subproject. The comment there states that internal dependency referring to external dependency is a misuse, but I think the use case is different here.

To Reproduce In this line: https://github.com/tlwg/libdatrie/blob/3985d6f7b5e56bc60bb9581337bf6a3782bd71be/meson.build#L39 locale_charset detection is performed. The function may be provided by iconv which can be compiler builtins or external library. The code works fine on Linux where said function is a builtins, but in Windows GNU libiconv needs to be installed.

However, when adding libiconv as a wrap (modify the deps to dependency('iconv', required: false, allow_fallback: true) plus adding a wrapfile) the detection throws ..\meson.build:31:31: ERROR: Dependencies must be external dependencies

I don't have libiconv wrap uploaded (I intend to upload it to wrapdb, but I need to test that it works first). The relevant lines should be:

iconv = library(
    'iconv',
    [
        config_h,
        '../libcharset/lib/localcharset.c',
        'relocatable.c',
        'relocatable.h',
        'iconv.c',
        additional_src,
    ],
    dependencies: [libcharset_dep],
    c_args: cflags,
    gnu_symbol_visibility: 'hidden',
    soversion: '8.1.6',  # LIBICONV_VERSION_INFO
    include_directories: include_dir,
    install: true,
)

iconv_dep = declare_dependency(
    include_directories: include_dir,
    link_with: iconv,
)
meson.override_dependency('iconv', iconv_dep)

Expected behavior

I expect that dependencies should work the same whether obtained through external package managers or internal package manager (wrapfile)

system parameters

The Meson build system
Version: 1.4.0
Source dir: C:\Users\Admin\Documents\libdatrie-0.2.13-25-ga3a5155
Build dir: C:\Users\Admin\Documents\libdatrie-0.2.13-25-ga3a5155\builddir
Build type: native build
Project name: libdatrie
Project version: 0.2.13-25-ga3a5155
Activating VS 17.10.0
C compiler for the host machine: cl (msvc 19.40.33808 "Microsoft (R) C/C++ Optimizing Compiler Version 19.40.33808 for x64")
C linker for the host machine: link link 14.40.33808.0
Host machine cpu family: x86_64
Host machine cpu: x86_64
eli-schwartz commented 1 month ago

The reason this is challenging -- and therefore the reason why meson reports that it cannot be done -- is because checking whether your wrap subproject of iconv contains

compiler.has_function('locale_charset', dependencies: iconv_dep)

requires writing out a C snippet and compiling and linking it. The linking is the problem. How can we:

works? Since we need to have fully configured the build in order to be able to produce libiconv.a / libiconv.so

eli-schwartz commented 1 month ago

I'm getting #1430 but in subproject. The comment there states that internal dependency referring to external dependency is a misuse, but I think the use case is different here.

This assumes that subprojects aren't internal, but subprojects are part of the same build.ninja and are indeed internal.

As noted in the other issue,

Internal dependencies are only available at build time and compiler checks are run at configure time.

whs commented 4 weeks ago

I've written ninja build generator as well, and I agree that this can't be done in Ninja without actually building something during the build generator stage, and soon you'll have reinvented Ninja.

Still, I believe the way this is done in other languages are exactly that - the packaging system itself whether Cargo or Nix are packaging/build system combo and they completely build all dependencies before the end packages. One could even write build.rs that depends on other packages.

I'm raising this to document an edge case of wrap's limitation - perhaps this can be addressed in future redesign of wrap. It's an issue in Meson the package manager, not Meson the build script generator.

Since the libdatrie PR is not yet merged, I'm not sure if there's a way to rewrite the check to work around this issue? The current check is ported as-is from autotools. Perhaps, for example, just checking for dependency('iconv') would replace both checks?

eli-schwartz commented 4 weeks ago

One possibility is to skip the check if the result of iconv_dep.type_name() == 'internal', as you assume your subproject always provides the invariant you need.

meson doesn't know whether to automatically return true or false, but you do know.

eli-schwartz commented 4 weeks ago

Aside: locale_charset is provided by localcharset.h, which is installed with GNU libiconv, but not GNU glibc as far as I know. Some programs include their own copy, usually by installing gnulib's "localcharset" module, and sometimes they even install the header and the symbol publicly (e.g. libunistring provides the symbol and the unistring/localcharset.h header).

You can maybe assume that if iconv's type_name is "system" rather than "builtin" that it is available. If it is "builtin" that symbol won't be in libc so checking for it is effectively a wasted compile.