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

bazel-runfiles package doesn’t return results correctly when invoked from Template Haskell #1337

Closed
patrickt opened this issue May 20, 2020 · 5 comments

Comments

@patrickt
Copy link

patrickt commented May 20, 2020

Describe the bug
I’m trying to build some source that accesses filesystem locations and generates code based on these files. (Gnarly, I know.) I ran into #1246, which interfered with our approach up until now (using cabal’s data-files), so I thought I’d try the haskell_library function’s data field, accessing the paths with the bazel-runfiles package. Unfortunately, though this works inside REPLs and executables, it doesn’t appear to work from Template Haskell splices. The call to getExecutableName here returns the path to the relevant ghc executable, which interferes when lookup up runfiles and their manifests.

To Reproduce
Call Runfiles.rlocation from a Template Haskell splice and observe the resulting error message.

Expected behavior
A clear and concise description of what you expected to happen.

Environment

  • OS name + version: macOS catalina
  • Bazel version: 3.10
  • Version of the rules: 0.12

Additional context
I’m probably going to work around this with file-embed for the time being, but I figured I’d file it for posterity.

@aherrmann
Copy link
Member

aherrmann commented May 22, 2020

This is an interesting use-case. With haskell_library|binary|test you can use the extra_srcs attribute to make data files available during compile time. The test-suite contains an example that uses such an extra source file with template Haskell. The example hard-codes the source file path for simplicity, however you can avoid that using Bazel's location expansion and CPP. E.g. something along the lines of

BUILD.bazel

haskell_library(
    name = "lib",
    srcs = ["Lib.hs"],
    extra_srcs = ["data-file"],
    compiler_flags = ['-DDATA_FILE="$(execpath :data-file)"'],
)

Lib.hs

{-# LANGUAGE CPP #-}
{-# LANGUAGE TemplateHaskell #-}

module Lib where

import Language.Haskell.TH
import Language.Haskell.TH.Syntax

embedFile :: Q Exp
embedFile path = do
  str <- runIO (readFile DATA_FILE)
  addDependentFile DATA_FILE
  [| str |]

The runfiles mechanism as it is currently implemented does not work for template Haskell at compile time. As you point out getExecutableName points to the compiler, but also the runfiles tree of deps is not an input to the compilation action. We wouldn't want to change this for the deps attribute as it would introduce (in most cases) unnecessary build dependencies, and even break some use-cases.

An idea that has been floating around is to have a separate attribute for template Haskell dependencies th_deps. This would allow us to not include .a/.so of regular deps into the build action, thereby thinning the dependency graph, and we could potentially include runfiles of th_deps into the build action similar to how we already do for preprocessors or plugins. That could make it possible to use runfiles in template Haskell at compile time.

However, given the current state, I would recommend extra_srcs for haskell_library|binary|test and file-embed for Cabal packages.

@patrickt
Copy link
Author

patrickt commented Jun 26, 2020

Using the preprocessor trick worked great!

I think a th_deps attribute would be pretty neat, especially given our very specific TH requirements.

@aherrmann
Copy link
Member

aherrmann commented Jul 1, 2020

Using the preprocessor trick worked great!

I'm glad to hear it 🎉

I think a th_deps attribute would be pretty neat, especially given our very specific TH requirements.

I've opened a tracking issue in #1382.

@mboes
Copy link
Member

mboes commented Aug 5, 2020

Thanks @patrickt for helping us document in this issue tracker yet another reason for thdeps / compile_deps being a thing we want. :)

Should we close this issue in favour of #1382, or is there shorter term fix we could shoot for?

@patrickt
Copy link
Author

patrickt commented Aug 28, 2020

I think we can close this in favor of #1382!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants