From dd2f5f771a04feb703641e65d08766e482afe541 Mon Sep 17 00:00:00 2001 From: Philip Patsch Date: Thu, 22 Nov 2018 02:53:34 +0100 Subject: [PATCH 1/6] specify `provides` for binary/library rules Adds a `provides` field to haskell_library and haskell_binary to tell users about our custom providers (and possibly throw better error messages). As a related change, the `load` targets for `providers.bzl` are made absolute to work around https://github.com/bazelbuild/bazel/issues/3115 --- haskell/c2hs.bzl | 2 +- haskell/cc.bzl | 2 +- haskell/doctest.bzl | 2 +- haskell/haddock.bzl | 2 +- haskell/haskell.bzl | 16 +++++++++++++++- haskell/import.bzl | 2 +- haskell/lint.bzl | 2 +- haskell/private/actions/compile.bzl | 2 +- haskell/private/actions/repl.bzl | 2 +- haskell/private/dependencies.bzl | 2 +- haskell/private/haskell_impl.bzl | 2 +- haskell/protobuf.bzl | 2 +- 12 files changed, 26 insertions(+), 12 deletions(-) diff --git a/haskell/c2hs.bzl b/haskell/c2hs.bzl index a653c1cbc..dfba31657 100644 --- a/haskell/c2hs.bzl +++ b/haskell/c2hs.bzl @@ -8,7 +8,7 @@ load( "target_unique_name", ) load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "C2hsLibraryInfo", ) load("@bazel_skylib//:lib.bzl", "paths") diff --git a/haskell/cc.bzl b/haskell/cc.bzl index 15c9be5f8..b61dea5e0 100644 --- a/haskell/cc.bzl +++ b/haskell/cc.bzl @@ -4,7 +4,7 @@ These rules are temporary and will be deprecated in the future. """ load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "CcSkylarkApiProviderHacked", "HaskellBinaryInfo", "HaskellBuildInfo", diff --git a/haskell/doctest.bzl b/haskell/doctest.bzl index 7bc9bc87c..6a049f997 100644 --- a/haskell/doctest.bzl +++ b/haskell/doctest.bzl @@ -1,7 +1,7 @@ """Doctest support""" load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "HaskellBinaryInfo", "HaskellBuildInfo", "HaskellLibraryInfo", diff --git a/haskell/haddock.bzl b/haskell/haddock.bzl index 360cbef58..b7cf64191 100644 --- a/haskell/haddock.bzl +++ b/haskell/haddock.bzl @@ -4,7 +4,7 @@ load(":private/context.bzl", "haskell_context") load(":private/path_utils.bzl", "module_name") load(":private/set.bzl", "set") load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "HaddockInfo", "HaskellBuildInfo", "HaskellLibraryInfo", diff --git a/haskell/haskell.bzl b/haskell/haskell.bzl index 16aa099f5..83b006173 100644 --- a/haskell/haskell.bzl +++ b/haskell/haskell.bzl @@ -1,6 +1,12 @@ """Core Haskell rules""" -load(":private/providers.bzl", "HaskellPrebuiltPackageInfo") +load( + "@io_tweag_rules_haskell//haskell:private/providers.bzl", + "HaskellBinaryInfo", + "HaskellBuildInfo", + "HaskellLibraryInfo", + "HaskellPrebuiltPackageInfo", +) load(":private/set.bzl", "set") load("@bazel_skylib//:lib.bzl", "paths") load( @@ -137,6 +143,10 @@ def _mk_binary_rule(**kwargs): "@io_tweag_rules_haskell//haskell:toolchain", "@bazel_tools//tools/cpp:toolchain_type", ], + provides = [ + HaskellBuildInfo, + HaskellBinaryInfo, + ], **kwargs ) @@ -202,6 +212,10 @@ haskell_library = rule( "@io_tweag_rules_haskell//haskell:toolchain", "@bazel_tools//tools/cpp:toolchain_type", ], + provides = [ + HaskellBuildInfo, + HaskellLibraryInfo, + ], ) """Build a library from Haskell source. diff --git a/haskell/import.bzl b/haskell/import.bzl index 154f85ceb..f77eda30f 100644 --- a/haskell/import.bzl +++ b/haskell/import.bzl @@ -3,7 +3,7 @@ load(":private/context.bzl", "haskell_context") load(":private/actions/package.bzl", "package") load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "HaddockInfo", "HaskellBuildInfo", "HaskellLibraryInfo", diff --git a/haskell/lint.bzl b/haskell/lint.bzl index 90ef849df..78b7a13aa 100644 --- a/haskell/lint.bzl +++ b/haskell/lint.bzl @@ -7,7 +7,7 @@ load( "target_unique_name", ) load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "HaskellBinaryInfo", "HaskellBuildInfo", "HaskellLibraryInfo", diff --git a/haskell/private/actions/compile.bzl b/haskell/private/actions/compile.bzl index 59a80d956..2c186748f 100644 --- a/haskell/private/actions/compile.bzl +++ b/haskell/private/actions/compile.bzl @@ -9,7 +9,7 @@ load( ) load(":private/pkg_id.bzl", "pkg_id") load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "C2hsLibraryInfo", "DefaultCompileInfo", ) diff --git a/haskell/private/actions/repl.bzl b/haskell/private/actions/repl.bzl index 24faacb18..55ab3da00 100644 --- a/haskell/private/actions/repl.bzl +++ b/haskell/private/actions/repl.bzl @@ -6,7 +6,7 @@ load( "shell", ) load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "HaskellBinaryInfo", "HaskellBuildInfo", "HaskellLibraryInfo", diff --git a/haskell/private/dependencies.bzl b/haskell/private/dependencies.bzl index 92dc4981e..3be77c42e 100644 --- a/haskell/private/dependencies.bzl +++ b/haskell/private/dependencies.bzl @@ -1,6 +1,6 @@ load(":private/path_utils.bzl", "ln") load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "CcSkylarkApiProviderHacked", "HaskellBinaryInfo", "HaskellBuildInfo", diff --git a/haskell/private/haskell_impl.bzl b/haskell/private/haskell_impl.bzl index c5073229d..5ecc639f8 100644 --- a/haskell/private/haskell_impl.bzl +++ b/haskell/private/haskell_impl.bzl @@ -21,7 +21,7 @@ load( "ln", ) load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "C2hsLibraryInfo", "HaskellBinaryInfo", "HaskellBuildInfo", diff --git a/haskell/protobuf.bzl b/haskell/protobuf.bzl index 0fdf5c676..2f6f1d1d4 100644 --- a/haskell/protobuf.bzl +++ b/haskell/protobuf.bzl @@ -1,7 +1,7 @@ """Support for protocol buffers""" load( - ":private/providers.bzl", + "@io_tweag_rules_haskell//haskell:private/providers.bzl", "HaskellBuildInfo", "HaskellLibraryInfo", "HaskellProtobufInfo", From dbfeb1ed57ba8e1d34bafe42a4030edffcf525c1 Mon Sep 17 00:00:00 2001 From: Philip Patsch Date: Thu, 22 Nov 2018 03:08:35 +0100 Subject: [PATCH 2/6] skylark: add sh_inline_test Like sh_test, but takes a (multiline) string to specify the shell script inline instead of a different file. --- skylark/BUILD | 5 ++- skylark/sh_inline_test.bzl | 60 ++++++++++++++++++++++++++++++++++ tests/BUILD | 10 ++++-- tests/scripts/test-threaded.sh | 6 ---- 4 files changed, 72 insertions(+), 9 deletions(-) create mode 100644 skylark/sh_inline_test.bzl delete mode 100755 tests/scripts/test-threaded.sh diff --git a/skylark/BUILD b/skylark/BUILD index eac1b8654..baa4efa7f 100644 --- a/skylark/BUILD +++ b/skylark/BUILD @@ -1,4 +1,7 @@ -exports_files(["lint.bzl"]) +exports_files([ + "lint.bzl", + "sh_inline_test.bzl", +]) sh_binary( name = "buildifier", diff --git a/skylark/sh_inline_test.bzl b/skylark/sh_inline_test.bzl new file mode 100644 index 000000000..d8fcb8817 --- /dev/null +++ b/skylark/sh_inline_test.bzl @@ -0,0 +1,60 @@ +load("@bazel_skylib//:lib.bzl", "shell") + +def quote_make_variables(s): + """Quote all genrule “Make” Variables in a string.""" + return s.replace("$", "$$") + +def target_from_string(name, string): + """Write a skylark string to a target.""" + native.genrule( + name = name + "-file", + outs = [name], + # this is exceptionally ugly. + cmd = """echo -n {quoted} > $(@)""".format( + # but should at least be quoted right + quoted = shell.quote(quote_make_variables(string)), + ), + ) + +bash_runfiles_boilerplate = """\ +# Copy-pasted from Bazel's Bash runfiles library (tools/bash/runfiles/runfiles.bash). +set -euo pipefail +if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ -f "$0.runfiles_manifest" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" + elif [[ -f "$0.runfiles/MANIFEST" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + fi +fi +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" +elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ + "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" +else + echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" + exit 1 +fi +# --- end runfiles.bash initialization --- +""" + +def sh_inline_test(name, script, **kwargs): + """Like sh_test, but instead of srcs takes the shell script + as verbatim bazel string. The bash runfiles are in scope, + using `rlocation` works by default. + """ + script_name = name + ".sh" + script = bash_runfiles_boilerplate + script + + target_from_string(script_name, script) + + deps = kwargs.pop("deps", []) + + native.sh_test( + name = name, + srcs = [script_name], + deps = ["@bazel_tools//tools/bash/runfiles"] + deps, + **kwargs + ) diff --git a/tests/BUILD b/tests/BUILD index b25382854..ce098e66c 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -11,6 +11,7 @@ load( ) load("@bazel_tools//tools/build_rules:test_rules.bzl", "rule_test") load("//skylark:lint.bzl", "skylark_lint") +load("//skylark:sh_inline_test.bzl", "sh_inline_test") load("@io_tweag_rules_haskell//haskell:import.bzl", haskell_import_new = "haskell_import") ghc_version = "8.4.4" @@ -268,12 +269,17 @@ rule_test( rule = "//tests/cc_haskell_import:cc-bin", ) -sh_test( +sh_inline_test( name = "test-haskell_binary-with-link-flags", size = "small", - srcs = ["scripts/test-threaded.sh"], args = ["$(location //tests/binary-with-link-flags:binary-with-link-flags)"], data = ["//tests/binary-with-link-flags"], + script = """\ +set -e + +# Fails if executable was linked without -threaded flag. +$1 +RTS -N +""", ) rule_test( diff --git a/tests/scripts/test-threaded.sh b/tests/scripts/test-threaded.sh deleted file mode 100755 index 37829c677..000000000 --- a/tests/scripts/test-threaded.sh +++ /dev/null @@ -1,6 +0,0 @@ -#/usr/bin/env sh - -set -e - -# Fails if executable was linked without -threaded flag. -$1 +RTS -N From fc3bb8377bb7601858e99be15951874d75f86220 Mon Sep 17 00:00:00 2001 From: Philip Patsch Date: Thu, 22 Nov 2018 03:10:58 +0100 Subject: [PATCH 3/6] tests/library-static-only: add sh tests to check linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two shell tests that use readelf to check that the `linkstatic` option has the expected behavior: - When it’s set to True it only produces static libraries (.a) - When it’s set to False it produces both .a and .so - .a is a static ELF (no interpreter) and .so is a dynamic ELF (interpreter appears in the ELF header INTERP field) --- tests/library-static-only/BUILD | 94 ++++++++++++++++++- .../library-static-only/get_library_files.bzl | 28 ++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 tests/library-static-only/get_library_files.bzl diff --git a/tests/library-static-only/BUILD b/tests/library-static-only/BUILD index 830f8358b..8299f601e 100644 --- a/tests/library-static-only/BUILD +++ b/tests/library-static-only/BUILD @@ -1,10 +1,15 @@ +# test whether `linkstatic` works as expected package(default_testonly = 1) +load("//skylark:sh_inline_test.bzl", "sh_inline_test") +load(":get_library_files.bzl", "get_libraries_as_runfiles") + load( "@io_tweag_rules_haskell//haskell:haskell.bzl", "haskell_library", ) +# only .a files haskell_library( name = "library-static-only", srcs = ["Lib.hs"], @@ -13,5 +18,92 @@ haskell_library( "@hackage//:base", "@hackage//:bytestring", ], - linkstatic = True, + linkstatic = True, # <-- +) + +# both .a and .so files +haskell_library( + name = "library-static-and-dynamic", + srcs = ["Lib.hs"], + visibility = ["//visibility:public"], + deps = [ + "@hackage//:base", + "@hackage//:bytestring", + ], + linkstatic = False, # <-- +) + +# extract all libraries from the haskell_library +get_libraries_as_runfiles( + name = "library-static-only-libraries", + library = ":library-static-only", +) +get_libraries_as_runfiles( + name = "library-static-and-dynamic-libraries", + library = ":library-static-and-dynamic", +) + +# sh_test’s `data` doesn’t add stuff to runfiles :( +# sh_library can bundle different targets as runfiles for sh_test +# TODO(Profpatsch): add functionality to sh_inline_test by default? +sh_library( + name = "bundled-dependency-files-static-only", + data = [":library-static-only-libraries"] +) +sh_library( + name = "bundled-dependency-files-static-and-dynamic", + data = [":library-static-and-dynamic-libraries"] +) + +# ensure that linkstatic=True only creates only .a, no .so +sh_inline_test( + name = "test-linkstatic-static-only", + script = """ +set -euo pipefail +for f in "$@"; do + if ! [[ "$f" =~ .a$ ]]; then + echo "not a static library: $f" + exit 1 + fi +done +""", + # pass the file names as arguments + args = ["$(rootpaths :library-static-only-libraries)"], + data = [ + # for rootpaths + ":library-static-only-libraries", + # to actually get the files … + ":bundled-dependency-files-static-only"], + size = "small", +) + +# test whether .so is linked dynamically and .a statically +sh_inline_test( + name = "test-libraries-static-and-dynamic", + script = """ +set -euo pipefail +is_dynamic () { + # taken from https://github.com/NixOS/nixpkgs/blob/0b3f50f844e2a6b507b18d7c5259bb850b382f87/pkgs/build-support/setup-hooks/auto-patchelf.sh#L167-L170 + readelf -l -- "$1" | grep -q "^ *INTERP\\>" +} + +for f in "$@"; do + if [[ "$f" =~ .a$ ]] && is_dynamic "$f"; then + echo "should be a static executable: $f" + exit 1 + fi + if [[ "$f" =~ .so$ ]] && ! is_dynamic "$f"; then + echo "should be a dynamic executable: $f" + exit 1 + fi +done +""", + # pass the file names as arguments + args = ["$(rootpaths :library-static-and-dynamic-libraries)"], + data = [ + # for rootpaths + ":library-static-and-dynamic-libraries", + # to actually get the files … + ":bundled-dependency-files-static-and-dynamic"], + size = "small", ) diff --git a/tests/library-static-only/get_library_files.bzl b/tests/library-static-only/get_library_files.bzl new file mode 100644 index 000000000..54490144e --- /dev/null +++ b/tests/library-static-only/get_library_files.bzl @@ -0,0 +1,28 @@ +load( + "@io_tweag_rules_haskell//haskell:private/providers.bzl", + "HaskellBuildInfo", + "HaskellLibraryInfo", +) +load("//haskell:private/set.bzl", "set") + +def _get_libraries_as_runfiles_impl(ctx): + """Extract all library files from a haskell_library target + and put them in this target’s files""" + bi = ctx.attr.library[HaskellBuildInfo] + return [DefaultInfo( + # not necessarily complete + files = depset( + direct = bi.static_libraries, + transitive = [set.to_depset(bi.dynamic_libraries)] + ), + )] + +get_libraries_as_runfiles = rule( + _get_libraries_as_runfiles_impl, + attrs = { + "library": attr.label( + mandatory = True, + providers = [HaskellBuildInfo, HaskellLibraryInfo], + ) + } +) From bc1ce90a202f2b16de62eadcc5cfdf3befbb507f Mon Sep 17 00:00:00 2001 From: Philip Patsch Date: Thu, 22 Nov 2018 13:33:09 +0100 Subject: [PATCH 4/6] tests: rename library-static-only to library-linkstatic-flag --- tests/{library-static-only => library-linkstatic-flag}/BUILD | 0 tests/{library-static-only => library-linkstatic-flag}/Lib.hs | 0 tests/{library-static-only => library-linkstatic-flag}/Main.hs | 0 .../get_library_files.bzl | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename tests/{library-static-only => library-linkstatic-flag}/BUILD (100%) rename tests/{library-static-only => library-linkstatic-flag}/Lib.hs (100%) rename tests/{library-static-only => library-linkstatic-flag}/Main.hs (100%) rename tests/{library-static-only => library-linkstatic-flag}/get_library_files.bzl (100%) diff --git a/tests/library-static-only/BUILD b/tests/library-linkstatic-flag/BUILD similarity index 100% rename from tests/library-static-only/BUILD rename to tests/library-linkstatic-flag/BUILD diff --git a/tests/library-static-only/Lib.hs b/tests/library-linkstatic-flag/Lib.hs similarity index 100% rename from tests/library-static-only/Lib.hs rename to tests/library-linkstatic-flag/Lib.hs diff --git a/tests/library-static-only/Main.hs b/tests/library-linkstatic-flag/Main.hs similarity index 100% rename from tests/library-static-only/Main.hs rename to tests/library-linkstatic-flag/Main.hs diff --git a/tests/library-static-only/get_library_files.bzl b/tests/library-linkstatic-flag/get_library_files.bzl similarity index 100% rename from tests/library-static-only/get_library_files.bzl rename to tests/library-linkstatic-flag/get_library_files.bzl From 100073a3b410d5d793b817a7118f81c0cd28c152 Mon Sep 17 00:00:00 2001 From: Philip Patsch Date: Thu, 22 Nov 2018 16:32:16 +0100 Subject: [PATCH 5/6] remove `provides` from rule(), because skydoc barfs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to https://github.com/bazelbuild/skydoc/issues/59 it also doesn’t support `provides`. Since `provides` is just nice-to-have, we drop it for now. --- haskell/haskell.bzl | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/haskell/haskell.bzl b/haskell/haskell.bzl index 83b006173..b7ed4221f 100644 --- a/haskell/haskell.bzl +++ b/haskell/haskell.bzl @@ -2,9 +2,6 @@ load( "@io_tweag_rules_haskell//haskell:private/providers.bzl", - "HaskellBinaryInfo", - "HaskellBuildInfo", - "HaskellLibraryInfo", "HaskellPrebuiltPackageInfo", ) load(":private/set.bzl", "set") @@ -143,10 +140,6 @@ def _mk_binary_rule(**kwargs): "@io_tweag_rules_haskell//haskell:toolchain", "@bazel_tools//tools/cpp:toolchain_type", ], - provides = [ - HaskellBuildInfo, - HaskellBinaryInfo, - ], **kwargs ) @@ -212,10 +205,6 @@ haskell_library = rule( "@io_tweag_rules_haskell//haskell:toolchain", "@bazel_tools//tools/cpp:toolchain_type", ], - provides = [ - HaskellBuildInfo, - HaskellLibraryInfo, - ], ) """Build a library from Haskell source. From 4778b74c0faa229bd443787d5e1911a850555cfb Mon Sep 17 00:00:00 2001 From: Philip Patsch Date: Fri, 23 Nov 2018 10:30:30 +0100 Subject: [PATCH 6/6] move sh_inline_test.bzl to tests/ --- skylark/BUILD | 1 - tests/BUILD | 2 +- tests/library-linkstatic-flag/BUILD | 2 +- {skylark => tests}/sh_inline_test.bzl | 0 4 files changed, 2 insertions(+), 3 deletions(-) rename {skylark => tests}/sh_inline_test.bzl (100%) diff --git a/skylark/BUILD b/skylark/BUILD index baa4efa7f..b128c7de6 100644 --- a/skylark/BUILD +++ b/skylark/BUILD @@ -1,6 +1,5 @@ exports_files([ "lint.bzl", - "sh_inline_test.bzl", ]) sh_binary( diff --git a/tests/BUILD b/tests/BUILD index ce098e66c..2ae6fde78 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -11,7 +11,7 @@ load( ) load("@bazel_tools//tools/build_rules:test_rules.bzl", "rule_test") load("//skylark:lint.bzl", "skylark_lint") -load("//skylark:sh_inline_test.bzl", "sh_inline_test") +load(":sh_inline_test.bzl", "sh_inline_test") load("@io_tweag_rules_haskell//haskell:import.bzl", haskell_import_new = "haskell_import") ghc_version = "8.4.4" diff --git a/tests/library-linkstatic-flag/BUILD b/tests/library-linkstatic-flag/BUILD index 8299f601e..af1bc5bfc 100644 --- a/tests/library-linkstatic-flag/BUILD +++ b/tests/library-linkstatic-flag/BUILD @@ -1,7 +1,7 @@ # test whether `linkstatic` works as expected package(default_testonly = 1) -load("//skylark:sh_inline_test.bzl", "sh_inline_test") +load("//tests:sh_inline_test.bzl", "sh_inline_test") load(":get_library_files.bzl", "get_libraries_as_runfiles") load( diff --git a/skylark/sh_inline_test.bzl b/tests/sh_inline_test.bzl similarity index 100% rename from skylark/sh_inline_test.bzl rename to tests/sh_inline_test.bzl