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

Rename haskell_import to haskell_toolchain_library #843

Merged
merged 2 commits into from
Apr 29, 2019

Conversation

mboes
Copy link
Member

@mboes mboes commented Apr 25, 2019

This is a substantial breaking change. But a simple one to handle: it
can be done with

sed -i s/^haskell_import/haskell_toolchain_library/ **/BUILD{,.bazel}

everywhere. I considered marking this as deprecated instead, but that
won't help much: the deprecation spam will make things unbearable
without switching to the new name anyways.

The motivation for this change can be found in #440 and #838.
haskell_import was always poorly named, because it did not
correspond to precedent in the CC and Java rules. Whereas those rules
"import" concrete artifacts with some metadata into the dependency
graph, our old haskell_import was merely giving a name in the
dependency graph to stuff that is magically available from somewhere,
namely the GHC boot libraries.

I will be submitting a followup PR to implement a new haskell_import
that is similar in spirit to cc_import and java_import, once
I manage to solve/workaround the two following bugs:

Meanwhile this PR just prepares the ground, by moving the existing
haskell_import out of the way and renaming it to
haskell_toolchain_library.

This is a massive breaking change. But a simple one to handle: it's just

```
sed -i s/^haskell_import/haskell_toolchain_library/ **/BUILD{,.bazel}
```

everywhere. I considered marking this as deprecated instead, but that
won't help much: the deprecation spam will make things unbearable
without switching to the new name anyways.
Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

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

LGTM, I agree for the reasoning about rename vs. deprecate.

This should probably be listed in the changelog.

To clarify, how does this relate to haskell_import_new? It sounds like haskell_import_new could then be replaced by/extended to the new haskell_import as described in this PR's message.

@mboes
Copy link
Member Author

mboes commented Apr 25, 2019

Yes. TBH I don't quite understand haskell_import_new. Maybe @regnat can comment on whether this can replace haskell_import_new, which AFAIK was an experiment that at this point is only used in the rules_haskell test suite. haskell_import_new enables an alternative modular way to interoperate with Nix. I don't know whether we'll be able to recover that (though it's not very usable at the moment because it's very slow).

@mboes mboes requested a review from Profpatsch April 25, 2019 09:03
@mboes mboes merged commit e9d5a99 into master Apr 29, 2019
@mboes mboes deleted the haskell_toolchain_library branch April 29, 2019 11:44
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