google / hrepl

Interactive development for Bazel/Haskell rules
Apache License 2.0
47 stars 10 forks source link

.HaskellCompile.pb: openBinaryFile: does not exist #10

Open aherrmann opened 4 years ago

aherrmann commented 4 years ago

hrepl attempts to build the haskell_compile_info output group of non-Haskell targets even if they are not explicitly listed on the command-line.

This issue can be reproduced on the daml repository as follows:

$ hrepl //compiler/damlc:damlc-bootstrap  //compiler/damlc/stable-packages:generate-stable-package
hrepl: /home/aj/.cache/bazel/_bazel_aj/f6afc9f2c21b16cf433d30b51afb8d67/execroot/com_github_digital_asset_daml/bazel-out/k8-opt/bin/compiler/damlc/stable-packages/gen-stable-packages.HaskellCompile.pb: openBinaryFile: does not exist (No such file or directory)

Both targets that are specified on the command line are haskell_binary targets. The failing target //compiler/damlc/stable-packages:gen-stable-packages is a genrule target.

Setting the --show-commands flag shows that that target first appears after the first allpaths query:

Running: bazel info bazel-bin --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'
Running: bazel cquery '(//compiler/damlc/stable-packages:generate-stable-package + //compiler/damlc:damlc-bootstrap)' --build_manual_tests --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'
Running: bazel cquery 'let ts = (//compiler/damlc:damlc-bootstrap + //compiler/damlc/stable-packages:generate-stable-package) in allpaths($ts, $ts)' --build_manual_tests --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'
Running: bazel info execution_root --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'
Running: bazel cquery 'let ts = (//compiler/damlc/stable-packages:generate-stable-package + (//compiler/damlc/stable-packages:gen-stable-packages + (//compiler/damlc/stable-packages:daml-stdlib/DA-Validation-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Time-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Semigroup-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-NonEmpty-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Monoid-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Logic-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Internal-Template.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Internal-Down.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Internal-Any.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Date-Types.dalf + (//compiler/damlc/stable-packages:daml-prim/GHC-Types.dalf + (//compiler/damlc/stable-packages:daml-prim/GHC-Tuple.dalf + (//compiler/damlc/stable-packages:daml-prim/GHC-Prim.dalf + (//compiler/damlc/stable-packages:daml-prim/DA-Types.dalf + (//compiler/damlc/stable-packages:daml-prim/DA-Internal-PromotedText.dalf + (//compiler/damlc/stable-packages:daml-prim/DA-Internal-Erased.dalf + (//compiler/damlc/stable-packages:stable-packages + //compiler/damlc:damlc-bootstrap))))))))))))))))))) in (kind('\''haskell_library|haskell_proto_library|haskell_toolchain_library|haskell_cabal_library'\'', deps($ts, 1)) - $ts)' --build_manual_tests --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'
Running: bazel cquery 'let ts = (//compiler/damlc/stable-packages:generate-stable-package + (//compiler/damlc/stable-packages:gen-stable-packages + (//compiler/damlc/stable-packages:daml-stdlib/DA-Validation-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Time-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Semigroup-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-NonEmpty-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Monoid-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Logic-Types.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Internal-Template.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Internal-Down.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Internal-Any.dalf + (//compiler/damlc/stable-packages:daml-stdlib/DA-Date-Types.dalf + (//compiler/damlc/stable-packages:daml-prim/GHC-Types.dalf + (//compiler/damlc/stable-packages:daml-prim/GHC-Tuple.dalf + (//compiler/damlc/stable-packages:daml-prim/GHC-Prim.dalf + (//compiler/damlc/stable-packages:daml-prim/DA-Types.dalf + (//compiler/damlc/stable-packages:daml-prim/DA-Internal-PromotedText.dalf + (//compiler/damlc/stable-packages:daml-prim/DA-Internal-Erased.dalf + (//compiler/damlc/stable-packages:stable-packages + //compiler/damlc:damlc-bootstrap))))))))))))))))))) in (kind(proto_library, deps(kind(haskell_proto_library, $ts), 1)) - $ts)' --build_manual_tests --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'
Running: bazel info bazel-bin --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'
Running: bazel build '--show_result=0' '--output_groups=haskell_library_info,haskell_transitive_deps' '//libs-haskell/da-hs-base:da-hs-base' '//compiler/damlc/daml-lf-conversion:daml-lf-conversion' '@stackage//:optparse-applicative' '//compiler/daml-lf-proto:daml-lf-proto' '//compiler/daml-lf-ast:daml-lf-ast' '@stackage//:text' '//compiler/damlc:damlc-lib' '@stackage//:bytestring' '@stackage//:base' --nostamp '--workspace_status_command=true' '--curses=yes' '--color=yes'

Indeed, //compiler/damlc:damlc-bootstrap has a data dependency on //compiler/damlc/stable-packages which is a filegroup capturing the outputs of the genrule //compiler/damlc/stable-packages:gen-stable-packages which in turn has a tools dependency on //compiler/damlc/stable-packages:generate-stable-package.

One could argue that it doesn't make sense to try and load both those binaries into one GHCi session. Unfortunately, that doesn't prevent this issue as both haskell_binary targets share common library dependencies. E.g. the following will trigger the same issue

$ hrepl //compiler/damlc:damlc-bootstrap  //compiler/damlc/daml-lf-conversion

where //compiler/damlc/daml-lf-conversion is a haskell_library target.

Unfortunately, I'm not aware of a cquery command to filter out targets that don't provide a certain output group. However, one possible way to avoid this issue would be to filter for Haskell rules before attempting to build the output groups. E.g. kind("haskell_binary|haskell_library|haskell_test", ...) for compile_info and kind("haskell_library|haskell_cabal_library", ...) for library_info. This would potentially also allow users to pass wildcards to hrepl, e.g.

$ hrepl //compiler/damlc/...

hrepl revision 33f879eecd986c92ba67f31aec05e3d7f4fbc025 daml revision 761efbe0856941ab398925846dee0d4bbf0b1811

judah commented 4 years ago

Ah, yeah, this is an unfortunate behavior. What I'd really like is for the allpaths query to only follow regular deps and not other attributes like data or srcs. But I don't see a way to do that with how Bazel currently works.

One possibility I've thought about would be to record more information about the graph of transitive dependencies in HaskellCompileInfo, and do the allpaths computation in Haskell rather than via blaze cquery.

For now, though, the approach you suggested sounds like a reasonable workaround. Would you like to send a patch?

By they way, even if we resolve this issue with genrules, you may have a problem loading everything into gghci at once; GHCi gets confused if you try to give it two source files that both have a "Main" module. In the case you mentioned, it might be fine since generate-stable-packages has a custom main_function.

aherrmann commented 4 years ago

One possibility I've thought about would be to record more information about the graph of transitive dependencies in HaskellCompileInfo, and do the allpaths computation in Haskell rather than via blaze cquery.

That sounds interesting. Could you expand a bit on what you have in mind? I'm not sure I understand how this would work.


For now, though, the approach you suggested sounds like a reasonable workaround. Would you like to send a patch?

I've opened #11 implementing this filter.

By they way, even if we resolve this issue with genrules, you may have a problem loading everything into gghci at once; GHCi gets confused if you try to give it two source files that both have a "Main" module. In the case you mentioned, it might be fine since generate-stable-packages has a custom main_function.

That's a good point. In this particular case we do indeed get away with it. Only :main doesn't work.


11 fixes the .HaskellCompile.pb: openBinaryFile: does not exist error. However, I've noticed that two library dependencies are not picked up for some reason. I.e.

$ hrepl //compiler/damlc:damlc-bootstrap //compiler/damlc/daml-lf-conversion

Fails with "could not load module" errors due to missing -package ghcide and missing -package shake. If I add them explicitly it works:

$ hrepl //compiler/damlc:damlc-bootstrap //compiler/damlc/daml-lf-conversion --show-commands --package @stackage//:ghcide --package @stackage//:shake

I'm not sure what is causing this. In case of ghcide there is only one step in between damlc-bootstrap -> damlc-lib -> ghcide. damlc-bootstrap has no direct dependencies apart from base and damlc-lib, so I wonder why it's only ghcide and shake that are skipped.

judah commented 4 years ago

@aherrmann shake and ghcide are a different rule type, haskell_cabal_library. That's probably why your query isn't picking them up. Can you try adding that (and also haskell_toolchain_library , I suppose) to the filter list?

judah commented 4 years ago

One possibility I've thought about would be to record more information about the graph of transitive dependencies in HaskellCompileInfo, and do the allpaths computation in Haskell rather than via blaze cquery.

That sounds interesting. Could you expand a bit on what you have in mind? I'm not sure I understand how this would work.

The idea is not particularly well-formed yet, to be honest. Here's one possibility: For each Haskell rule, add a haskell_dep_info output group that emits a new DepInfo proto:

message DepInfo {
  repeated string dep_labels = 1;  // Immediate dependencies
}

Then, replace getAllIntermediateTargets (which is currently just an allpaths($targets, $targets)query) with the following algorithm:

  1. Run blaze query "allpaths($targets, $targets)" to get a list of labels.
  2. Run blaze build --output_groups=haskell_dep_info on those labels, and read the DepInfo for each one. Ignore labels that don't have a DepInfo, i.e., that aren't Haskell targets.
  3. Run an "allpaths($targets,$targets)" algorithm manually, using the graph defined by the DepInfo (vertices are labels). This will give us a correct set which is refined from (1).

Some things I'm not sure about;

aherrmann commented 4 years ago

@aherrmann shake and ghcide are a different rule type, haskell_cabal_library. That's probably why your query isn't picking them up. Can you try adding that (and also haskell_toolchain_library , I suppose) to the filter list?

I dug a bit deeper. The issue is not due to the filter that I added in #12. The filter for haskell_binary|haskell_library|haskell_test is only applied to getAllIntermediateTargets. I.e. only affects packages that should be loaded by source. ghcide and shake are external dependencies and should be loaded as compiled packages. (haskell_cabal_library also doesn't return haskell_compile_info)

The compiled dependencies are determined by the query defined in buildDependentPackages. In this case it looks as follows (as shown by --show-commands)

bazel cquery 'let ts = (//compiler/damlc/daml-lf-conversion:daml-lf-conversion + (//compiler/damlc/stable-packages:generate-stable-package + (//compiler/damlc/daml-ide:daml-ide + (//compiler/damlc/daml-compiler:daml-compiler + (//compiler/damlc/daml-ide-core:daml-ide-core + (//compiler/damlc:damlc-lib + //compiler/damlc:damlc-bootstrap)))))) in (kind('\''haskell_library|haskell_proto_library|haskell_toolchain_library|haskell_cabal_library'\'', deps($ts, 1)) - $ts)'

Running this query indeed doesn't include ghcide or shake in the listed dependencies, even though both are direct dependencies of //compiler/damlc:damlc-lib which is included in ts.

It turns out that @stackage//:ghcide and @stackage//:shake are vendored_packages in the corresponding stack_snapshot call. I.e. they are alias rules that point to haskell_cabal_library somewhere else. But the query that hrepl applies doesn't handle alias.

aherrmann commented 4 years ago

Thanks for outlining the DepInfo approach. That looks like it could work, though I haven't thought about the three open questions you raise in the bottom, yet. I think this might also provide a solution to the alias issue I've described above.