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

Merge hazel into rules haskell #733

Merged
merged 138 commits into from
Mar 27, 2019
Merged

Merge hazel into rules haskell #733

merged 138 commits into from
Mar 27, 2019

Conversation

Profpatsch
Copy link
Contributor

@Profpatsch Profpatsch commented Mar 7, 2019

This merges all commits from https://github.com/DACH-NY/hazel/commits/da-master into rules_haskell.
So far, only the mechanical work has been done (moving files and merging the branches).

There are a few unsolved questions before we can merge:

  • Should we keep the hazel WORKSPACE file, and if yes, which parts
  • How to manage attribution and licenses
    The fork is from Digital Asset, while the original work and “official upstream” is by FormationAI. We chose the fork because it is compatible with current rules_haskell.

cc @mboes @judah @thumphries @aherrmann @gdeest @nmattia

judah and others added 30 commits April 13, 2018 13:52
It is able to download and build packages automatically from Hackage.  So far
it compiles a small but nontrivial number of packages; see
`.circleci/config.yaml` for examples.

See the Readme for more details on use.
Also support for:
- `*.lhs` and `*.hsc` files
- Main files with ".." in their name
- Better normalisation in general
- Duplicate dependencies and files
There's more potential cleanup, but this seemed like a good milestone.

Modifies the previous version of Hazel to use `cabal2build`'s approach:
Have a Haskell file that that reifies the `.cabal` structure to `*.bzl`,
but otherwise write all logic directly as a Bazel macro.  (The previous
version had a more awkward round tripp between Haskell and Bazel.)

Supports:
- libraries and executables
- boot files
- preprocessors (happy/alex)
- data files
- `./configure` scripts
- `Paths_*` modules

In particular, builds a few nontrivial packages:
- `aeson`
- `language-c` (codegen from `happy`/`alex`)
- `lens` (dependencies use `Paths_` modules)
- `network` (`./configure` script)

See `third_party/cabal2bazel/Readme.md` for a list of changes from the
original `cabal2bazel` sources.

Note: this PR manually checks in the data-files that Happy and Alex use
and which are generated automatically by their custom `Setup.hs` scripts.
Figuring out a better solution is TODO; see `templates/Readme.md` for more
details.
Something changed in its CI environment and now several things don't work
anymore.

- Pin the docker image tag and the nixpkgs revision.  (Not sure if that was
  actually the cause of the breakage; things didn't change afterwards.)
- "Brace expansions" aren't working.  Take the opportunity to
  move our list of "golden" packages to a separate file.
- The network package doesn't build anymore.  Temporarily remove it from
  that list.
It turns out they're generated automatically by GHC since version 8.0,
as long as we pass the right package name and version.  That's now possible
as of #246.  (So bump `rules_haskell` accordingly.)

Also add network back since the new `rules_haskell` handles `cc_library` `defines` attributes better.
Make it more clear which parts are about setting up a new project, versus
writing rules that use the autogenerated repositories.
* Allow to provide extra libs to auto-generated packages

* Simplify the solution according to Judah's feedback
That package has extra-src-files with `.` in their path, which Bazel
doesn't allow:
```
extra-source-files:   ./cbits/rdrand.c, ./cbits/rdrand.h, README.md
```
A package may list them twice with different version ranges.  Fr example, in `http-client`:

```
build-depends:
   ...
   , network >= 2.4
if flag(network-uri)
    build-depends: network >= 2.6, network-uri >= 2.6
else
     build-depends: network < 2.6
```
The package expects to be able to import a header from the `unix` package's
`install-includes`, which are part of the GHC distribution.

Error:
```
ERROR:
/private/var/tmp/_bazel_judah.jacobson/0dd0c61b72eb0880ee09f3b1c25cfb68/external/haskell_unix_compat/BUILD:14:1:
C++ compilation of rule '@haskell_unix_compat//:unix-compat-cbits' failed
(Exit 1)
In file included from external/haskell_unix_compat/cbits/HsUnixCompat.c:1:
external/haskell_unix_compat/include/HsUnixCompat.h:1:10: fatal error:
'HsUnixConfig.h' file not found
         ^~~~~~~~~~~~~~~~
```

This solution is a little hacky because it hard-codes the logic
specifically for `unix` in `BUILD.ghc` and `cabal_package.bzl`.  It should
be possible to add more install-includes in the future, though at first glance
I couldn't find any other packages that would need it.

Honestly, long-term we should probably replace `BUILD.ghc` with a more
principled repository rule.  But hopefully this change will give us some more
mileage out of the current approach.
The current logic was broken in a previous refactor that removed
the trailing `"-lib"`.  Add `pretty-show` to the CI tests.

It turns out that we can just remove the special case altogether,
since it's fine to refer to a target in the same BUILD file by its
absolute path.
- unix-time declares one of the `./configure` script's outputs in `extra-tmp-files`
- old-time's C library expects `__GLASGOW_HASKELL__` to be set.
By default, GHC loads the user database at `~/.ghc/.../package.conf.d`.
This makes the build less hermetic.  For example, my laptop has `Cabal-2.2`
installed in the user DB, even though we're using a GHC with an older version
of Cabal, and this broke the build because cabal2build doesn't
work yet with that version.

I'll make another PR to get cabal2bazel building with newer
versions of Cabal.
* Expose the issue with conflicting module names

* Hide “other-modules” automatically
* Allow to fetch GitHub repos via SSH

* Try to use newer nixpkgs
Also adds a rule for ghc-paths.  I left out the `docdir` parameter
since (a) I wasn't sure how to get it, and (b) it's only really useful for
`haddock`, but we already use the one distributed with GHC.
The `native.new_http_archive` rule has been deprecated as of bazel-0.12.
The current rules_haskell setup is heavily Nix-based (including the Bazel
binary) and its usability relies on having Cachix or some other custom Nix
cache.

- Switch to an Ubuntu-based container
- Add and use cc_configure.bzl.
- Update the versions of nixpkgs for consistency
- Use the FormationAI/rules_haskell fork, temporarily
Uses the functionality from #337.

Hazel now generates repository rules for "core" GHC packages like `base`
and `bytestring`.  Thus, rules that uses Hazel shouldn't need to specify
the `prebuilt_dependencies` attribute anymore.

Also change all references to "prebuilt" to "core packages", to match the
terminology in stack build plans.
@gdeest gdeest force-pushed the merge-hazel-repo branch from 5cb848e to c8535a2 Compare March 25, 2019 16:16
hazel/.circleci/config.yml Show resolved Hide resolved
hazel/.gitignore Show resolved Hide resolved
hazel/LICENSE Show resolved Hide resolved
hazel/README.md Show resolved Hide resolved
hazel/test/BUILD Show resolved Hide resolved
hazel/third_party/cabal2bazel/LICENSE Show resolved Hide resolved
hazel/third_party/cabal2bazel/Readme.md Show resolved Hide resolved
hazel/tools/bazel.rc Show resolved Hide resolved
@@ -0,0 +1,28 @@
load("@bazel_tools//tools/cpp:lib_cc_configure.bzl", "get_cpu_value")

default_ghc_workspaces = {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is @ghc? Are these good defaults, are they correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

These defaults expect rules_haskell users to define a @ghc workspace. I think we should rather use the default names for bindists.

build_file = "@ai_formation_hazel//third_party/haskell:BUILD.text-metrics",
)

hazel_custom_package_github(
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work because of the new CI names. Also embedFile should be fixed once and for all.

hazel/repositories.bzl Outdated Show resolved Hide resolved
@nmattia nmattia mentioned this pull request Mar 26, 2019
Copy link
Contributor

@gdeest gdeest left a comment

Choose a reason for hiding this comment

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

I am fine with merging this now, but we should really look into CI run time on Darwin…

@nmattia nmattia changed the title WIP: merge hazel into rules haskell Merge hazel into rules haskell Mar 27, 2019
@Profpatsch
Copy link
Contributor Author

Can you squash all WIP commits? That makes it easier to review.

@Profpatsch
Copy link
Contributor Author

Looks good! Let’s merge once CI is green.

@nmattia nmattia merged commit 75e5b9e into master Mar 27, 2019
@nmattia nmattia deleted the merge-hazel-repo branch March 27, 2019 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.