Remove HaskellPrebuiltPackageInfo

[thufschmitt]

(depends on tweag/rules_nixpkgs#29)

Solve #440 by provinding an alternative haskell_import rule in haskell/import.bzl which directly generates an HaskellLibraryInfo, HaskellBuildInfo and HaddockInfo for the dependency.

There is also a new set of rules in haskell/nix.bzl to generate these rules from a nixpkgs definition with just a bit of boilerplate (see the documentation of haskell_nixpkgs_packages).

Apart from the testsuite, I've succesfully tried it on a mid-sized haskell repository (apart from doctest, but it wasn't working before either because of #367, so nothing new on this side)

@mrkkrp since you proposed some help, there's probably a lot to say regarding this PR as it is my first real skylark work and I'm not familiar with the internals of rules_haskell

[thufschmitt]

Hum, an unpleasant side-effect of this (more precisely of the gen_packages_list rule) is that we now need nix-build in path to compute the build graph (because we load the result of a nixpkgs_package in the WORKSPACE file). I don't know how much this in undesirable.

(the gen_packages_list function is technically optional as we could replace its result by a hand-written list of the haskell packages needed, but this list needs to be closed by transitive dependency so maintaining it by hand is probably gonna be quite awful)

[Profpatsch]

I was able to review the first commits, but got stuck at the “Get Haddock to work” commit.

[Profpatsch]

What about the haskell_import rule from haskell/haskell.bzl? Shouldn’t it be removed?

[thufschmitt]

Eventually yes, though we probably should have a way to generate the correct (new) haskell_import statements for other inputs than nix (like ghc bindist or an external ghc) first.

[thufschmitt]

@Profpatsch thanks to the early review.
I've rebased the PR to have more meaningful commits (I tried not to touch the three first too much since you already reviewed them).

[lunaris]

Can't quite get this to work. With WORKSPACE:

workspace(name = "test")

# allow-multiple-repositories branch's most-recent commit at time of writing
http_archive(
    name = "io_tweag_rules_nixpkgs",
    strip_prefix = "rules_nixpkgs-b81967b6ccac855f71151785dbb9fe56f81a0562",
    urls = ["https://github.com/tweag/rules_nixpkgs/archive/b81967b6ccac855f71151785dbb9fe56f81a0562.tar.gz"],
)

load("@io_tweag_rules_nixpkgs//nixpkgs:nixpkgs.bzl", "nixpkgs_git_repository", "nixpkgs_package")

# Commit in this PR before documentation fix
http_archive(
  name = "io_tweag_rules_haskell",
  strip_prefix = "rules_haskell-4666061507b88f6e0934e9d1db04d95fccd4b2b1",
  urls = ["https://github.com/tweag/rules_haskell/archive/4666061507b88f6e0934e9d1db04d95fccd4b2b1.tar.gz"],
)

load("@io_tweag_rules_haskell//haskell:repositories.bzl", "haskell_repositories")
haskell_repositories()

# Recent Nixpkgs commit
nixpkgs_git_repository(
  name = "nixpkgs",
  revision = "d22e22e2dd83510ca11ab6966cacd2ee929a2d73",
)

nixpkgs_package(
  name = "ghc",
  repositories = {"nixpkgs": "@nixpkgs"},
  attribute_path = "haskell.compiler.ghc822",
)

register_toolchains("//:ghc")

load("@io_tweag_rules_haskell//haskell:nix.bzl", "haskell_nixpkgs_packages", "gen_packages_list")

gen_packages_list(
  name = "hackage_packages",
  repositories = {"nixpkgs": "@nixpkgs"},
  nix_file = "//:haskell-packages.nix",
  base_attribute_path = "haskellPackages",
)

load("@hackage_packages//:all-haskell-packages.bzl", "packages")

haskell_nixpkgs_packages(
  name = "hackage",
  packages = packages,
  repositories = {"nixpkgs": "@nixpkgs"},
  nix_file = "haskell-packages.nix",
  base_attribute_path = "haskellPackages",
)

BUILD:

load("@io_tweag_rules_haskell//haskell:haskell.bzl", "haskell_toolchain")

haskell_toolchain(
  name = "ghc",
  version = "8.2.2",
  tools = "@ghc//:bin",
)

haskell-packages.nix:

with import <nixpkgs> {};

let
  genBazelBuild = callPackage <bazel_haskell_wrapper> { };
in {
  haskellPackages = genBazelBuild haskell.packages.ghc822;
}

shell.nix:

{ pkgs ? import ./nixpkgs.nix {} }:

with pkgs;
with darwin.apple_sdk.frameworks;

# XXX On Darwin, workaround
# https://github.com/NixOS/nixpkgs/issues/42059. See also
# https://github.com/NixOS/nixpkgs/pull/41589.
let
  cc = stdenv.mkDerivation {
    name = "cc-wrapper-bazel";
    buildInputs = [ stdenv.cc makeWrapper ];
    phases = [ "fixupPhase" ];
    postFixup = ''
      mkdir -p $out/bin
      makeWrapper ${stdenv.cc}/bin/clang $out/bin/clang \
        --add-flags "-isystem ${llvmPackages.libcxx}/include \
                    -F${CoreFoundation}/Library/Frameworks \
                    -F${CoreServices}/Library/Frameworks \
                    -F${Security}/Library/Frameworks \
                    -F${Foundation}/Library/Frameworks \
                    -L${libcxx}/lib \
                    -L${darwin.libobjc}/lib"
    '';
  };
  mkShell = pkgs.mkShell.override {
    stdenv = if stdenv.isDarwin then overrideCC stdenv cc else stdenv;
  };
in
  mkShell {
    buildInputs = [
      binutils
      git
      go
      nix
      which
      python
    ]
    # TODO use Bazel from Nixpkgs even on Darwin. Blocked by
    # https://github.com/NixOS/nixpkgs/issues/30590.
    ++ lib.optional stdenv.isLinux bazel;
  }

nixpkgs.nix:

# Keep this value in sync with `WORKSPACE`
import (fetchTarball "https://github.com/NixOS/nixpkgs/archive/d22e22e2dd83510ca11ab6966cacd2ee929a2d73.tar.gz")

I get:

➜  test git:(master) ✗ nix-shell --pure --run "bazel build --verbose_failures @hackage//:containers"                      
INFO: Build options have changed, discarding analysis cache.
ERROR: /.../.cache/bazel/_bazel_.../.../external/hackage/BUILD:15239:1: no such package '@hackage-containers//': Not a regular file: /.../test/external/haskell-packages.nix and referenced by '@hackage//:containers'
ERROR: Analysis of target '@hackage//:containers' failed; build aborted: Analysis failed
INFO: Elapsed time: 6.827s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (3 packages loaded)
➜  test git:(master) ✗ nix-shell --pure --run "bazel build --verbose_failures @hackage-containers//..."
ERROR: Not a regular file: /.../test/external/haskell-packages.nix
INFO: Elapsed time: 0.208s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)

The issue seems to be the /external/ being placed before haskell-packages.nix when it's being referenced, but digging through the code in this PR I can't see where that might be happening/whether I'm misnaming something to cause this. Any pointers?

[thufschmitt]

@lunaris That's another typo in the example, it should be nix_file = "//:haskell-packages.nix" and not nix_file = "haskell-packages.nix at line 51 of the WORKSPACE (I just tried your example with this change and it's enough to make it build)

[lunaris]

Awesome -- it works! I'll put some testing in now but I think this is sweet and it's just a matter of what the final version should be (e.g. the _new stuff and bikeshedding -- should gen_packages_list be haskell_gen_packages_list or the like). And perhaps (though I've built locally so this may already be there) some more documentation in the Skydoc vein.

[lunaris]

Alas, it seems there is something more to the subpackage issue. If you take the files I posted earlier and add something like this to BUILD:

package(default_visibility = ["//visibility:public"])

load("@io_tweag_rules_haskell//haskell:haskell.bzl", "haskell_library")
load("@io_tweag_rules_haskell//haskell:haskell.bzl", "haskell_toolchain")

haskell_toolchain(
  name = "ghc",
  version = "8.4.3",
  tools = "@ghc//:bin",
)

haskell_library(
  name = "pandoc-test",
  src_strip_prefix = "pandoc-test",
  srcs = glob(["pandoc-test/**/*.hs"]),
  deps = [
    "@hackage//:base",
    "@hackage//:pandoc",
  ],
)

with a pandoc-test/Main.hs containing anything, e.g.:

module Main (main) where

main :: IO ()
main = putStrLn "Hello"

Then this will fail to build due to not being able to satisfy our old friend haddock-library. Adding @hackage//:haddock-library to the deps doesn't help either -- the key is that it can't satisfy haddock-library-...-attoparsec.

[thufschmitt]

I still can't build your example (because haddock-libaryr doesn't build), but I can reproduce the bug by replacing ghc822 by ghc843. This comes from the fact that multi-library packages export one package.conf per library, and I was only importing one into bazel. I've pushed a commit which fixes it (at least your example builds fine with ghc843) and I'll add a regression test for it too

[lunaris]

Yes, works for me too!

[mrkkrp]

So this is still blocked at least on tweag/rules_nixpkgs#29 ?

[thufschmitt]

@mrkkrp yes, still. Though tweag/rules_nixpkgs#29 is a much simpler change so should be easy to merge if we think that this one is good-to-go

[Profpatsch]

Phew, this took a while. Looks quite okay to me, though I can’t say I understood it all. There might be a few outdated comments, because I went through commit-by-commit, they should be relevant in any case though.

I like the deduplication when importing nix packages from one source!

[thufschmitt]

I've just rebased on top of master, there wasn't any major conflict, just some package renamings in tests.

I'm wondering if there's any possibility that it affects Bazel's remote caching support

I don't see what in this change could cause it, but who knows…

does this interact with b58050c so that we don't get CPP macros generated any more

In theory it shouldn't since the choice of generating or not the macro is done via a ghc flag passed during the build of the package and we don't touch these. Did you have any issue with it?

[lunaris]

After rebasing, two packages that had built before that use -XCPP (bloodhound and hs-webdriver) refused to do so as CPP cited it didn't understand #if !MIN_VERSION_base(4,8,0) and the like. Naturally when I rebased I pulled in stuff from master too, but the commit in question seems to indicate macros are only generated if a version is passed and I wondered what with pinning things to Nixpkgs that maybe we'd stopped doing that in this PR.

[thufschmitt]

Hm… it's actually surprising that it worked before because I forgot to forward the include-dirs which are needed for these macros.

If it's really the problem, it should be easy to fix. Do you by any chance have a small repro handy?

[lunaris]

@regnat Reproducing repo for the CPP problem at https://github.com/lunaris/bazel-example (master); hopefully that helps.