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

Use system zlib when available, tested on archlinux #653

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

tobchem
Copy link

@tobchem tobchem commented May 23, 2018

Haven't worked with meson before, so I hope it does not break things on other platforms. See here: http://mesonbuild.com/Dependencies.html

@mooskagh
Copy link
Contributor

Why not just use bundled one if it's included in sources anyway?

@Error323
Copy link
Collaborator

Error323 commented May 23, 2018 via email

@tobchem
Copy link
Author

tobchem commented May 23, 2018

It basically skips compilation on systems when it finds a pre-installed zlib.

I don't see a need to use an extra version of zlib, if the same version is already installed on my machine anyway.

@mooskagh mooskagh added the lc0 new tensorflow based implementation label May 25, 2018
@mooskagh
Copy link
Contributor

While I think there's no substantial reason to chose one variant over another, a have some weak arguments for always using zlib built from source (better than "I don't see a need"):

  • Fewer configuration variants-> fewer things that can possibly break, and easier to notice when they do break.
  • Also using system library makes it (a tiny bit) less likely to work on another machine after compilation and copy.

As for for warnings, they fixed it in meson wrap github already, but didn't push a new wrapdb revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lc0 new tensorflow based implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants