From 76198edc790de8e8514bddaa3895d1145fccd6aa Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Tue, 1 Jun 2021 13:59:16 -0700 Subject: [PATCH] Fixed dangling symlinks in builds (#656) --- examples/third_party/BUILD.bazel | 3 ++ .../third_party/libjpeg_turbo/BUILD.bazel | 16 +++++++++ .../libjpeg_turbo/BUILD.libjpeg_turbo.bazel | 27 ++++++++++++++ .../libjpeg_turbo_repositories.bzl | 14 ++++++++ examples/third_party/libssh2/BUILD.bazel | 1 - examples/third_party/repositories.bzl | 2 ++ foreign_cc/built_tools/ninja_build.bzl | 2 +- foreign_cc/private/framework.bzl | 8 +++++ .../private/framework/toolchains/commands.bzl | 12 +++++++ .../framework/toolchains/linux_commands.bzl | 28 +++++++++++++-- .../framework/toolchains/macos_commands.bzl | 36 ++++++++++++++++--- .../framework/toolchains/windows_commands.bzl | 26 +++++++++++++- test/expected/inner_fun_text.txt | 20 +++++++++-- test/expected/inner_fun_text_macos.txt | 20 +++++++++-- 14 files changed, 202 insertions(+), 13 deletions(-) create mode 100644 examples/third_party/libjpeg_turbo/BUILD.bazel create mode 100644 examples/third_party/libjpeg_turbo/BUILD.libjpeg_turbo.bazel create mode 100644 examples/third_party/libjpeg_turbo/libjpeg_turbo_repositories.bzl diff --git a/examples/third_party/BUILD.bazel b/examples/third_party/BUILD.bazel index 5d48036e..7fb1b699 100644 --- a/examples/third_party/BUILD.bazel +++ b/examples/third_party/BUILD.bazel @@ -10,6 +10,7 @@ test_suite( "//gn:gn_launch_test", "//gperftools:test", "//libgit2:libgit2_build_test", + "//libjpeg_turbo:libjpeg_turbo_build_test", "//libpng:test_libpng", "//libssh2:libssh2_build_test", "//openssl:openssl_build_test", @@ -29,6 +30,7 @@ test_suite( # "//gn:gn_launch_test", "//gperftools:test", "//libgit2:libgit2_build_test", + "//libjpeg_turbo:libjpeg_turbo_build_test", "//libpng:test_libpng", "//libssh2:libssh2_build_test", "//openssl:openssl_build_test", @@ -46,6 +48,7 @@ test_suite( "//gperftools:test", "//iconv:iconv_build_test", "//libgit2:libgit2_build_test", + "//libjpeg_turbo:libjpeg_turbo_build_test", "//libpng:test_libpng", "//libssh2:libssh2_build_test", "//openssl:openssl_build_test", diff --git a/examples/third_party/libjpeg_turbo/BUILD.bazel b/examples/third_party/libjpeg_turbo/BUILD.bazel new file mode 100644 index 00000000..6d187340 --- /dev/null +++ b/examples/third_party/libjpeg_turbo/BUILD.bazel @@ -0,0 +1,16 @@ +load("@bazel_skylib//rules:build_test.bzl", "build_test") + +exports_files( + [ + "BUILD.libjpeg_turbo.bazel", + ], + visibility = ["//visibility:public"], +) + +build_test( + name = "libjpeg_turbo_build_test", + targets = [ + "@libjpeg_turbo//:libjpeg_turbo", + ], + visibility = ["//:__pkg__"], +) diff --git a/examples/third_party/libjpeg_turbo/BUILD.libjpeg_turbo.bazel b/examples/third_party/libjpeg_turbo/BUILD.libjpeg_turbo.bazel new file mode 100644 index 00000000..933faa16 --- /dev/null +++ b/examples/third_party/libjpeg_turbo/BUILD.libjpeg_turbo.bazel @@ -0,0 +1,27 @@ +load("@rules_foreign_cc//foreign_cc:defs.bzl", "cmake") + +filegroup( + name = "all_files", + srcs = glob(["**"]), +) + +cmake( + name = "libjpeg_turbo", + cache_entries = { + "CMAKE_BUILD_TYPE": "Release", + "WITH_JAVA": "0", + }, + lib_source = ":all_files", + out_shared_libs = select({ + "@platforms//os:linux": [ + "libjpeg.so", + ], + "@platforms//os:macos": [ + "libturbojpeg.dylib", + ], + "@platforms//os:windows": [ + "libjpeg.dll", + ], + }), + visibility = ["//visibility:public"], +) diff --git a/examples/third_party/libjpeg_turbo/libjpeg_turbo_repositories.bzl b/examples/third_party/libjpeg_turbo/libjpeg_turbo_repositories.bzl new file mode 100644 index 00000000..69dad605 --- /dev/null +++ b/examples/third_party/libjpeg_turbo/libjpeg_turbo_repositories.bzl @@ -0,0 +1,14 @@ +"""A module defining the third party dependency libjpeg_turbo""" + +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") + +def libjpeg_turbo_repositories(): + maybe( + http_archive, + name = "libjpeg_turbo", + sha256 = "6a965adb02ad898b2ae48214244618fe342baea79db97157fdc70d8844ac6f09", + strip_prefix = "libjpeg-turbo-2.0.90", + url = "https://github.com/libjpeg-turbo/libjpeg-turbo/archive/2.0.90.tar.gz", + build_file = Label("//libjpeg_turbo:BUILD.libjpeg_turbo.bazel"), + ) diff --git a/examples/third_party/libssh2/BUILD.bazel b/examples/third_party/libssh2/BUILD.bazel index 6ab21f0b..2b08209b 100644 --- a/examples/third_party/libssh2/BUILD.bazel +++ b/examples/third_party/libssh2/BUILD.bazel @@ -3,7 +3,6 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") exports_files( [ "BUILD.libssh2.bazel", - "WORKSPACE.libssh2.bazel", ], visibility = ["//visibility:public"], ) diff --git a/examples/third_party/repositories.bzl b/examples/third_party/repositories.bzl index 8fc79a1d..ffac81dc 100644 --- a/examples/third_party/repositories.bzl +++ b/examples/third_party/repositories.bzl @@ -9,6 +9,7 @@ load("//gn:gn_repositories.bzl", "gn_repositories") load("//gperftools:gperftools_repositories.bzl", "gperftools_repositories") load("//iconv:iconv_repositories.bzl", "iconv_repositories") load("//libgit2:libgit2_repositories.bzl", "libgit2_repositories") +load("//libjpeg_turbo:libjpeg_turbo_repositories.bzl", "libjpeg_turbo_repositories") load("//libpng:libpng_repositories.bzl", "libpng_repositories") load("//libssh2:libssh2_repositories.bzl", "libssh2_repositories") load("//openssl:openssl_repositories.bzl", "openssl_repositories") @@ -25,6 +26,7 @@ def repositories(): gperftools_repositories() iconv_repositories() libgit2_repositories() + libjpeg_turbo_repositories() libpng_repositories() libssh2_repositories() openssl_repositories() diff --git a/foreign_cc/built_tools/ninja_build.bzl b/foreign_cc/built_tools/ninja_build.bzl index db121709..d65271dd 100644 --- a/foreign_cc/built_tools/ninja_build.bzl +++ b/foreign_cc/built_tools/ninja_build.bzl @@ -12,7 +12,7 @@ def _ninja_tool_impl(ctx): script = [ "./configure.py --bootstrap", "mkdir $$INSTALLDIR$$/bin", - "cp -a ./ninja{} $$INSTALLDIR$$/bin/".format( + "cp -p ./ninja{} $$INSTALLDIR$$/bin/".format( ".exe" if "win" in os_name(ctx) else "", ), ] diff --git a/foreign_cc/private/framework.bzl b/foreign_cc/private/framework.bzl index aba00717..bdbf2f56 100644 --- a/foreign_cc/private/framework.bzl +++ b/foreign_cc/private/framework.bzl @@ -385,11 +385,19 @@ def cc_external_rule_impl(ctx, attrs): "##replace_absolute_paths## $$INSTALLDIR$$ $$EXT_BUILD_DEPS$$", installdir_copy.script, "cd $$EXT_BUILD_ROOT$$", + ] + [ + "##replace_symlink## {}".format(file.path) + for file in ( + outputs.libraries.static_libraries + + outputs.libraries.shared_libraries + + outputs.libraries.interface_libraries + ) ] script_text = "\n".join([ shebang(ctx), convert_shell_script(ctx, script_lines), + "", ]) wrapped_outputs = wrap_outputs(ctx, lib_name, attrs.configure_name, script_text) diff --git a/foreign_cc/private/framework/toolchains/commands.bzl b/foreign_cc/private/framework/toolchains/commands.bzl index 691a5c67..f5909219 100644 --- a/foreign_cc/private/framework/toolchains/commands.bzl +++ b/foreign_cc/private/framework/toolchains/commands.bzl @@ -177,6 +177,18 @@ PLATFORM_COMMANDS = { ], doc = "Replaces all occurrences of 'from_' to 'to_' recursively in the directory 'dir'.", ), + "replace_symlink": _command_info( + arguments = [ + _argument_info( + name = "file", + data_type = type(""), + doc = "Target file", + ), + ], + doc = ( + "Replace the target symlink with resolved file it points to if `file` is a symlink" + ), + ), "script_extension": _command_info( arguments = [], doc = "Return the extension for the current set of commands (`.sh` for bash, `.ps1` for powershell)", diff --git a/foreign_cc/private/framework/toolchains/linux_commands.bzl b/foreign_cc/private/framework/toolchains/linux_commands.bzl index a59ca57c..77002fa5 100644 --- a/foreign_cc/private/framework/toolchains/linux_commands.bzl +++ b/foreign_cc/private/framework/toolchains/linux_commands.bzl @@ -88,6 +88,14 @@ def copy_dir_contents_to_dir(source, target): def symlink_contents_to_dir(source, target): text = """\ +if [[ -z "$1" ]]; then + echo "arg 1 to symlink_contents_to_dir is unexpectedly empty" + exit 1 +fi +if [[ -z "$2" ]]; then + echo "arg 2 to symlink_contents_to_dir is unexpectedly empty" + exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -109,6 +117,14 @@ fi def symlink_to_dir(source, target): text = """\ +if [[ -z "$1" ]]; then + echo "arg 1 to symlink_to_dir is unexpectedly empty" + exit 1 +fi +if [[ -z "$2" ]]; then + echo "arg 2 to symlink_to_dir is unexpectedly empty" + exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -121,7 +137,7 @@ if [[ -f "$1" ]]; then ln -s -f -t "$target" "$1" fi elif [[ -L "$1" && ! -d "$1" ]]; then - cp -a "$1" "$2" + cp -pR "$1" "$2" elif [[ -d "$1" ]]; then SAVEIFS=$IFS IFS=$'\n' @@ -130,7 +146,7 @@ elif [[ -d "$1" ]]; then local dirname=$(basename "$1") mkdir -p "$target/$dirname" for child in "${children[@]:-}"; do - if [[ "$dirname" != *.ext_build_deps ]]; then + if [[ -n "$child" && "$dirname" != *.ext_build_deps ]]; then ##symlink_to_dir## "$child" "$target/$dirname" fi done @@ -200,3 +216,11 @@ def replace_absolute_paths(dir_, abs_path): REPLACE_VALUE = _REPLACE_VALUE, abs_path = abs_path, ) + +def replace_symlink(file): + return """\ +if [[ -L "{file}" ]]; then + target="$(readlink -f "{file}")" + rm "{file}" && cp -a "${{target}}" "{file}" +fi +""".format(file = file) diff --git a/foreign_cc/private/framework/toolchains/macos_commands.bzl b/foreign_cc/private/framework/toolchains/macos_commands.bzl index 9b4198ab..8ae3fd79 100644 --- a/foreign_cc/private/framework/toolchains/macos_commands.bzl +++ b/foreign_cc/private/framework/toolchains/macos_commands.bzl @@ -91,11 +91,11 @@ local target="$2" mkdir -p "${target}" for child in "${children[@]:-}"; do if [[ -f "$child" ]]; then - cp -a "$child" "$target" + cp -pR "$child" "$target" elif [[ -L "$child" ]]; then local actual=$(readlink "$child") if [[ -f "$actual" ]]; then - cp -a "$actual" "$target" + cp -pR "$actual" "$target" else local dirn=$(basename "$actual") mkdir -p "$target/$dirn" @@ -112,6 +112,14 @@ done def symlink_contents_to_dir(source, target): text = """\ +if [[ -z "$1" ]]; then + echo "arg 1 to symlink_contents_to_dir is unexpectedly empty" + exit 1 +fi +if [[ -z "$2" ]]; then + echo "arg 2 to symlink_contents_to_dir is unexpectedly empty" + exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -133,6 +141,14 @@ fi def symlink_to_dir(source, target): text = """\ +if [[ -z "$1" ]]; then + echo "arg 1 to symlink_to_dir is unexpectedly empty" + exit 1 +fi +if [[ -z "$2" ]]; then + echo "arg 2 to symlink_to_dir is unexpectedly empty" + exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -145,7 +161,7 @@ if [[ -f "$1" ]]; then ln -s -f "$1" "$target" fi elif [[ -L "$1" && ! -d "$1" ]]; then - cp -a "$1" "$2" + cp -pR "$1" "$2" elif [[ -d "$1" ]]; then SAVEIFS=$IFS IFS=$'\n' @@ -154,7 +170,7 @@ elif [[ -d "$1" ]]; then local dirname=$(basename "$1") mkdir -p "$target/$dirname" for child in "${children[@]:-}"; do - if [[ "$dirname" != *.ext_build_deps ]]; then + if [[ -n "$child" && "$dirname" != *.ext_build_deps ]]; then ##symlink_to_dir## "$child" "$target/$dirname" fi done @@ -224,3 +240,15 @@ def replace_absolute_paths(dir_, abs_path): REPLACE_VALUE = _REPLACE_VALUE, abs_path = abs_path, ) + +def replace_symlink(file): + # On macos, the `readlink` binary doesn't have a `-f` argument like the linux + # equivilant. As a result, we need another way to fully resolve chaining symlinks. + # Python is used to do this as it's expected to be available on all systems, just + # as `readlink` is. + return """\ +if [[ -L "{file}" ]]; then + target="$(python -c "import os; print(os.path.realpath('{file}'))")" + rm "{file}" && cp -a "${{target}}" "{file}" +fi +""".format(file = file) diff --git a/foreign_cc/private/framework/toolchains/windows_commands.bzl b/foreign_cc/private/framework/toolchains/windows_commands.bzl index 4b6ed4c6..b653a084 100644 --- a/foreign_cc/private/framework/toolchains/windows_commands.bzl +++ b/foreign_cc/private/framework/toolchains/windows_commands.bzl @@ -89,6 +89,14 @@ def copy_dir_contents_to_dir(source, target): def symlink_contents_to_dir(source, target): text = """\ +if [[ -z "$1" ]]; then + echo "arg 1 to symlink_contents_to_dir is unexpectedly empty" + exit 1 +fi +if [[ -z "$2" ]]; then + echo "arg 2 to symlink_contents_to_dir is unexpectedly empty" + exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -110,6 +118,14 @@ fi def symlink_to_dir(source, target): text = """\ +if [[ -z "$1" ]]; then + echo "arg 1 to symlink_to_dir is unexpectedly empty" + exit 1 +fi +if [[ -z "$2" ]]; then + echo "arg 2 to symlink_to_dir is unexpectedly empty" + exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -131,7 +147,7 @@ elif [[ -d "$1" ]]; then IFS=$SAVEIFS local dirname=$(basename "$1") for child in "${children[@]}"; do - if [[ "$dirname" != *.ext_build_deps ]]; then + if [[ -n "$child" && "$dirname" != *.ext_build_deps ]]; then ##symlink_to_dir## "$child" "$target/$dirname" fi done @@ -211,3 +227,11 @@ def replace_absolute_paths(dir_, abs_path): REPLACE_VALUE = _REPLACE_VALUE, abs_path = abs_path, ) + +def replace_symlink(file): + return """\ +if [[ -L "{file}" ]]; then + target="$(readlink -f "{file}")" + rm "{file}" && cp -a "${{target}}" "{file}" +fi +""".format(file = file) diff --git a/test/expected/inner_fun_text.txt b/test/expected/inner_fun_text.txt index 7806a89c..b46e4fcd 100755 --- a/test/expected/inner_fun_text.txt +++ b/test/expected/inner_fun_text.txt @@ -1,4 +1,12 @@ function symlink_contents_to_dir() { +if [[ -z "$1" ]]; then +echo "arg 1 to symlink_contents_to_dir is unexpectedly empty" +exit 1 +fi +if [[ -z "$2" ]]; then +echo "arg 2 to symlink_contents_to_dir is unexpectedly empty" +exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -18,6 +26,14 @@ done fi } function symlink_to_dir() { +if [[ -z "$1" ]]; then +echo "arg 1 to symlink_to_dir is unexpectedly empty" +exit 1 +fi +if [[ -z "$2" ]]; then +echo "arg 2 to symlink_to_dir is unexpectedly empty" +exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -30,7 +46,7 @@ else ln -s -f -t "$target" "$1" fi elif [[ -L "$1" && ! -d "$1" ]]; then -cp -a "$1" "$2" +cp -pR "$1" "$2" elif [[ -d "$1" ]]; then SAVEIFS=$IFS IFS=$' @@ -40,7 +56,7 @@ IFS=$SAVEIFS local dirname=$(basename "$1") mkdir -p "$target/$dirname" for child in "${children[@]:-}"; do -if [[ "$dirname" != *.ext_build_deps ]]; then +if [[ -n "$child" && "$dirname" != *.ext_build_deps ]]; then symlink_to_dir "$child" "$target/$dirname" fi done diff --git a/test/expected/inner_fun_text_macos.txt b/test/expected/inner_fun_text_macos.txt index 6ee15266..22747a1c 100755 --- a/test/expected/inner_fun_text_macos.txt +++ b/test/expected/inner_fun_text_macos.txt @@ -1,4 +1,12 @@ function symlink_contents_to_dir() { +if [[ -z "$1" ]]; then +echo "arg 1 to symlink_contents_to_dir is unexpectedly empty" +exit 1 +fi +if [[ -z "$2" ]]; then +echo "arg 2 to symlink_contents_to_dir is unexpectedly empty" +exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -18,6 +26,14 @@ done fi } function symlink_to_dir() { +if [[ -z "$1" ]]; then +echo "arg 1 to symlink_to_dir is unexpectedly empty" +exit 1 +fi +if [[ -z "$2" ]]; then +echo "arg 2 to symlink_to_dir is unexpectedly empty" +exit 1 +fi local target="$2" mkdir -p "$target" if [[ -f "$1" ]]; then @@ -30,7 +46,7 @@ else ln -s -f "$1" "$target" fi elif [[ -L "$1" && ! -d "$1" ]]; then -cp -a "$1" "$2" +cp -pR "$1" "$2" elif [[ -d "$1" ]]; then SAVEIFS=$IFS IFS=$' @@ -40,7 +56,7 @@ IFS=$SAVEIFS local dirname=$(basename "$1") mkdir -p "$target/$dirname" for child in "${children[@]:-}"; do -if [[ "$dirname" != *.ext_build_deps ]]; then +if [[ -n "$child" && "$dirname" != *.ext_build_deps ]]; then symlink_to_dir "$child" "$target/$dirname" fi done