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

Full CC/Haskell/CC sandwich using CC Skylark API #831

Merged
merged 2 commits into from
Apr 16, 2019
Merged

Conversation

mboes
Copy link
Member

@mboes mboes commented Apr 16, 2019

We had the full sandwich previously, but it relied on two wrapper
rules, cc_haskell_import and haskell_cc_import along with various
hacks in order to do the job. Both of those rules are now deprecated.
Haskell rules now return a CcInfo provider the way any cc_library
does.

We had the full sandwich previously, but it relied on two wrapper
rules, `cc_haskell_import` and `haskell_cc_import` along with various
hacks in order to do the job. Both of those rules are now deprecated.
Haskell rules now return a `CcInfo` provider the way any `cc_library`
does.
@mboes mboes requested a review from guibou April 16, 2019 04:34
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.

That's great! I'm all for it.

Though, previously CcInfo in dep would never be True for haskell_library targets and the like. Now it is. I'm not sure about the implications of this. E.g. in cc_interop_info Haskell libraries will now be included in the resulting CcInfo. That may not be what we want. There might be more occurrences of this in the code base.

Haskell and CC libraries now both return a `CcInfo` provider. But the
CC interop machinery was designed to gather data about CC libraries
specifically, even though in principle there should be little
difference. Out of precaution, ahead of a refactor getting rid of the
interop machinery altogether, we gather `CcInfo`'s that do not have
a `HaskellBuildInfo`.
@mboes
Copy link
Member Author

mboes commented Apr 16, 2019

I have pushed a new commit excluding haskell libraries from the interop logic, out of precaution. The interop logic should really just go away at this point. We should be treating Haskell and CC libraries uniformly as far as linking is concerned. But that will be a pretty massive refactor - for later PR's.

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

@mboes mboes merged commit 9caec20 into master Apr 16, 2019
@mboes mboes deleted the cc-api-full-sandwich branch April 16, 2019 14:06
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