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

Fix pkg-config dependencies leaking out (debbug 892956) #3251

Merged
merged 3 commits into from
Mar 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion mesonbuild/modules/pkgconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,20 @@ def _process_libs(self, libs, public):
if obj.found():
processed_libs += obj.get_link_args()
processed_cflags += obj.get_compile_args()
elif isinstance(obj, (build.SharedLibrary, build.StaticLibrary)):
elif isinstance(obj, build.SharedLibrary):
processed_libs.append(obj)
if public:
if not hasattr(obj, 'generated_pc'):
obj.generated_pc = self.name
elif isinstance(obj, build.StaticLibrary):
# Due to a "feature" in pkgconfig, it leaks out private dependencies.
# Thus we will not add them to the pc file unless the target
# we are processing is a static library.
#
# This way (hopefully) "pkgconfig --libs --static foobar" works
# and "pkgconfig --cflags/--libs foobar" does not have any trace
# of dependencies that the build file creator has not explicitly
# added to the dependency list.
processed_libs.append(obj)
if public:
if not hasattr(obj, 'generated_pc'):
Copy link
Contributor

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)

Copy link
Member Author

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.

Copy link
Contributor

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.

Expand Down
44 changes: 41 additions & 3 deletions run_unittests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious, why is this needed?

Copy link
Contributor

@textshell textshell Mar 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.environ is a dict like object but not actually a dict. setUp and tearDown previously tried to restore the state of os.environ by assigning os.environ.copy() to os.environ. But that is a normal dict and thus after that os.environ no longer has effect on processes started with subprocess.

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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
'''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pkgg = import('pkgconfig')
# libmain internally use libinternal and expose libexpose in its API
exposed_lib = shared_library('libexposed', 'exposed.c')
internal_lib = shared_library('libinternal', 'internal.c')
main_lib = shared_library('libmain', link_with : [exposed_lib, internal_lib])
main_lib = static_library('libmain', link_with : [exposed_lib, internal_lib])

pkgg.generate(libraries : exposed_lib,
version : '1.0',
Expand Down
7 changes: 7 additions & 0 deletions test cases/unit/24 pkgconfig usage/dependee/meson.build
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)

6 changes: 6 additions & 0 deletions test cases/unit/24 pkgconfig usage/dependee/pkguser.c
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;
}
24 changes: 24 additions & 0 deletions test cases/unit/24 pkgconfig usage/dependency/meson.build
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)
7 changes: 7 additions & 0 deletions test cases/unit/24 pkgconfig usage/dependency/pkgdep.c
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();
}
3 changes: 3 additions & 0 deletions test cases/unit/24 pkgconfig usage/dependency/pkgdep.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#pragma once

int pkgdep();
3 changes: 3 additions & 0 deletions test cases/unit/24 pkgconfig usage/dependency/privatelib.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
int internal_thingy() {
return 99;
}