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

Toolchain registration macros #610

Merged
merged 9 commits into from
Jan 22, 2019
Merged

Toolchain registration macros #610

merged 9 commits into from
Jan 22, 2019

Conversation

mboes
Copy link
Member

@mboes mboes commented Jan 22, 2019

It was previously hard to let the user select between a Nixpkgs based
toolchain and a binary distributions based one. Most
projects (including rules_haskell) were essentially hardcoding this
choice for the user. It is now much easier to do. The trick is to
supply three new workspace macros:

haskell_register_toolchains()    # Same as the second one.
haskell_register_ghc_bindists()
haskell_register_ghc_nixpkgs()

The second function above defines and registers one toolchain per
binary distribution. So e.g. one for Windows, one for Linux etc. The
third one currently defines exactly one toolchain, which matches the
constraints of the host platform. It might define more in the
future (in particular cross-compilers).

As a project author, you are free to just the second one or just the
third one, or both. While the Nixpkgs toolchain was always selected if
it existed previously, now it will only be selected if the user
explicitly sets the host platform in a certain way. See the
documentation about the details.

mboes added 2 commits January 22, 2019 15:32
The rules exported by this file all pertain to Nixpkgs, not Nix
directly.
@mboes mboes requested review from nmattia and guibou January 22, 2019 15:48
Copy link
Contributor

@nmattia nmattia left a comment

Choose a reason for hiding this comment

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

Very nice!

@@ -299,3 +308,10 @@ def haskell_toolchain(
exec_compatible_with = exec_compatible_with,
target_compatible_with = target_compatible_with,
)

def haskell_register_toolchains(version):
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to update the CHANGELOG, since AFAICT the default changed from nixpkgs to bindist

def _ghc_nixpkgs_toolchain_impl(repository_ctx):
# These constraints might look tautological, because they always
# match the host platform if it is the same as the target
# platform. But they are important to state because Bazel
Copy link
Contributor

Choose a reason for hiding this comment

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

🤦‍♂️

mboes added 7 commits January 22, 2019 20:00
This function registers all known GHC binary distributions as
toolchains. Each toolchain has appropriate exec and target constraints
defined, so that on any given platform, only one bindist will be
selected.
This works the same as `haskell_register_ghc_bindists`, except that we
are registering a Nixpkgs package, rather than a bindist. Nixpkgs
toolchains are now gated with the `//haskell/platforms:nixpkgs` config
value. To turn it on, you need to specify the `linux_x86_64_nixpkgs`
or `darwin_x86_64_nixpkgs` platforms.
@mboes mboes force-pushed the multi-toolchain-register branch from ec45656 to 19f51e9 Compare January 22, 2019 19:00
@mboes mboes merged commit e46991e into master Jan 22, 2019
@mboes mboes deleted the multi-toolchain-register branch January 22, 2019 19:00
@@ -264,6 +226,9 @@ rule_test(
rule = "//tests/java_classpath",
)

# Keep in sync with test_ghc_version in WORKSPACE.
Copy link
Contributor

Choose a reason for hiding this comment

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

Conflict with #614, be careful.

)

haskell_register_ghc_bindists(version = test_ghc_version)
Copy link
Contributor

Choose a reason for hiding this comment

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

No compiler_flags or haddock_flags for that one?

Copy link
Member Author

Choose a reason for hiding this comment

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

These should be added, for symmetry with haskell_register_ghc_nixpkgs. They weren't needed for the MVP though (since we currently only use bindists on windows and on windows we currently don't want to set any special flags - until we can propertly support them).

@guibou guibou mentioned this pull request Jan 22, 2019
mboes added a commit that referenced this pull request Jan 23, 2019
Since #610, the Nixpkgs toolchain is no longer the default in the
rules_haskell project (users can still keep it the default in their
own projects). The README clarifies this.

Closes #617.

[skip ci]
gdeest pushed a commit that referenced this pull request Jan 28, 2019
Since #610, the Nixpkgs toolchain is no longer the default in the
rules_haskell project (users can still keep it the default in their
own projects). The README clarifies this.

Closes #617.

[skip ci]
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.

3 participants