From c4886cbf16b16dd0b07a5655a32d68389ea03a31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Tue, 22 Sep 2020 11:48:42 +0000 Subject: [PATCH] Add a module description --- cc/system_library.bzl | 127 ++++++++++++++++++-- tests/system_library/system_library_test.sh | 30 ++++- 2 files changed, 145 insertions(+), 12 deletions(-) diff --git a/cc/system_library.bzl b/cc/system_library.bzl index 16a59f9..09fb186 100644 --- a/cc/system_library.bzl +++ b/cc/system_library.bzl @@ -1,3 +1,108 @@ +"""system_library is a repository rule for importing system libraries + +`system_library` is a repository rule for safely depending on system-provided +libraries on Linux. It can be used with remote caching and remote execution. +Under the hood it uses gcc/clang for finding the library files and headers +and symlinks them into the build directory. Symlinking allows Bazel to take +these files into account when it calculates a checksum of the project. +This prevents cache poisoning from happening. + +Currently `system_library` requires two exeperimental flags: +--experimental_starlark_cc_import +--experimental_repo_remote_exec + +A typical usage looks like this: +WORKSPACE +``` +system_library( + name = "jpeg", + hdrs = [ + "jpeglib.h", + ], + shared_lib_names = ["libjpeg.so, libjpeg.so.62"], + static_lib_names = ["libjpeg.a"], + includes = ["/usr/additional_includes"], + lib_path_hints = ["/usr/additional_libs", "/usr/some/other_path"] + optional_hdrs = [ + "jconfig.h", + "jmorecfg.h", + ], +) +``` + +BUILD +``` +cc_binary( + name = "foo", + srcs = ["foo.cc"], + deps = ["@jpeg"] +) +``` + +foo.cc +``` +#include "jpeglib.h" + +[code using symbols from jpeglib] +``` + +`system_library` requires users to specify at least one header +(as it makes no sense to import a library without headers). +Public headers of a library (i.e. those included in the user-written code, +like `jpeglib.h` in the example above) should be put in `hdrs` param, as they +are required for the library to work. However, some libraries may use more +"private" headers. They should be imported as well, but their names may differ +from system to system. They should be specified in the `optional_hdrs` param. +The build will not fail if some of them are not found, so it's safe to put a +superset there, containing all possible combinations of names for different +versions/distributions. It's up to the user to determine which headers are +required for the library to work. + +One `system_library` target always imports exactly one library. +Users can specify many potential names for the library file, +as these names can differ from system to system. The order of names establishes +the order of preference. As some libraries can be linked both statically +and dynamically, the names of files of each kind can be specified separately. +`system_library` rule will try to find library archives of both kinds, but it's +up to the top-level target (for example, `cc_binary`) to decide which kind of +linking will be used. + +`system_library` rule depends on gcc/clang (whichever is installed) for +finding the actual locations of library archives and headers. +Libraries installed in a standard way by a package manager +(`sudo apt install libjpeg-dev`) are usually placed in one of directories +searched by the compiler/linker by default - on Ubuntu library most archives +are stored in `/usr/lib/x86_64-linux-gnu/` and their headers in +`/usr/include/`. If the maintainer of a project expects the files +to be installed in a non-standard location, they can use the `includes` +parameter to add directories to the search path for headers +and `lib_path_hints` to add directories to the search path for library +archives. + +User building the project can override or extend these search paths by +providing these environment variables to the build: +BAZEL_INCLUDE_ADDITIONAL_PATHS, BAZEL_INCLUDE_OVERRIDE_PATHS, +BAZEL_LIB_ADDITIONAL_PATHS, BAZEL_LIB_OVERRIDE_PATHS. +The syntax for setting the env variables is: +`=,=`. +Users can provide multiple paths for one library by repeating this segment: +`=`. + +So in order to build the example presented above but with custom paths for the +jpeg lib, one would use the following command: + +``` +bazel build //:foo \ + --experimental_starlark_cc_import \ + --experimental_repo_remote_exec \ + --action_env=BAZEL_LIB_OVERRIDE_PATHS=jpeg=/custom/libraries/path \ + --action_env=BAZEL_INCLUDE_OVERRIDE_PATHS=jpeg=/custom/include/path,jpeg=/inc +``` + +Some libraries can depend on other libraries. `system_library` rule provides +a `deps` parameter for specifying such relationships. +""" + BAZEL_LIB_ADDITIONAL_PATHS_ENV_VAR = "BAZEL_LIB_ADDITIONAL_PATHS" BAZEL_LIB_OVERRIDE_PATHS_ENV_VAR = "BAZEL_LIB_OVERRIDE_PATHS" BAZEL_INCLUDE_ADDITIONAL_PATHS_ENV_VAR = "BAZEL_INCLUDE_ADDITIONAL_PATHS" @@ -112,7 +217,7 @@ def _find_header_path(repo_ctx, lib_name, header_name, includes): compiler = _find_compiler(repo_ctx) - # Taken from https://stackoverflow.com/questions/63052707/which-header-exactly-will-c-preprocessor-include/63052918#63052918 + # Taken from https://stackoverflow.com/a/63052918/922404 cmd = """ f=\"{}\"; \\ echo | \\ @@ -133,7 +238,7 @@ def _find_header_path(repo_ctx, lib_name, header_name, includes): ) return _execute_bash(repo_ctx, cmd) -def system_library_impl(repo_ctx): +def _system_library_impl(repo_ctx): repo_name = repo_ctx.attr.name includes = repo_ctx.attr.includes hdrs = repo_ctx.attr.hdrs @@ -322,22 +427,22 @@ alias( ) system_library = repository_rule( - implementation = system_library_impl, + implementation = _system_library_impl, local = True, remotable = True, environ = [ - BAZEL_LIB_ADDITIONAL_PATHS_ENV_VAR, - BAZEL_LIB_OVERRIDE_PATHS_ENV_VAR, BAZEL_INCLUDE_ADDITIONAL_PATHS_ENV_VAR, BAZEL_INCLUDE_OVERRIDE_PATHS_ENV_VAR, + BAZEL_LIB_ADDITIONAL_PATHS_ENV_VAR, + BAZEL_LIB_OVERRIDE_PATHS_ENV_VAR, ], attrs = { - "static_lib_names": attr.string_list(), - "shared_lib_names": attr.string_list(), - "lib_path_hints": attr.string_list(), - "includes": attr.string_list(), - "hdrs": attr.string_list(mandatory = True, allow_empty = False), - "optional_hdrs": attr.string_list(), "deps": attr.string_list(), + "hdrs": attr.string_list(mandatory = True, allow_empty = False), + "includes": attr.string_list(), + "lib_path_hints": attr.string_list(), + "optional_hdrs": attr.string_list(), + "shared_lib_names": attr.string_list(), + "static_lib_names": attr.string_list(), }, ) diff --git a/tests/system_library/system_library_test.sh b/tests/system_library/system_library_test.sh index 9978e2e..018bae7 100755 --- a/tests/system_library/system_library_test.sh +++ b/tests/system_library/system_library_test.sh @@ -162,4 +162,32 @@ EOF || fail "Expected test_static_hardcoded_path to run successfully" } -run_suite "Integration tests for system_library." +function test_system_library_no_lib_names() { + cat << EOF > WORKSPACE +load("//:cc/system_library.bzl", "system_library") +system_library( + name = "foo", + hdrs = [ + "foo.h", + ] +) +EOF + + cat << EOF > BUILD +cc_binary( + name = "test", + srcs = ["test.cc"], + deps = ["@foo"] +) +EOF + + # It should fail when no static_lib_names and static_lib_names are given + bazel run //:test \ + --experimental_starlark_cc_import \ + --experimental_repo_remote_exec \ + &> $TEST_log \ + || true + expect_log "Library foo could not be found" +} + +run_suite "Integration tests for system_library." \ No newline at end of file