-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix pkg-config dependencies leaking out (debbug 892956) #3251
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -510,7 +510,8 @@ def tearDown(self): | |
windows_proof_rmtree(path) | ||
except FileNotFoundError: | ||
pass | ||
os.environ = self.orig_env | ||
os.environ.clear() | ||
os.environ.update(self.orig_env) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious, why is this needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I spend some time debugging why the test worked in isolation but not with the whole test suit and decided to fix the root cause, by not assigning but by replacing the contents back into the special object. |
||
super().tearDown() | ||
|
||
def _run(self, command, workdir=None): | ||
|
@@ -585,10 +586,11 @@ def clean(self): | |
def run_tests(self): | ||
self._run(self.test_command, workdir=self.builddir) | ||
|
||
def install(self): | ||
def install(self, *, use_destdir=True): | ||
if self.backend is not Backend.ninja: | ||
raise unittest.SkipTest('{!r} backend can\'t install files'.format(self.backend.name)) | ||
os.environ['DESTDIR'] = self.installdir | ||
if use_destdir: | ||
os.environ['DESTDIR'] = self.installdir | ||
self._run(self.install_command, workdir=self.builddir) | ||
|
||
def uninstall(self): | ||
|
@@ -2712,6 +2714,42 @@ def test_old_gnome_module_codepaths(self): | |
self.build() | ||
mesonbuild.modules.gnome.native_glib_version = None | ||
|
||
@unittest.skipIf(shutil.which('pkg-config') is None, 'Pkg-config not found.') | ||
def test_pkgconfig_usage(self): | ||
testdir1 = os.path.join(self.unit_test_dir, '24 pkgconfig usage/dependency') | ||
testdir2 = os.path.join(self.unit_test_dir, '24 pkgconfig usage/dependee') | ||
if subprocess.call(['pkg-config', '--cflags', 'glib-2.0'], | ||
stdout=subprocess.DEVNULL, | ||
stderr=subprocess.DEVNULL) != 0: | ||
raise unittest.SkipTest('Glib 2.0 dependency not available.') | ||
with tempfile.TemporaryDirectory() as tempdirname: | ||
self.init(testdir1, ['--prefix=' + tempdirname, '--libdir=lib'], default_args=False) | ||
self.install(use_destdir=False) | ||
shutil.rmtree(self.builddir) | ||
os.mkdir(self.builddir) | ||
pkg_dir = os.path.join(tempdirname, 'lib/pkgconfig') | ||
self.assertTrue(os.path.exists(os.path.join(pkg_dir, 'libpkgdep.pc'))) | ||
lib_dir = os.path.join(tempdirname, 'lib') | ||
os.environ['PKG_CONFIG_PATH'] = pkg_dir | ||
# Private internal libraries must not leak out. | ||
pkg_out = subprocess.check_output(['pkg-config', '--static', '--libs', 'libpkgdep']) | ||
self.assertFalse(b'libpkgdep-int' in pkg_out, 'Internal library leaked out.') | ||
# Dependencies must not leak to cflags when building only a shared library. | ||
pkg_out = subprocess.check_output(['pkg-config', '--cflags', 'libpkgdep']) | ||
self.assertFalse(b'glib' in pkg_out, 'Internal dependency leaked to headers.') | ||
# Test that the result is usable. | ||
self.init(testdir2) | ||
self.build() | ||
myenv = os.environ.copy() | ||
myenv['LD_LIBRARY_PATH'] = lib_dir | ||
if is_cygwin(): | ||
bin_dir = os.path.join(tempdirname, 'bin') | ||
myenv['PATH'] = bin_dir + os.pathsep + myenv['PATH'] | ||
self.assertTrue(os.path.isdir(lib_dir)) | ||
test_exe = os.path.join(self.builddir, 'pkguser') | ||
self.assertTrue(os.path.isfile(test_exe)) | ||
subprocess.check_call(test_exe, env=myenv) | ||
|
||
|
||
class LinuxArmCrossCompileTests(BasePlatformTests): | ||
''' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
project('pkgconfig user', 'c') | ||
|
||
pkgdep = dependency('libpkgdep') | ||
|
||
executable('pkguser', 'pkguser.c', | ||
dependencies : pkgdep) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
#include<pkgdep.h> | ||
|
||
int main(int argc, char **argv) { | ||
int res = pkgdep(); | ||
return res != 99; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
project('pkgconfig dep', 'c', | ||
version : '1.0.0') | ||
|
||
# This is not used in the code, only to check that it does not | ||
# leak out to --libs. | ||
glib_dep = dependency('glib-2.0') | ||
|
||
pkgconfig = import('pkgconfig') | ||
|
||
intlib = static_library('libpkgdep-int', 'privatelib.c') | ||
intdep = declare_dependency(link_with : intlib) | ||
|
||
lib = shared_library('pkgdep', 'pkgdep.c', | ||
dependencies : [glib_dep, intdep], | ||
install : true) | ||
|
||
install_headers('pkgdep.h') | ||
|
||
pkgconfig.generate( | ||
filebase : 'libpkgdep', | ||
name : 'Libpkgdep', | ||
description : 'Sample pkgconfig dependency library', | ||
version : meson.project_version(), | ||
libraries : lib) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
#include<pkgdep.h> | ||
|
||
int internal_thingy(); | ||
|
||
int pkgdep() { | ||
return internal_thingy(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#pragma once | ||
|
||
int pkgdep(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
int internal_thingy() { | ||
return 99; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this if should also be done for build.SharedLibrary. (i.e. obj.generated_pc = self.name if public)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing that is exactly what caused this bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
I think what causes this bug is one (or both) of the
self.add_priv_libs(
lines.obj.generated_pc is only used if something else depends on this library, possibly even explicitly.