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

Rely only on pkg-config to find zlib include directory #460

Merged
merged 1 commit into from Mar 26, 2024

Conversation

sagudev
Copy link
Member

@sagudev sagudev commented Mar 26, 2024

This solves servo/servo#31851 for me. The problem is that if libz is installed on system we add it's path to includes which causes problems. I think we do not need this as SM build system depends on pkg-config to find this.

@sagudev
Copy link
Member Author

sagudev commented Mar 26, 2024

Relevant configure code:

@depends(js_standalone, target)
def system_zlib_default(js_standalone, target):
return (
js_standalone
and target.kernel not in ("WINNT", "Darwin")
and target.os != "Android"
)
option(
"--with-system-zlib",
nargs="?",
default=system_zlib_default,
help="{Use|Do not use} system libz",
)
@depends("--with-system-zlib")
def deprecated_system_zlib_path(value):
if len(value) == 1:
die(
"--with-system-zlib=PATH is not supported anymore. Please use "
"--with-system-zlib and set any necessary pkg-config environment variable."
)
pkg_check_modules("MOZ_ZLIB", "zlib >= 1.2.3", when="--with-system-zlib")
set_config("MOZ_SYSTEM_ZLIB", True, when="--with-system-zlib")
@depends("--with-system-zlib", js_shared, moz_linker, target.os)
def zlib_in_mozglue(system_zlib, js_shared, linker, os):
if not system_zlib and (js_shared or linker or os == "Android"):
return True
set_config("ZLIB_IN_MOZGLUE", zlib_in_mozglue)
set_define("ZLIB_IN_MOZGLUE", zlib_in_mozglue)

@sagudev
Copy link
Member Author

sagudev commented Mar 26, 2024

System zlib is only used on linux, on other platform (android, darwin, windows) it is built from sources, and the only way to detect it on system is via pkg-config (which windows env does not have even in moztools).

@sagudev
Copy link
Member Author

sagudev commented Mar 26, 2024

We could potentially completely remove libz dependency and instead relay on included zlib on linux too, or just limit libz on linux target.

@sagudev sagudev marked this pull request as ready for review March 26, 2024 18:25
@sagudev sagudev changed the title use only DEP_Z_ROOT if needed Relay on pkg-config only for zlib include Mar 26, 2024
@sagudev sagudev changed the title Relay on pkg-config only for zlib include Relay only on pkg-config for zlib include Mar 26, 2024
@mrobinson mrobinson changed the title Relay only on pkg-config for zlib include Rely only on pkg-config to find zlib include directory Mar 26, 2024
@sagudev sagudev added this pull request to the merge queue Mar 26, 2024
Merged via the queue into servo:main with commit 0172cf4 Mar 26, 2024
20 checks passed
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

2 participants