Skip to content

Commit

Permalink
Fix how we choose the compiler to use in the third-party build
Browse files Browse the repository at this point in the history
Summary:
In this diff I am removing the use of the compiler-wrapper.sh script
when building third-party dependencies from the yugabyte-db source tree,
while also allowing the Linuxbrew path detection logic in
common-build-env.sh to pick the default Linuxbrew location without
relying on BUILD_ROOT being set when we are building third-party
dependencies.

Also back-porting other the relevant changes from
https://github.com/yugabyte/yugabyte-db-thirdparty. Soon the thirdparty
directory in the main https://github.com/yugabyte/yugabyte-db repository
will be replaced with a submodule pointing to
https://github.com/yugabyte/yugabyte-db-thirdparty, and this is a step
towards bringing the third-party building scripts in both repositories
more in sync.

Here is a command that I've used to reconcile and reduce the set of
differences between yugabyte-db/thirdparty and yugabyte-db-thirdparty
codebases:

  diff -r \
      '--exclude=.git' \
      '--exclude=installed' \
      '--exclude=build' \
      '--exclude=src' \
      '--exclude=.gitignore' \
      '--exclude=*.pyc' \
      '--exclude=*.sw?' --ignore-space-change \
      ~/code/yugabyte-db-thirdparty \
      ~/yugabyte-db/thirdparty

Even with this diff applied the above command reports some differences,
but those will be reconciled as part of replacing the thirdparty
directory with a submodule.

This diff also includes a fix for post_install.sh: when building with
"downloadable" third-party dependencies and Linuxbrew that are
installed in /opt/yb-build/..., we need to expand the Linuxbrew path
using `os.path.realpath` before including it into the generated
post_install.sh script.

Test Plan:
Jenkins: compile only

- On a CentOS dev server:
  `./yb_build.sh tsan --clean --clean-thirdparty --no-nfs-shared-thirdparty`
- On a macOS laptop: `./yb_build.sh --clean --clean-thirdparty`

Reviewers: dmitry, sergei

Reviewed By: sergei

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D7851
  • Loading branch information
mbautin committed Jan 29, 2020
1 parent e937af4 commit 44f2aee
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 65 deletions.
9 changes: 6 additions & 3 deletions build-support/common-build-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,9 @@ popd() {
# Creates files such as thirdparty_url.txt, thirdparty_path.txt, linuxbrew_path.txt in the build
# directory. This is only being done if the file does not exist.
save_var_to_file_in_build_dir() {
if [[ ${YB_IS_BUILD_THIRDPARTY_SCRIPT:-0} == "1" ]]; then
return
fi
expect_num_args 2 "$@"
local value=$1
if [[ -z ${value:-} ]]; then
Expand Down Expand Up @@ -1203,7 +1206,6 @@ wait_for_directory_existence() {
fatal "Gave up waiting for directory '$dir_path' to appear after $attempt seconds"
fi
log "Directory '$dir_path' not found, waiting for it to mount"
( set +e; ls "$dir_path"/* >/dev/null )
let attempt+=1
sleep 1
done
Expand All @@ -1229,8 +1231,9 @@ save_paths_to_build_dir() {
}

detect_linuxbrew() {
if [[ -z ${BUILD_ROOT:-} ]]; then
fatal "BUILD_ROOT is not set"
if [[ ${YB_IS_BUILD_THIRDPARTY_SCRIPT:-0} == "0" && -z ${BUILD_ROOT:-} ]]; then
fatal "BUILD_ROOT is not set, and we are not building third-party dependencies, not trying" \
"to use the default version of Linuxbrew."
fi
if ! is_linux; then
fatal "Expected this function to only be called on Linux"
Expand Down
9 changes: 6 additions & 3 deletions build-support/post_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ find "$lib_dir" "$linuxbrew_dir" -name "*.so*" ! -name "ld.so*" -exec "$patchelf
>(grep -v 'warning: working around a Linux kernel bug by creating a hole' >&2) \;

ORIG_BREW_HOME=${original_linuxbrew_path_to_patch}
ORIG_LEN=${#ORIG_BREW_HOME}
ORIG_LEN=${original_linuxbrew_path_length}

# Take $ORIG_LEN number of '\0' from /dev/zero, replace '\0' with 'x', then prepend to
# "$distribution_dir/linuxbrew-" and keep first $ORIG_LEN symbols, so we have a path of $ORIG_LEN
Expand All @@ -95,7 +95,10 @@ fi

ln -sfT "$linuxbrew_dir" "$BREW_HOME"

find "$distribution_dir" -type f -not -path "$distribution_dir/yugabyte-logs/*" \
-exec sed -i --binary "s%$ORIG_BREW_HOME%$BREW_HOME%g" {} \;
find "$distribution_dir" \( \
-type f -and \
-not -path "$distribution_dir/yugabyte-logs/*" -and \
-not -name "post_install.sh" \
\) -exec sed -i --binary "s%$ORIG_BREW_HOME%$BREW_HOME%g" {} \;

touch "$completion_file"
7 changes: 5 additions & 2 deletions python/yb/library_packager.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,10 @@ def package_binaries(self):
post_install_script = post_install_script_input.read()

new_post_install_script = post_install_script
replacements = [("original_linuxbrew_path_to_patch", linuxbrew_home.linuxbrew_dir)]
replacements = [
("original_linuxbrew_path_to_patch", linuxbrew_home.linuxbrew_dir),
("original_linuxbrew_path_length", len(linuxbrew_home.linuxbrew_dir)),
]
for macro_var_name, list_of_binary_names in [
("main_elf_names_to_patch", main_elf_names_to_patch),
("postgres_elf_names_to_patch", postgres_elf_names_to_patch),
Expand All @@ -448,7 +451,7 @@ def package_binaries(self):

for macro_var_name, value in replacements:
new_post_install_script = new_post_install_script.replace(
'${%s}' % macro_var_name, value)
'${%s}' % macro_var_name, str(value))

with open(post_install_path, 'w') as post_install_script_output:
post_install_script_output.write(new_post_install_script)
Expand Down
2 changes: 1 addition & 1 deletion python/yb/linuxbrew.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def __init__(self, build_root=None):
find_script_result.program_path,
linuxbrew_dir,
find_script_result))
self.linuxbrew_dir = os.path.abspath(linuxbrew_dir)
self.linuxbrew_dir = os.path.realpath(linuxbrew_dir)

# Directories derived from the Linuxbrew top-level one.
self.linuxbrew_link_target = os.path.realpath(linuxbrew_dir)
Expand Down
1 change: 1 addition & 0 deletions src/yb/util/delayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <condition_variable>
#include <deque>
#include <functional>
#include <mutex>
#include <thread>

Expand Down
2 changes: 1 addition & 1 deletion thirdparty/README.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
This directory contains scripts which download and install several third-party
dependencies of YugaByte. Most of these dependencies are statically linked into
dependencies of YugaByte. Most of these dependencies are dynamically linked into
YugaByte binaries, though a few are used only at build-time.

See LICENSE.txt in this file for information on the licensing of each of these
Expand Down
9 changes: 9 additions & 0 deletions thirdparty/build_definitions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,15 @@ def __init__(self, name, version, url_pattern, build_group):
self.archive_name = make_archive_name(name, version, self.download_url)
self.patch_version = 0

def get_additional_c_cxx_flags(self, builder):
return []

def get_additional_c_flags(self, builder):
return []

def get_additional_cxx_flags(self, builder):
return []

def should_build(self, builder):
return True

Expand Down
3 changes: 2 additions & 1 deletion thirdparty/build_definitions/bitshuffle.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ def __init__(self):

def build(self, builder):
log_prefix = builder.log_prefix(self)
compile_command = [builder.cc_wrapper, '-std=c99', '-I{}/include'.format(builder.prefix),
compile_command = [builder.get_c_compiler(), '-std=c99',
'-I{}/include'.format(builder.prefix),
'-O3', '-DNDEBUG', '-fPIC', '-c',
'src/bitshuffle_core.c', 'src/bitshuffle.c',
'src/iochain.c']
Expand Down
2 changes: 1 addition & 1 deletion thirdparty/build_definitions/boost.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def build(self, builder):
out.write(PROJECT_CONFIG.format(
compiler_type,
compiler_version,
builder.cxx_wrapper,
builder.get_cxx_compiler(),
' '.join(['<compileflags>' + flag for flag in cxx_flags]),
' '.join(['<linkflags>' + flag for flag in cxx_flags + builder.ld_flags]),
' '.join(['--with-{}'.format(lib) for lib in libs])))
Expand Down
9 changes: 9 additions & 0 deletions thirdparty/build_definitions/crcutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ def __init__(self):
self.patch_strip = 0
self.patches = ['crcutil-fix-libtoolize-on-osx.patch', 'crcutil-fix-offsetof.patch']

def get_additional_c_cxx_flags(self, builder):
if builder.building_with_clang():
return []
# -mcrc32 (https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html)
# This option enables built-in functions __builtin_ia32_crc32qi, __builtin_ia32_crc32hi,
# __builtin_ia32_crc32si and __builtin_ia32_crc32di to generate the crc32 machine
# instruction.
return ['-mcrc32']

def build(self, builder):
log_prefix = builder.log_prefix(self)
log_output(log_prefix, ['./autogen.sh'])
Expand Down
13 changes: 5 additions & 8 deletions thirdparty/build_definitions/llvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ def __init__(self):
self.copy_sources = False

def build(self, builder):
prefix = builder.get_prefix('llvm7')

# The LLVM build can fail if a different version is already installed
# in the install prefix. It will try to link against that version instead
# of the one being built.
subprocess.check_call(
"rm -Rf {0}/include/{{llvm*,clang*}} {0}/lib/lib{{LLVM,LTO,clang}}* {0}/lib/clang/ "
"{0}/lib/cmake/{{llvm,clang}}".format(builder.prefix), shell=True)
"{0}/lib/cmake/{{llvm,clang}}".format(prefix), shell=True)

python_executable = which('python')
if not os.path.exists(python_executable):
Expand All @@ -61,7 +63,7 @@ def build(self, builder):

builder.build_with_cmake(self,
['-DCMAKE_BUILD_TYPE=Release',
'-DCMAKE_INSTALL_PREFIX={}'.format(builder.prefix),
'-DCMAKE_INSTALL_PREFIX={}'.format(prefix),
'-DLLVM_INCLUDE_DOCS=OFF',
'-DLLVM_INCLUDE_EXAMPLES=OFF',
'-DLLVM_INCLUDE_TESTS=OFF',
Expand All @@ -74,14 +76,9 @@ def build(self, builder):
],
use_ninja='auto')

# Create a link from Clang to thirdparty/clang-toolchain. This path is used
# for all Clang invocations. The link can't point to the Clang installed in
# the prefix directory, since this confuses CMake into believing the
# thirdparty prefix directory is the system-wide prefix, and it omits the
# thirdparty prefix directory from the rpath of built binaries.
link_path = os.path.join(builder.tp_dir, 'clang-toolchain')
remove_path(link_path)
list_dest = os.path.relpath(os.getcwd(), builder.tp_dir)
list_dest = os.path.relpath(prefix, builder.tp_dir)
log("Link {} => {}".format(link_path, list_dest))
os.symlink(list_dest, link_path)

Expand Down
2 changes: 1 addition & 1 deletion thirdparty/build_definitions/squeasel.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def __init__(self):

def build(self, builder):
log_prefix = builder.log_prefix(self)
compile_command = [builder.cc_wrapper, '-std=c99', '-O3', '-DNDEBUG', '-fPIC', '-c',
compile_command = [builder.get_c_compiler(), '-std=c99', '-O3', '-DNDEBUG', '-fPIC', '-c',
'squeasel.c']
compile_command += builder.compiler_flags + builder.c_flags
log_output(log_prefix, compile_command)
Expand Down
1 change: 1 addition & 0 deletions thirdparty/build_thirdparty.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
set -euo pipefail
. "${BASH_SOURCE%/*}/../build-support/common-build-env.sh"

export YB_IS_BUILD_THIRDPARTY_SCRIPT=1
activate_virtualenv
export PYTHONPATH=$YB_SRC_ROOT/python:${PYTHONPATH:-}
detect_brew
Expand Down
Loading

0 comments on commit 44f2aee

Please sign in to comment.