Skip to content

Use pkg-config to find libxml2 and libcurl on Linux [SR-5603] #1152

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

Merged
merged 1 commit into from
Aug 12, 2017
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
44 changes: 32 additions & 12 deletions build.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

swift_cflags = ['-DDEPLOYMENT_RUNTIME_SWIFT']
if Configuration.current.target.sdk == OSType.Linux:
foundation.CFLAGS = '`${PKG_CONFIG} icu-uc icu-i18n --cflags-only-I` -DDEPLOYMENT_TARGET_LINUX -D_GNU_SOURCE -DCF_CHARACTERSET_DATA_DIR="CoreFoundation/CharacterSets"'
foundation.LDFLAGS = '${SWIFT_USE_LINKER} -Wl,@./CoreFoundation/linux.ld -lswiftGlibc `${PKG_CONFIG} icu-uc icu-i18n --libs` -Wl,-Bsymbolic '
foundation.CFLAGS = '-DDEPLOYMENT_TARGET_LINUX -D_GNU_SOURCE -DCF_CHARACTERSET_DATA_DIR="CoreFoundation/CharacterSets"'
foundation.LDFLAGS = '${SWIFT_USE_LINKER} -Wl,@./CoreFoundation/linux.ld -lswiftGlibc -Wl,-Bsymbolic '
Configuration.current.requires_pkg_config = True
elif Configuration.current.target.sdk == OSType.FreeBSD:
foundation.CFLAGS = '-DDEPLOYMENT_TARGET_FREEBSD -I/usr/local/include -I/usr/local/include/libxml2 -I/usr/local/include/curl '
Expand Down Expand Up @@ -61,35 +61,55 @@
'-Wno-unused-variable',
'-Wno-int-conversion',
'-Wno-unused-function',
'-I${SYSROOT}/usr/include/libxml2',
'-I${SYSROOT}/usr/include/curl',
'-I./',
])

swift_cflags += [
'-I${BUILD_DIR}/Foundation/usr/lib/swift',
'-I${SYSROOT}/usr/include/libxml2',
'-I${SYSROOT}/usr/include/curl'
]

if "XCTEST_BUILD_DIR" in Configuration.current.variables:
swift_cflags += [
'-I${XCTEST_BUILD_DIR}',
'-L${XCTEST_BUILD_DIR}',
'-I${SYSROOT}/usr/include/libxml2',
'-I${SYSROOT}/usr/include/curl'
]

triple = Configuration.current.target.triple
if triple.find("linux") != -1:
foundation.LDFLAGS += '-lcurl '
if Configuration.current.requires_pkg_config:
pkg_config_dependencies = [
'icu-i18n',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These should probably specify a minimum package version (probably from Ubuntu 14.04 as it's the oldest supported target), WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no harm in doing that, I guess. Currently we link against the system libicu which has plusses and minuses.

'icu-uc',
'libcurl',
'libxml-2.0',
]
for package_name in pkg_config_dependencies:
try:
package = PkgConfig(package_name)
except PkgConfig.Error as e:
sys.exit("pkg-config error for package {}: {}".format(package_name, e))
foundation.CFLAGS += ' {} '.format(' '.join(package.cflags))
foundation.LDFLAGS += ' {} '.format(' '.join(package.ldflags))
swift_cflags += package.swiftc_flags
else:
foundation.CFLAGS += ''.join([
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't these get added via package.cflags on line 89 above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the else block of Configuration.current.requires_pkg_config, so these are hardcoded guess paths (moved from being unconditionally included on line 64-65 and other places). When we have pkg-config there's no reason to include them as they'll be included automatically by pkg-config <package_name> --libs and pkg-config <package_name> --cflags respectively. More importantly, if the PKG_CONFIG_PATH environment variable points to somewhere other than the system libraries and headers for these packages we won't consider them and will instead use the intended paths.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks.

'-I${SYSROOT}/usr/include/curl ',
Copy link
Contributor Author

@kevints kevints Aug 1, 2017

Choose a reason for hiding this comment

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

I mechanically moved this from the previous unconditional block; however I'm not sure it ever did anything given that CF headers include <curl/curl.h> and this directory appears to contain curl.h and friends, without subdirectories. It also seems to have been repeated at least 4 times - I've condensed 3 of those into this line but I'm unsure about the 4th in the FreeBSD config (the pkg-plist seems to suggest that it wouldn't be needed there either but I don't have a system to test on).

'-I${SYSROOT}/usr/include/libxml2 ',
])
foundation.LDFLAGS += ''.join([
'-lcurl ',
'-lxml2 ',
])
swift_cflags += ''.join([
'-I${SYSROOT}/usr/include/curl ',
'-I${SYSROOT}/usr/include/libxml2 ',
])

triple = Configuration.current.target.triple
if triple == "armv7-none-linux-androideabi":
foundation.LDFLAGS += '-llog '
else:
foundation.LDFLAGS += '-lpthread '

foundation.LDFLAGS += '-ldl -lm -lswiftCore -lxml2 '
foundation.LDFLAGS += '-ldl -lm -lswiftCore '
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to remove all of these flags? -lswiftCore and -ldl should be added by the swift compiler and I am also pretty sure that the libswiftCore already uses some math functions so -lm is probably getting added somewhere lower down if needed. I dont think -ldl is needed on FreeBSD anyway so a cleanup of these flags would be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the flags that get passed to the build for the CoreFoundation C library, which includes <math.h> and <dlfcn.h> so I believe -lm and -ldl should remain here in the spirit of include-what-you-use. I'm not sure about -lswiftCore as CoreFoundation doesn't appear to be importing any swift headers and swiftc should indeed be passing that flag, but maybe there is some magic I don't understand. I'll file a bug to follow up as I'd like to keep this change focused on the PR title.

Copy link
Contributor

Choose a reason for hiding this comment

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

-lswiftCore is actually added by the compiler for any swift program that is compiled. -ldl is a dependancy of libswiftCore.so so it gets picked up there. Math functions are also used by the stdlib and I think -lm is auto added by the C compiler (the same a -lc) since it is dependant on how a given C library is structured, ie if certain functions are broken out into other libraries


# Configure use of Dispatch in CoreFoundation and Foundation if libdispatch is being built
if "LIBDISPATCH_SOURCE_DIR" in Configuration.current.variables:
Expand Down
2 changes: 2 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ from lib.product import StaticAndDynamicLibrary
from lib.product import Application
from lib.product import Executable

from lib.pkg_config import PkgConfig

from lib.script import Script

from lib.target import ArchSubType
Expand Down
3 changes: 2 additions & 1 deletion lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@
"config",
"path",
"phases",
"pkg_config",
"product",
"script",
"target",
"workspace",
]
]
65 changes: 65 additions & 0 deletions lib/pkg_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
from __future__ import print_function

import shlex
import subprocess
import sys

from .config import Configuration


class PkgConfig(object):
class Error(Exception):
"""Raised when information could not be obtained from pkg-config."""

def __init__(self, package_name):
"""Query pkg-config for information about a package.

:type package_name: str
:param package_name: The name of the package to query.
:raises PkgConfig.Error: When a call to pkg-config fails.
"""
self.package_name = package_name
self._cflags = self._call("--cflags")
self._cflags_only_I = self._call("--cflags-only-I")
self._cflags_only_other = self._call("--cflags-only-other")
self._libs = self._call("--libs")
self._libs_only_l = self._call("--libs-only-l")
self._libs_only_L = self._call("--libs-only-L")
self._libs_only_other = self._call("--libs-only-other")

def _call(self, *pkg_config_args):
try:
cmd = [Configuration.current.pkg_config] + list(pkg_config_args) + [self.package_name]
print("Executing command '{}'".format(cmd), file=sys.stderr)
return shlex.split(subprocess.check_output(cmd))
except subprocess.CalledProcessError as e:
raise self.Error("pkg-config exited with error code {}".format(e.returncode))

@property
def swiftc_flags(self):
"""Flags for this package in a format suitable for passing to `swiftc`.

:rtype: list[str]
"""
return (
["-Xcc {}".format(s) for s in self._cflags_only_other]
+ ["-Xlinker {}".format(s) for s in self._libs_only_other]
+ self._cflags_only_I
+ self._libs_only_L
+ self._libs_only_l)

@property
def cflags(self):
"""CFLAGS for this package.

:rtype: list[str]
"""
return self._cflags

@property
def ldflags(self):
"""LDFLAGS for this package.

:rtype: list[str]
"""
return self._libs