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

Extra source files #292

Merged
merged 3 commits into from
Jun 5, 2018
Merged

Extra source files #292

merged 3 commits into from
Jun 5, 2018

Conversation

mrkkrp
Copy link
Member

@mrkkrp mrkkrp commented Jun 4, 2018

Close #291.

This makes it possible to compile TH code that references external files with arbitrary extensions, see the test. Unfortunately this still doesn't work out-of-the-box with Hackage packages because the path to files should include package names. Not a problem for new code though.

Copy link
Collaborator

@judah judah left a comment

Choose a reason for hiding this comment

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

LGTM with one documentation suggestion

@@ -46,6 +46,10 @@ _haskell_common_attrs = {
allow_files = FileType([".hs", ".hsc", ".chs", ".lhs", ".hs-boot", ".lhs-boot", ".h"]),
doc = "Haskell source files.",
),
"extra_srcs": attr.label_list(
allow_files = True,
doc = "Extra (non-Haskell) source files that will be needed at compile time (e.g. by TH).",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: clarify these docs by expanding "TH" to "Template Haskell".

embedFile :: FilePath -> Q Exp
embedFile path = do
str <- runIO (readFile path)
addDependentFile path
Copy link
Collaborator

Choose a reason for hiding this comment

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

A question: I guess addDependentFile is mostly only needed for reloading in the REPL, right? Otherwise I believe Bazel would track the dependency on the input file automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well I just added it because it was the right thing to do. It doesn't hurt in either case, does it?

@mrkkrp mrkkrp merged commit 87d2928 into master Jun 5, 2018
@mrkkrp mrkkrp deleted the extra-source-files branch June 5, 2018 06:54
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