Make package names more similar to the rule name.

As of #222, both the package name and ID are z-encoded.  However,
GHC only needs the package *ID* to be unique, as long as you
call `-package-id` instead of `-package`.  As a result we can
simplify how package-names are encoded: we can keep them the same
as the rule name, other than fixing underscores.

In addition to (I think) making error messages nicer, this paves the way for
some improvements in Hazel (or similar tools), now that the rule and package
names can be identical:
- It helps support the PackageImports extension.
- GHC >= 8.0 generates the Cabal macros (e.g., `MIN_VERSION_*`) automatically.

Prebuilt dependencies are still passed with `-package` since their IDs
are internal details of GHC.

haskell/actions.bzl

@@ -412,8 +412,8 @@ def link_haskell_bin(ctx, object_files):
 
   args.add([f.path for f in object_files])
 
-  for package in set.to_list(dep_info.package_names):
-    args.add(["-package", package])
+  for package in set.to_list(dep_info.package_ids):
+    args.add(["-package-id", package])
 
   for cache in set.to_list(dep_info.package_caches):
     args.add(["-package-db", cache.dirname])
@@ -478,7 +478,7 @@ def compile_haskell_lib(ctx):
   """
   c = _compilation_defaults(ctx)
   c.args.add([
-    "-package-name", get_pkg_id(ctx),
+    "-package-name", get_pkg_name(ctx)
   ])
 
   # This is absolutely required otherwise GHC doesn't know what package it's
@@ -559,12 +559,11 @@ def link_dynamic_lib(ctx, object_files):
 
   dep_info = gather_dep_info(ctx)
 
-  for package in set.to_list(
-      set.union(
-        dep_info.package_names,
-        set.from_list(ctx.attr.prebuilt_dependencies),
-      )):
-    args.add(["-package", package])
+  for package in set.to_list(dep_info.package_ids):
+    args.add(["-package-id", package])
+  for package in set.to_list(set.from_list(ctx.attr.prebuilt_dependencies)):
+    args.add(["-package-name", package])
+
 
   for cache in set.to_list(dep_info.package_caches):
     args.add(["-package-db", cache.dirname])
@@ -636,7 +635,7 @@ def create_ghc_package(ctx, interfaces_dir, static_library, dynamic_library, exp
     "dynamic-library-dirs": "${pkgroot}",
     "hs-libraries": _get_library_name(ctx),
     "depends":
-      ", ".join([ d[HaskellLibraryInfo].package_name for d in
+      ", ".join([ d[HaskellLibraryInfo].package_id for d in
                   ctx.attr.deps if HaskellLibraryInfo in d])
   }
   ctx.actions.write(
@@ -735,8 +734,8 @@ def _compilation_defaults(ctx):
     haddock_args.add(items, before_each="--optghc")
 
   # Expose all bazel dependencies
-  for package in set.to_list(dep_info.package_names):
-    items = ["-package", package]
+  for package in set.to_list(dep_info.package_ids):
+    items = ["-package-id", package]
     args.add(items)
     if package != get_pkg_id(ctx):
       haddock_args.add(items, before_each="--optghc")
@@ -866,35 +865,41 @@ def _zencode(s):
   return s.replace("Z", "ZZ").replace("_", "ZU").replace("/", "ZS")
 
 def get_pkg_name(ctx):
-  """Get package name. Package name includes Bazel package and name of the
-  component. We can't use just the latter because then two components with
-  the same names in different packages would clash.
+  """Get package name.
+
+  The name is not required to be unique/injective; however it must be a valid
+  GHC package name (i.e., no underscores).  This encoding is not aims to
+  change as little as possible since it is used for display and also for the
+  "PackageImports" extension.
 
   Args:
     ctx: Rule context
 
   Returns:
     string: GHC package name to use.
   """
-  return _zencode("/".join([i for i in [
-    ctx.label.workspace_root,
-    ctx.label.package,
-    ctx.attr.name,
-  ] if i != ""]))
+  return ctx.attr.name.replace("_", "ZU")
 
 def get_pkg_id(ctx):
   """Get package identifier of the form `name-version`.
 
+  The identifier is required to be unique for each Haskell rule.
+  It includes the Bazel package and the name of the this component.
+  We can't use just the latter because then two components with
+  the same names in different packages would clash.
+
   Args:
     ctx: Rule context
 
   Returns:
     string: GHC package ID to use.
   """
   return "{0}-{1}".format(
-    get_pkg_name(ctx),
-    ctx.attr.version,
-  )
+    _zencode(paths.join(
+        ctx.label.workspace_root,
+        ctx.label.package,
+        ctx.attr.name)),
+    ctx.attr.version)
 
 def _get_library_name(ctx):
   """Get core library name for this package. This is "HS" followed by package ID.
@@ -923,7 +928,7 @@ def gather_dep_info(ctx):
   """
 
   acc = HaskellBuildInfo(
-    package_names = set.empty(),
+    package_ids = set.empty(),
     package_confs = set.empty(),
     package_caches = set.empty(),
     static_libraries = [],
@@ -936,13 +941,13 @@ def gather_dep_info(ctx):
   for dep in ctx.attr.deps:
     if HaskellBuildInfo in dep:
       binfo = dep[HaskellBuildInfo]
-      package_names = acc.package_names
+      package_ids = acc.package_ids
       if HaskellBinaryInfo in dep:
         fail("Target {0} cannot depend on binary".format(ctx.attr.name))
       if HaskellLibraryInfo in dep:
-        set.mutable_insert(package_names, dep[HaskellLibraryInfo].package_name)
+        set.mutable_insert(package_ids, dep[HaskellLibraryInfo].package_id)
       acc = HaskellBuildInfo(
-        package_names = package_names,
+        package_ids = package_ids,
         package_confs = set.mutable_union(acc.package_confs, binfo.package_confs),
         package_caches = set.mutable_union(acc.package_caches, binfo.package_caches),
         static_libraries = acc.static_libraries + binfo.static_libraries,
@@ -955,7 +960,7 @@ def gather_dep_info(ctx):
       # If not a Haskell dependency, pass it through as-is to the
       # linking phase.
       acc = HaskellBuildInfo(
-        package_names = acc.package_names,
+        package_ids = acc.package_ids,
         package_confs = acc.package_confs,
         package_caches = acc.package_caches,
         static_libraries = acc.static_libraries,

haskell/ghci-repl.bzl

@@ -60,9 +60,9 @@ def build_haskell_repl(
   args = ["-hide-all-packages"]
   for dep in set.to_list(build_info.prebuilt_dependencies):
     args += ["-package ", dep]
-  for package in set.to_list(build_info.package_names):
-    if not (interpreted and lib_info != None and package == lib_info.package_name):
-      args += ["-package", package]
+  for package in set.to_list(build_info.package_ids):
+    if not (interpreted and lib_info != None and package == lib_info.package_id):
+      args += ["-package-id", package]
   for cache in set.to_list(build_info.package_caches):
     args += ["-package-db", cache.dirname]
 

haskell/haddock.bzl

@@ -49,7 +49,7 @@ def _haskell_doc_aspect_impl(target, ctx):
   if HaskellBuildInfo not in target or HaskellLibraryInfo not in target:
     return []
 
-  pkg_id = target[HaskellLibraryInfo].package_name
+  pkg_id = target[HaskellLibraryInfo].package_id
 
   doc_dir_raw = "doc-{0}".format(pkg_id)
 

haskell/haskell-impl.bzl

@@ -87,7 +87,7 @@ def haskell_library_impl(ctx):
   )
 
   build_info = HaskellBuildInfo(
-    package_names = set.insert(dep_info.package_names, get_pkg_id(ctx)),
+    package_ids = set.insert(dep_info.package_ids, get_pkg_id(ctx)),
     package_confs = set.insert(dep_info.package_confs, conf_file),
     package_caches = set.insert(dep_info.package_caches, cache_file),
     # NOTE We have to use lists for static libraries because the order is
@@ -101,7 +101,7 @@ def haskell_library_impl(ctx):
     external_libraries = dep_info.external_libraries,
   )
   lib_info = HaskellLibraryInfo(
-    package_name = get_pkg_id(ctx),
+    package_id = get_pkg_id(ctx),
     import_dirs = c.import_dirs,
     exposed_modules = exposed_modules,
     other_modules = other_modules,

haskell/lint.bzl

@@ -66,9 +66,9 @@ def _haskell_lint_aspect_impl(target, ctx):
     args.add(["-package", prebuilt_dep])
 
   # Expose all bazel dependencies
-  for package in set.to_list(build_info.package_names):
-    if lib_info == None or package != lib_info.package_name:
-      args.add(["-package", package])
+  for package in set.to_list(build_info.package_ids):
+    if lib_info == None or package != lib_info.package_id:
+      args.add(["-package-id", package])
 
   for cache in set.to_list(build_info.package_caches):
     args.add(["-package-db", cache.dirname])
@@ -171,9 +171,9 @@ def _haskell_doctest_aspect_impl(target, ctx):
     args.add(["-package", prebuilt_dep])
 
   # Expose all bazel dependencies
-  for package in set.to_list(build_info.package_names):
-    if lib_info != None or package != lib_info.package_name:
-      args.add(["-package", package])
+  for package in set.to_list(build_info.package_ids):
+    if lib_info != None or package != lib_info.package_id:
+      args.add(["-package-id", package])
 
   for cache in set.to_list(build_info.package_caches):
     args.add(["-package-db", cache.dirname])

haskell/providers.bzl

@@ -1,7 +1,7 @@
 HaskellBuildInfo = provider(
   doc = "Common information about build process: dependencies, etc.",
   fields = {
-    "package_names": "Set of all package names of transitive dependencies.",
+    "package_ids": "Set of all package ids of transitive dependencies.",
     "package_confs": "Set of package .conf files.",
     "package_caches": "Set of package cache files.",
     "static_libraries": "Ordered collection of compiled library archives.",
@@ -15,7 +15,7 @@ HaskellBuildInfo = provider(
 HaskellLibraryInfo = provider(
   doc = "Library-specific information.",
   fields = {
-    "package_name": "Package name, usually of the form name-version.",
+    "package_id": "Package name, usually of the form name-version.",
     "import_dirs": "Import hierarchy roots.",
     "exposed_modules": "Set of exposed module names.",
     "other_modules": "Set of non-public module names.",

tests/package-name/BUILD

@@ -0,0 +1,24 @@
+# Tests around our use of package names.
+package(default_testonly = 1)
+
+load(
+  "@io_tweag_rules_haskell//haskell:haskell.bzl",
+  "haskell_library",
+  "haskell_test",
+)
+
+haskell_library(
+    # A package with the character "Z" in it shouldn't change its name
+    name = "lib-aZ",
+    srcs = ["Lib.hs"],
+    prebuilt_dependencies = ["base"],
+    version = "1.2.3.4",
+)
+
+haskell_test(
+    name = "bin",
+    srcs = ["Main.hs"],
+    main_file = "Main.hs",
+    prebuilt_dependencies = ["base"],
+    deps = [":lib-aZ"],
+)

tests/package-name/Lib.hs

@@ -0,0 +1,4 @@
+module Lib (foo) where
+
+foo :: Integer
+foo = 42

tests/package-name/Main.hs

@@ -0,0 +1,32 @@
+{-# LANGUAGE CPP #-}
+{-# LANGUAGE PackageImports #-}
+module Main (main) where
+
+import Control.Monad (when)
+
+-- Test the PackageImports extension.
+import "lib-aZ" Lib (foo)
+
+-- Test macros that GHC creates automatically based on the package version.
+versionHighEnoughTest, versionTooLowTest :: Bool
+#if MIN_VERSION_lib_aZ(1,2,3)
+versionHighEnoughTest = True
+#else
+versionHighEnoughTest = False
+#endif
+
+#if MIN_VERSION_lib_aZ(1,2,4)
+versionTooLowTest = False
+#else
+versionTooLowTest = True
+#endif
+
+check :: (Show a, Eq a) => a -> a -> IO ()
+check x y = when (x /= y) $ error $ "Failed check: " ++ show (x, y)
+
+main :: IO ()
+main = do
+    check foo 42
+    check VERSION_lib_aZ "1.2.3.4"
+    check versionHighEnoughTest True
+    check versionTooLowTest True

tests/repl-targets/BUILD

@@ -18,10 +18,17 @@ haskell_library(
   visibility = ["//visibility:public"],
 )
 
+haskell_library(
+    name = "QuuxLib",
+    srcs = ["QuuxLib.hs"],
+    prebuilt_dependencies = ["base"],
+)
+
 haskell_binary(
   name = "hs-bin",
   srcs = ["Quux.hs"],
   main_file = "Quux.hs",
+  deps = [":QuuxLib"],
   prebuilt_dependencies = ["base"],
   visibility = ["//visibility:public"],
 )

tests/repl-targets/Quux.hs

@@ -1,4 +1,6 @@
 module Main (main) where
 
+import QuuxLib (message)
+
 main :: IO ()
-main = putStrLn "Hello GHCi!"
+main = putStrLn message

tests/repl-targets/QuuxLib.hs

@@ -0,0 +1,4 @@
+module QuuxLib (message) where
+
+message :: String
+message = "Hello GHCi!"