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 a "system" dependency for zlib #6421

Merged
merged 6 commits into from
Feb 7, 2020

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Jan 7, 2020

This adds a system depenedency for zlib, so that platforms that ship zlib as part of the base os (macOS, FreeBSD, and probably other BSDs but I don't have VMs for all of them). This allows zlib to be found without the use of cmake of pkg-config, which is especially helpful on macOS.

This has been updated on top of #6443

Fixes #2654

@xclaesse
Copy link
Member

xclaesse commented Jan 7, 2020

It is not limited to macosx/bsd, for example when cross compiling for Android NDK it also have the zlib but no pkg-config file. For example, here is how GLib search for zlib: https://gitlab.gnome.org/GNOME/glib/blob/master/meson.build#L1872. This is the kind of mess I'm trying to solve with #4595.

That being said, I guess macosx/freebsd are popular enough to have their special case until we have a fully-fledged solution.

@dcbaker
Copy link
Member Author

dcbaker commented Jan 7, 2020

I wouldn't want to do this for every dependency, but zlib is really popular so having a built-in solution seems better to me.

@scivision
Copy link
Member

I have added several dependencies like this, and would eventually add numerous more, because of the frequent imperfection in packages' pkg-config files and other corner cases. Another choice would be to lean heavily on CMake inside Meson, since CMake has typically already solved the corner cases.

@jpakkane
Copy link
Member

jpakkane commented Jan 8, 2020

  File "D:\a\1\s\mesonbuild\dependencies\base.py", line 2206, in find_external_dependency
    candidates = _build_external_dependency_list(name, env, kwargs)
  File "D:\a\1\s\mesonbuild\dependencies\base.py", line 2281, in _build_external_dependency_list
    dep = packages[lname]._factory(env, kwargs)
  File "D:\a\1\s\mesonbuild\dependencies\misc.py", line 377, in _factory
    methods = cls._process_method_kw(kwargs)
TypeError: _process_method_kw() missing 1 required positional argument: 'kwargs'

@dcbaker
Copy link
Member Author

dcbaker commented Jan 8, 2020

Yup. I've got another series that cleans up a lot of cruft in the dependencies (mostly in the dependencies that have > 1 way to find them) that I think should land before this, but isn't done. Consider this on hold until then.

@nirbheek
Copy link
Member

There's two more corner-cases that should probably go in here: the library is sometimes called zlib.lib and sometimes zlib1.lib: https://gitlab.gnome.org/GNOME/glib/blob/master/meson.build#L1872

@dcbaker
Copy link
Member Author

dcbaker commented Jan 17, 2020

@nirbheek

There's two more corner-cases that should probably go in here: the library is sometimes called zlib.lib and sometimes zlib1.lib: https://gitlab.gnome.org/GNOME/glib/blob/master/meson.build#L1872

That code seems to have been (re)moved. I'm assuming (based on .lib) that's for windows?

@xclaesse
Copy link
Member

@dcbaker it's just a few lines down:

# Don't use the bundled ZLib sources until we are sure that we can't find it on
# the system
libz_dep = dependency('zlib', required : false)
if not libz_dep.found()
  if cc.get_id() != 'msvc' and cc.get_id() != 'clang-cl'
    libz_dep = cc.find_library('z', required : false)
  else
    libz_dep = cc.find_library('zlib1', required : false)
    if not libz_dep.found()
      libz_dep = cc.find_library('zlib', required : false)
    endif
  endif
  if not libz_dep.found() or not cc.has_header('zlib.h')
    libz_dep = subproject('zlib').get_variable('zlib_dep')
  endif
endif

@dcbaker
Copy link
Member Author

dcbaker commented Jan 17, 2020

Okay, added windows, added a test case. lets see what happens.

@dcbaker dcbaker force-pushed the zlib-system-dep branch 2 times, most recently from 05e00e9 to 63b725b Compare January 17, 2020 20:33
This comes pre-installed, but currently we don't have a way to detect it
without relying on pkg-config or cmake. This is only valid with the
apple clang-based compilers, as they do some special magic to get
headers.
I've tested this on FreeBSD, and dragonfly's userland is close enough
I'm willing to call it good without testing. OpenBSD and NetBSD also
have a zlib in their base configurations, but I'm not confident enough
to enable those without testing.
@dcbaker
Copy link
Member Author

dcbaker commented Feb 3, 2020

Okay off hold, @xclaesse, does this look good?

@scivision
Copy link
Member

I have not tried to use Zlib from Windows. So this may be just a fluke test result. Here's what I got from VS 2019:

> meson build

The Meson build system
Version: 0.53.999
Source dir: meson\test cases\common\228 zlib
Build dir: meson\test cases\common\228 zlib\build
Build type: native build
Project name: zlib system dependency
Project version: undefined
C compiler for the host machine: cl (msvc 19.24.28314)
C linker for the host machine: link link 14.24.28314.0
Host machine cpu family: x86_64
Host machine cpu: x86_64
Library z found: NO
Library zlib found: NO
Library zlib1 found: NO

meson.build:19:2: ERROR: Problem encountered: MESON_SKIP_TEST Cannot seem to find zlib via find_library, this test will probably fail.

@dcbaker
Copy link
Member Author

dcbaker commented Feb 5, 2020

I'm not sure how @nirbheek and @xclaesse are installing zlib on windows, it's not part of the base OS AFAICT

@scivision
Copy link
Member

OK then disregard my result from VS 2019, thanks

@jpakkane
Copy link
Member

jpakkane commented Feb 5, 2020

LGTM.

@jpakkane jpakkane merged commit 31d89a4 into mesonbuild:master Feb 7, 2020
@lantw44
Copy link
Contributor

lantw44 commented Feb 9, 2020

This adds a system depenedency for zlib, so that platforms that ship zlib as part of the base os (macOS, FreeBSD, and probably other BSDs but I don't have VMs for all of them).

In fact, FreeBSD provides zlib.pc as a part of the OS since 10.1 release: https://svnweb.freebsd.org/changeset/base/267376. Since all FreeBSD releases without zlib.pc are EOL, it should be safe to say that zlib.pc is always available on FreeBSD.

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.

Add special-case dependency for zlib
6 participants