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

Make nixpkgs GHC 9.6.x work #1921

Merged
merged 10 commits into from
Dec 11, 2023
3 changes: 3 additions & 0 deletions .github/workflows/workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,16 @@ jobs:
ghc:
- 9.2.8
- 9.4.6
- 9.6.2
exclude:
- module: rules_haskell_nix
bzlmod: false
# TODO: in a MODULE.bazel file we declare version specific dependencies, would need to use stack snapshot json
# and stack config per GHC version
- ghc: 9.4.6
bzlmod: true
- ghc: 9.6.2
bzlmod: true
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
Expand Down
9 changes: 9 additions & 0 deletions haskell/cabal.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,15 @@ def _prepare_cabal_inputs(
]
extra_args = ["--flags=" + " ".join(flags)]

if hs.toolchain.is_darwin:
# assume `otool` and `install_name_tool` are available at the same location as `ar`
ar_bindir = paths.dirname(cc.tools.ar)

extra_args.append("--ghc-option=-pgmotool=" + paths.join(ar_bindir, "otool"))
extra_args.append("--ghc-option=-pgminstall_name_tool=" + paths.join(ar_bindir, "install_name_tool"))
extra_args.append("--haddock-option=--optghc=-pgmotool=" + paths.join(ar_bindir, "otool"))
extra_args.append("--haddock-option=--optghc=-pgminstall_name_tool=" + paths.join(ar_bindir, "install_name_tool"))

ghc_version = [int(x) for x in hs.toolchain.version.split(".")]
if dynamic_file:
# See Note [No PIE when linking] in haskell/private/actions/link.bzl
Expand Down
7 changes: 7 additions & 0 deletions haskell/experimental/private/module.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,13 @@ def _build_haskell_module(
args.add_all(hs.toolchain.ghcopts)
args.add_all(user_ghcopts)

if hs.toolchain.is_darwin:
# assume `otool` and `install_name_tool` are available at the same location as `ar`
ar_bindir = paths.dirname(cc.tools.ar)

args.add(paths.join(ar_bindir, "otool"), format = "-pgmotool=%s")
args.add(paths.join(ar_bindir, "install_name_tool"), format = "-pgminstall_name_tool=%s")

if plugins and not enable_th:
# For #1681. These suppresses bogus warnings about missing libraries which
# aren't really needed.
Expand Down
9 changes: 9 additions & 0 deletions haskell/private/actions/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,15 @@ def _compilation_defaults(
compile_flags += hs.toolchain.ghcopts
compile_flags += user_compile_flags

if hs.toolchain.is_darwin:
# assume `otool` and `install_name_tool` are available at the same location as `ar`
ar_bindir = paths.dirname(cc.tools.ar)

compile_flags += [
"-pgmotool=" + paths.join(ar_bindir, "otool"),
"-pgminstall_name_tool=" + paths.join(ar_bindir, "install_name_tool"),
]
Comment on lines +127 to +134
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this may fail for users that try to use a hermetic bindist cc toolchain like grail's llvm toolchain. I'm fine with this for now as it is an improvement over the status quo. But, from what I understand so far, this may not cover all use-cases, yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we probably will need a better solution to this in the future.


package_ids = []
all_plugins = plugins + non_default_plugins
for plugin in all_plugins:
Expand Down
16 changes: 16 additions & 0 deletions haskell/private/actions/link.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@ def link_binary(
args.add_all(cc.linker_flags, format_each = "-optl%s")
if with_profiling:
args.add("-prof")

if hs.toolchain.is_darwin:
# assume `otool` and `install_name_tool` are available at the same location as `ar`
ar_bindir = paths.dirname(cc.tools.ar)

args.add(paths.join(ar_bindir, "otool"), format = "-pgmotool=%s")
args.add(paths.join(ar_bindir, "install_name_tool"), format = "-pgminstall_name_tool=%s")

args.add_all(hs.toolchain.ghcopts)
args.add_all(compiler_flags)

Expand Down Expand Up @@ -366,6 +374,14 @@ def link_library_dynamic(hs, cc, posix, dep_info, extra_srcs, object_files, my_p
args = hs.actions.args()
args.add_all(cc.linker_flags, format_each = "-optl%s")
args.add_all(["-shared", "-dynamic"])

if hs.toolchain.is_darwin:
# assume `otool` and `install_name_tool` are available at the same location as `ar`
ar_bindir = paths.dirname(cc.tools.ar)

args.add(paths.join(ar_bindir, "otool"), format = "-pgmotool=%s")
args.add(paths.join(ar_bindir, "install_name_tool"), format = "-pgminstall_name_tool=%s")

args.add_all(hs.toolchain.ghcopts)
args.add_all(compiler_flags)

Expand Down
76 changes: 60 additions & 16 deletions haskell/private/pkgdb_to_bzl.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,43 @@
if len(sys.argv) == 3:
repo_dir = "external/" + sys.argv[1]
topdir = sys.argv[2]

if os.path.exists(os.path.join(topdir, 'package.conf.d')):
package_conf_dir = os.path.join(topdir, 'package.conf.d')
elif os.path.exists(os.path.join(topdir, 'lib', 'package.conf.d')):
topdir = os.path.join(topdir, 'lib')
package_conf_dir = os.path.join(topdir, 'package.conf.d')
else:
sys.exit("could not find package.conf.d directory at {}".format(topdir))
repo_root = os.getcwd()
else:
sys.exit("Usage: pkgdb_to_bzl.py <REPO_NAME> <TOPDIR>")

def resolve(path, pkgroot):
"""Resolve references to ${pkgroot} with the given value, resolve $topdir with `topdir`"""
if path.find("${pkgroot}") != -1:
norm_path = os.path.normpath(path.strip("\"").replace("${pkgroot}", pkgroot))
if not os.path.isabs(norm_path) and norm_path.startswith('..'):
return resolve(path, os.path.realpath(pkgroot))
else:
return norm_path
elif path.startswith("$topdir"):
return os.path.normpath(path.replace("$topdir", topdir)).replace('\\', '/')
else:
return path

def path_to_label(path, pkgroot):
"""Substitute one pkgroot for another relative one to obtain a label."""
if path.find("${pkgroot}") != -1:
return os.path.normpath(path.strip("\"").replace("${pkgroot}", topdir)).replace('\\', '/')
# determine if the given path is inside the repository root
# if it is not, return None to signal it needs to be symlinked into the
# repository
norm_path = os.path.normpath(resolve(path, pkgroot))
relative_path = os.path.relpath(norm_path, start=repo_root)

return None if relative_path.startswith('..') else relative_path.replace('\\', '/')

topdir_relative_path = path.replace(pkgroot, "$topdir")
topdir_relative_path = path.replace(os.path.realpath(pkgroot), "$topdir")
if topdir_relative_path.find("$topdir") != -1:
return os.path.normpath(topdir_relative_path.replace("$topdir", topdir)).replace('\\', '/')

Expand Down Expand Up @@ -79,15 +107,17 @@ def hs_library_pattern(name, mode = "static", profiling = False):

# Accumulate package id to package name mappings.
pkg_id_map = []
for conf in glob.glob(os.path.join(topdir, "package.conf.d", "*.conf")):

# pkgroot is not part of .conf files. It's a computed value. It is
# defined to be the directory enclosing the package database
# directory.
pkgroot = os.path.dirname(package_conf_dir)


for conf in glob.glob(os.path.join(package_conf_dir, '*.conf')):
with open(conf, 'r') as f:
pkg = package_configuration.parse_package_configuration(f)

# pkgroot is not part of .conf files. It's a computed value. It is
# defined to be the directory enclosing the package database
# directory.
pkgroot = os.path.dirname(os.path.dirname(os.path.realpath(conf)))

pkg_id_map.append((pkg.name, pkg.id))

# Haddock handling
Expand All @@ -105,31 +135,45 @@ def hs_library_pattern(name, mode = "static", profiling = False):
haddock_html = None

if pkg.haddock_html:
haddock_html = path_to_label(pkg.haddock_html, pkgroot)
# We check if the file exists because cabal will unconditionally
# generate the database entry even if no haddock was generated.
if not haddock_html and os.path.exists(pkg.haddock_html):
haddock_html = os.path.join("haddock", "html", pkg.name)
output.append("#SYMLINK: {} {}".format(pkg.haddock_html, haddock_html))
resolved_haddock_html = resolve(pkg.haddock_html, pkgroot)

if not os.path.exists(resolved_haddock_html):
# try to resolve relative to the package.conf.d dir
# see https://gitlab.haskell.org/ghc/ghc/-/issues/23476
resolved_haddock_html = resolve(pkg.haddock_html, package_conf_dir)

if os.path.exists(resolved_haddock_html):
haddock_html = path_to_label(pkg.haddock_html, pkgroot)
if not haddock_html:
haddock_html = os.path.join("haddock", "html", pkg.name)
output.append("#SYMLINK: {} {}".format(resolved_haddock_html.replace('\\', '/'), haddock_html))

# If there is many interfaces, we give them a number
interface_id = 0
haddock_interfaces = []
for interface_path in pkg.haddock_interfaces:
interface = path_to_label(interface_path, pkgroot)
resolved_path = resolve(interface_path, pkgroot).replace('\\', '/')

if not os.path.exists(resolved_path):
# try to resolve relative to the package.conf.d dir
# see https://gitlab.haskell.org/ghc/ghc/-/issues/23476
resolved_path = resolve(interface_path, package_conf_dir)

# We check if the file exists because cabal will unconditionally
# generate the database entry even if no haddock was generated.
if not os.path.exists(interface or interface_path):
continue
if not os.path.exists(resolved_path): continue

interface = path_to_label(interface_path, pkgroot)

if not interface:
interface = os.path.join(
"haddock",
"interfaces",
pkg.name + "_" + str(interface_id) + ".haddock",
)
output.append("#SYMLINK: {} {}".format(interface_path, interface))
output.append("#SYMLINK: {} {}".format(resolved_path, interface))
interface_id += 1
haddock_interfaces.append(interface)

Expand Down
4 changes: 1 addition & 3 deletions rules_haskell_tests/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,7 @@ rule_test(
"haddock/testsZShaddockZShaddock-lib-b",
"haddock/testsZShaddockZShaddock-lib-deep",
],
}[TEST_GHC_VERSION] + ([
"haddock/rts-1.0.2",
] if is_windows else []),
}[TEST_GHC_VERSION],
rule = "//tests/haddock",
)

Expand Down
7 changes: 6 additions & 1 deletion rules_haskell_tests/tests/RunTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import System.Directory (copyFile)
import System.FilePath ((</>))
import System.Info (os)
import System.IO.Temp (withSystemTempDirectory)
import System.Environment (lookupEnv)

import qualified System.Process as Process
import Test.Hspec.Core.Spec (SpecM)
Expand All @@ -26,11 +27,15 @@ main = hspec $ do
assertSuccess (bazel ["test", "//..."])

it "bazel test prof" $ do
ghcVersion <- lookupEnv "GHC_VERSION"

-- In .github/workflows/workflow.yaml we specify --test_tag_filters
-- -dont_test_on_darwin. However, specifiying --test_tag_filters
-- -requires_dynamic here alone would override that filter. So,
-- we have to duplicate that filter here.
let tagFilter | os == "darwin" = "-dont_test_on_darwin,-requires_dynamic,-skip_profiling"
let tagFilter | os == "darwin" = "-dont_test_on_darwin,-requires_dynamic,-skip_profiling" ++ (
-- skip tests for specific GHC version, see https://github.com/tweag/rules_haskell/issues/2073
maybe "" (",-dont_build_on_macos_with_ghc_" ++) ghcVersion)
| otherwise = "-requires_dynamic,-skip_profiling"
assertSuccess (bazel ["test", "-c", "dbg", "//...", "--build_tag_filters", tagFilter, "--test_tag_filters", tagFilter])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ haskell_library(
narrowed_deps = [
":TestLib2",
],
tags = [
# skip building this target on macos with GHC 9.6.2 since it crashes ghc-iserv
# see https://github.com/tweag/rules_haskell/issues/2073
"dont_build_on_macos_with_ghc_9.6.2",
],
deps = [
":NonModulesTestLib",
":TestLib",
Expand All @@ -113,6 +118,11 @@ haskell_test(
name = "Test",
modules = [":TestBinModule"],
narrowed_deps = [":lib"],
tags = [
# skip testing this target on macos with GHC 9.6.2 since it crashes ghc-iserv
# see https://github.com/tweag/rules_haskell/issues/2073
"dont_build_on_macos_with_ghc_9.6.2",
],
visibility = ["//visibility:public"],
deps = [
"//tests/hackage:base",
Expand Down