Skip to content
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

Refactoring: remove visible-binaries #557

Merged
merged 1 commit into from
Jan 14, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
* The `prebuilt_dependencies` attribute of all haskell rules
had been deprecated two versions ago and is removed.
Use `haskell_import` instead (see docs for usage).
* The `extra_binaries` field is now no longer supported.

## [0.7] - 2018-12-24

Expand Down
3 changes: 1 addition & 2 deletions haskell/BUILD
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
exports_files(
glob(["*.bzl"]) + [
"assets/ghci_script",
"private/c2hs_wrapper.sh",
"private/ghci_repl_wrapper.sh",
"private/haddock_wrapper.sh",
"private/haddock_wrapper.sh.tpl",
],
)

Expand Down
20 changes: 12 additions & 8 deletions haskell/c2hs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def _c2hs_library_impl(ctx):
args.add([chs_file.path, "-o", hs_file.path])

args.add(["-C-E"])
args.add(["--cpp", hs.tools.cc.path])
args.add(["--cpp", cc.tools.cc])
args.add("-C-includeghcplatform.h")
args.add("-C-includeghcversion.h")
args.add(["-C" + x for x in cc.cpp_flags])
Expand All @@ -48,14 +48,22 @@ def _c2hs_library_impl(ctx):
]
args.add(chi_includes)

hs.actions.run(
hs.actions.run_shell(
inputs = depset(transitive = [
depset(cc.hdrs),
depset([hs.tools.cc, hs.tools.ghc, hs.tools.c2hs, chs_file]),
depset([hs.tools.ghc, hs.tools.c2hs, chs_file]),
depset(dep_chi_files),
depset(cc.files),
]),
outputs = [hs_file, chi_file],
executable = ctx.file._chs_wrapper,
command = """
# Include libdir in include path just like hsc2hs does.
libdir=$({ghc} --print-libdir)
{c2hs} -C-I$libdir/include "$@"
""".format(
ghc = hs.tools.ghc.path,
c2hs = hs.tools.c2hs.path,
),
mnemonic = "HaskellC2Hs",
arguments = [args],
env = hs.env,
Expand Down Expand Up @@ -84,10 +92,6 @@ c2hs_library = rule(
"src_strip_prefix": attr.string(
doc = "Directory in which module hierarchy starts.",
),
"_chs_wrapper": attr.label(
allow_single_file = True,
default = Label("@io_tweag_rules_haskell//haskell:private/c2hs_wrapper.sh"),
),
"_cc_toolchain": attr.label(
default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),
),
Expand Down
21 changes: 21 additions & 0 deletions haskell/cc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ load(
CcInteropInfo = provider(
doc = "Information needed for interop with cc rules.",
fields = {
"tools": "Tools from the CC toolchain",
# See the following for why this is needed:
# https://stackoverflow.com/questions/52769846/custom-c-rule-with-the-cc-common-api
"files": "Files for all tools (input to any action that uses tools)",
"hdrs": "CC headers",
"cpp_flags": "Preprocessor flags",
"compiler_flags": "Flags for compilation",
Expand Down Expand Up @@ -121,7 +125,24 @@ def cc_interop_info(ctx):
# XXX Workaround https://github.com/bazelbuild/bazel/issues/6876.
linker_flags = [flag for flag in linker_flags if flag not in ["-shared"]]

tools = {
"ar": cc_toolchain.ar_executable(),
"cc": cc_toolchain.compiler_executable(),
"ld": cc_toolchain.ld_executable(),
"cpp": cc_toolchain.preprocessor_executable(),
}

# If running on darwin but XCode is not installed (i.e., only the Command
# Line Tools are available), then Bazel will make ar_executable point to
# "/usr/bin/libtool". Since we call ar directly, override it.
# TODO: remove this if Bazel fixes its behavior.
# Upstream ticket: https://github.com/bazelbuild/bazel/issues/5127.
if tools["ar"].find("libtool") >= 0:
tools["ar"] = "/usr/bin/ar"

return CcInteropInfo(
tools = struct(**tools),
files = ctx.files._cc_toolchain,
hdrs = hdrs.to_list(),
cpp_flags = cpp_flags,
include_args = include_args,
Expand Down
17 changes: 14 additions & 3 deletions haskell/doctest.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,26 @@ def _haskell_doctest_single(target, ctx):
depset([exposed_modules_file]),
depset(
toolchain.doctest +
[hs.tools.cat, hs.tools.ghc],
[hs.tools.ghc],
),
]),
outputs = [doctest_log],
mnemonic = "HaskellDoctest",
progress_message = "HaskellDoctest {}".format(ctx.label),
command = """
{doctest} "$@" $(cat {module_list} | tr , ' ') > {output} 2>&1 || rc=$? && cat {output} && exit $rc
""".format(doctest = toolchain.doctest[0].path, output = doctest_log.path, module_list = exposed_modules_file.path),
{env}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason not to use run_shell's env parameter ?

{doctest} "$@" $(cat {module_list} | tr , ' ') > {output} 2>&1 || rc=$? && cat {output} && exit $rc
""".format(
doctest = toolchain.doctest[0].path,
output = doctest_log.path,
module_list = exposed_modules_file.path,
# XXX Workaround
# https://github.com/bazelbuild/bazel/issues/5980.
env = "\n".join([
"export {}={}".format(k, v)
for k, v in hs.env.items()
]),
),
arguments = [args],
# NOTE It looks like we must specify the paths here as well as via -L
# flags because there are at least two different "consumers" of the info
Expand Down
36 changes: 24 additions & 12 deletions haskell/haddock.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,25 @@ def _haskell_doc_aspect_impl(target, ctx):
depset([hs.toolchain.locale_archive]) if hs.toolchain.locale_archive != None else depset()
)

# TODO(mboes): we should be able to instantiate this template only
# once per toolchain instance, rather than here.
haddock_wrapper = ctx.actions.declare_file("haddock_wrapper-{}".format(hs.name))
ctx.actions.expand_template(
template = ctx.file._haddock_wrapper_tpl,
output = haddock_wrapper,
substitutions = {
"%{ghc-pkg}": hs.tools.ghc_pkg.path,
"%{haddock}": hs.tools.haddock.path,
# XXX Workaround
# https://github.com/bazelbuild/bazel/issues/5980.
"%{env}": "\n".join([
"export {}={}".format(k, v)
for k, v in hs.env.items()
]),
},
is_executable = True,
)

ctx.actions.run(
inputs = depset(transitive = [
set.to_depset(target[HaskellBuildInfo].package_caches),
Expand All @@ -105,24 +124,21 @@ def _haskell_doc_aspect_impl(target, ctx):
set.to_depset(target[HaskellLibraryInfo].source_files),
target[HaskellLibraryInfo].extra_source_files,
depset([
hs.tools.bash,
hs.tools.ghc_pkg,
hs.tools.haddock,
hs.tools.mktemp,
hs.tools.rmdir,
]),
locale_archive_depset,
]),
outputs = [haddock_file, html_dir],
mnemonic = "HaskellHaddock",
progress_message = "HaskellHaddock {}".format(ctx.label),
executable = ctx.file._haddock_wrapper,
executable = haddock_wrapper,
arguments = [
prebuilt_deps,
args,
ghc_args,
],
env = hs.env,
use_default_shell_env = True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the haddock wrapper script use mktemp and rmdir. Previously, those were "visible binaries". Now, they are taken from the default shell environment (which Bazel will be restricting soon, by flipping --experimental_strict_action_env on by default).

)

transitive_html.update({package_id: html_dir})
Expand All @@ -140,9 +156,9 @@ def _haskell_doc_aspect_impl(target, ctx):
haskell_doc_aspect = aspect(
_haskell_doc_aspect_impl,
attrs = {
"_haddock_wrapper": attr.label(
"_haddock_wrapper_tpl": attr.label(
allow_single_file = True,
default = Label("@io_tweag_rules_haskell//haskell:private/haddock_wrapper.sh"),
default = Label("@io_tweag_rules_haskell//haskell:private/haddock_wrapper.sh.tpl"),
),
},
attr_aspects = ["deps"],
Expand Down Expand Up @@ -191,11 +207,7 @@ def _haskell_doc_rule_impl(ctx):
html_dict_copied[package_id] = output_dir

ctx.actions.run_shell(
inputs = [
hs.tools.cp,
hs.tools.mkdir,
html_dir,
],
inputs = [html_dir],
outputs = [output_dir],
command = """
mkdir -p "{doc_dir}"
Expand Down
18 changes: 11 additions & 7 deletions haskell/lint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,26 @@ def _haskell_lint_aspect_impl(target, ctx):
set.to_depset(build_info.interface_dirs),
set.to_depset(build_info.dynamic_libraries),
depset([e.mangled_lib for e in set.to_list(build_info.external_libraries)]),
depset([
hs.tools.ghc,
hs.tools.cat,
]),
depset([hs.tools.ghc]),
]),
outputs = [lint_log],
mnemonic = "HaskellLint",
progress_message = "HaskellLint {}".format(ctx.label),
command = """
ghc "$@" > {output} 2>&1 || rc=$? && cat {output} && exit $rc
""".format(
{env}
{ghc} "$@" > {output} 2>&1 || rc=$? && cat {output} && exit $rc
""".format(
ghc = hs.tools.ghc.path,
output = lint_log.path,
# XXX Workaround
# https://github.com/bazelbuild/bazel/issues/5980.
env = "\n".join([
"export {}={}".format(k, v)
for k, v in hs.env.items()
]),
),
arguments = [args],
env = hs.env,
use_default_shell_env = True,
)

lint_info = HaskellLintInfo(outputs = set.singleton(lint_log))
Expand Down
14 changes: 8 additions & 6 deletions haskell/private/actions/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ def _process_hsc_file(hs, cc, hsc_flags, hsc_file):
hs_out = declare_compiled(hs, hsc_file, ".hs", directory = hsc_dir_raw)
args.add([hsc_file.path, "-o", hs_out.path])

args.add(["-c", hs.tools.cc])
args.add(["-l", hs.tools.cc])
args.add(["-c", cc.tools.cc])
args.add(["-l", cc.tools.cc])
args.add("-ighcplatform.h")
args.add("-ighcversion.h")
args.add(["--cflag=" + f for f in cc.cpp_flags])
Expand All @@ -49,7 +49,8 @@ def _process_hsc_file(hs, cc, hsc_flags, hsc_file):
hs.actions.run(
inputs = depset(transitive = [
depset(cc.hdrs),
depset([hs.tools.cc, hsc_file]),
depset([hsc_file]),
depset(cc.files),
]),
outputs = [hs_out],
mnemonic = "HaskellHsc2hs",
Expand Down Expand Up @@ -250,7 +251,6 @@ def _compilation_defaults(hs, cc, java, dep_info, srcs, import_dir_map, extra_sr
set.to_depset(dep_info.dynamic_libraries),
depset([e.mangled_lib for e in set.to_list(dep_info.external_libraries)]),
java.inputs,
depset([hs.tools.cc]),
locale_archive_depset,
]),
objects_dir = objects_dir,
Expand Down Expand Up @@ -291,7 +291,8 @@ def compile_binary(hs, cc, java, dep_info, srcs, ls_modules, import_dir_map, ext

hs.toolchain.actions.run_ghc(
hs,
inputs = c.inputs + hs.extra_binaries,
cc,
inputs = c.inputs,
outputs = c.outputs,
mnemonic = "HaskellBuildBinary",
progress_message = "HaskellBuildBinary {}".format(hs.label),
Expand Down Expand Up @@ -348,7 +349,8 @@ def compile_library(hs, cc, java, dep_info, srcs, ls_modules, other_modules, exp

hs.toolchain.actions.run_ghc(
hs,
inputs = c.inputs + hs.extra_binaries,
cc,
inputs = c.inputs,
outputs = c.outputs,
mnemonic = "HaskellBuildLibrary",
progress_message = "HaskellBuildLibrary {}".format(hs.label),
Expand Down
11 changes: 5 additions & 6 deletions haskell/private/actions/link.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ def link_binary(
)
hs.toolchain.actions.run_ghc(
hs,
cc,
inputs = depset(transitive = [
depset(extra_srcs),
set.to_depset(dep_info.package_caches),
Expand All @@ -213,7 +214,6 @@ def link_binary(
depset(dep_info.static_libraries_prof),
depset([objects_dir]),
depset([e.mangled_lib for e in set.to_list(dep_info.external_libraries)]),
depset(hs.extra_binaries),
]),
outputs = [compile_output],
mnemonic = "HaskellLinkBinary",
Expand Down Expand Up @@ -298,8 +298,7 @@ def link_library_static(hs, cc, dep_info, objects_dir, my_pkg_id, with_profiling
with_profiling = with_profiling,
)
args = hs.actions.args()
inputs = ([objects_dir, objects_dir_manifest, hs.tools.ar] +
hs.tools_runfiles.ar + hs.extra_binaries)
inputs = [objects_dir, objects_dir_manifest] + cc.files

if hs.toolchain.is_darwin:
# On Darwin, ar doesn't support params files.
Expand All @@ -316,7 +315,7 @@ def link_library_static(hs, cc, dep_info, objects_dir, my_pkg_id, with_profiling
inputs = inputs,
outputs = [static_library],
mnemonic = "HaskellLinkStaticLibrary",
command = "{ar} qc $1 $(< $2)".format(ar = hs.tools.ar.path),
command = "{ar} qc $1 $(< $2)".format(ar = cc.tools.ar),
arguments = [args],

# Use the default macosx toolchain
Expand All @@ -332,7 +331,7 @@ def link_library_static(hs, cc, dep_info, objects_dir, my_pkg_id, with_profiling
inputs = inputs,
outputs = [static_library],
mnemonic = "HaskellLinkStaticLibrary",
executable = hs.tools.ar,
executable = cc.tools.ar,
arguments = [args],
)

Expand Down Expand Up @@ -405,8 +404,8 @@ def link_library_dynamic(hs, cc, dep_info, extra_srcs, objects_dir, my_pkg_id):

hs.toolchain.actions.run_ghc(
hs,
cc,
inputs = depset([objects_dir], transitive = [
depset(hs.extra_binaries),
depset(extra_srcs),
set.to_depset(dep_info.package_caches),
set.to_depset(dep_info.dynamic_libraries),
Expand Down
7 changes: 0 additions & 7 deletions haskell/private/c2hs_wrapper.sh

This file was deleted.

3 changes: 0 additions & 3 deletions haskell/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def haskell_context(ctx, attr = None):
)

env = {
"PATH": toolchain.visible_bin_path,
"LANG": toolchain.locale,
}

Expand All @@ -35,8 +34,6 @@ def haskell_context(ctx, attr = None):
label = ctx.label,
toolchain = toolchain,
tools = toolchain.tools,
tools_runfiles = toolchain.tools_runfiles,
extra_binaries = toolchain.extra_binaries,
src_root = src_root,
env = env,
mode = ctx.var["COMPILATION_MODE"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

set -eo pipefail

%{env}

PREBUILT_DEPS_FILE=$1
shift

Expand All @@ -19,8 +21,8 @@ do
# If there were more than one file, going by the output for the `depends`,
# the file names would be separated by a space character.
# https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/packages.html#installedpackageinfo-a-package-specification
haddock_interfaces=$(ghc-pkg --simple-output field $pkg haddock-interfaces)
haddock_html=$(ghc-pkg --simple-output field $pkg haddock-html)
haddock_interfaces=$(%{ghc-pkg} --simple-output field $pkg haddock-interfaces)
haddock_html=$(%{ghc-pkg} --simple-output field $pkg haddock-html)

# Sometimes the referenced `.haddock` file does not exist
# (e.g. for `nixpkgs.haskellPackages` deps with haddock disabled).
Expand All @@ -43,5 +45,5 @@ cleanup() { rmdir "$TEMP"; }
# XXX Override TMPDIR to prevent race conditions on certain platforms.
# This is a workaround for
# https://github.com/haskell/haddock/issues/894.
TMPDIR=$TEMP haddock "${extra_args[@]}" "$@"
TMPDIR=$TEMP %{haddock} "${extra_args[@]}" "$@"
cleanup
Loading