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

Tree artifact as input for cc_library fails with headers #5092

Closed
marcohu opened this issue Apr 25, 2018 · 10 comments
Closed

Tree artifact as input for cc_library fails with headers #5092

marcohu opened this issue Apr 25, 2018 · 10 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: bug

Comments

@marcohu
Copy link
Contributor

marcohu commented Apr 25, 2018

Using a tree artifact as input for cc_library does not work if the directory contains both sources and headers. If the directory only contains sources, it works fine.

My expectation would be that it works for both cases. The following contrived example demonstrates the behavior:

tree.bzl

def _impl(ctx):
    tree = ctx.actions.declare_directory(ctx.attr.name + ".cc")
    ctx.actions.run_shell(
        inputs = [],
        outputs = [tree],
        arguments = [tree.path],
        progress_message = "Generating cc files into '%s'" % tree.path,
        command = """
mkdir {}
cd {}
touch dummy.h
touch dummy.cpp
""".format(tree.path, tree.path),
    )
    return [DefaultInfo(files=depset([tree]))]

tree = rule(
    implementation = _impl,
)

BUILD

load(":tree.bzl", "tree")

tree(
    name = "gen_tree",
)

cc_library(
    name = "tree",
    srcs = [":gen_tree"]
)

The current bazel@HEAD yields the following exception:

java.lang.RuntimeException: Unrecoverable error while evaluating node 'ActionTemplateExpansionKey{actionLookupKey=//antlr4/Native:Native e5c50b673c5eaa945f0b593ed7a30689 false (1717974475), actionIndex=1}' (requested by nodes 'antlr4/Native/_pic_objs/Native/antlr4/Native/generated //antlr4/Native:Native e5c50b673c5eaa945f0b593ed7a30689 false (1717974475)')
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:438)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:355)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IllegalStateException
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder.getActionName(CppCompileActionBuilder.java:248)
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder.buildAndVerify(CppCompileActionBuilder.java:325)
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionTemplate.createAction(CppCompileActionTemplate.java:120)
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionTemplate.generateActionForInputArtifacts(CppCompileActionTemplate.java:91)
	at com.google.devtools.build.lib.skyframe.ActionTemplateExpansionFunction.compute(ActionTemplateExpansionFunction.java:83)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:360)
	... 4 more
java.lang.RuntimeException: Unrecoverable error while evaluating node 'ActionTemplateExpansionKey{actionLookupKey=//antlr4/Native:Native e5c50b673c5eaa945f0b593ed7a30689 false (1717974475), actionIndex=1}' (requested by nodes 'antlr4/Native/_pic_objs/Native/antlr4/Native/generated //antlr4/Native:Native e5c50b673c5eaa945f0b593ed7a30689 false (1717974475)')
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:438)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:355)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IllegalStateException
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder.getActionName(CppCompileActionBuilder.java:248)
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder.buildAndVerify(CppCompileActionBuilder.java:325)
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionTemplate.createAction(CppCompileActionTemplate.java:120)
	at com.google.devtools.build.lib.rules.cpp.CppCompileActionTemplate.generateActionForInputArtifacts(CppCompileActionTemplate.java:91)
	at com.google.devtools.build.lib.skyframe.ActionTemplateExpansionFunction.compute(ActionTemplateExpansionFunction.java:83)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:360)

@meteorcloudy
Copy link
Member

@mhlopko Can you take a look?

@marcohu
Copy link
Contributor Author

marcohu commented Apr 28, 2018

After careful studying of the documentation, I found a way to make things work. The rule documentation states that output to genfiles is deprecated, except when necessary and header files seem to be this exception.

Splitting the output into bazel-bin for the sources and bazel-genfiles for the headers seems to do the trick.

load("@rules_antlr//antlr:antlr4.bzl", "antlr4", "headers", "sources")

antlr4(
    name = "generated",
    srcs = ["src/antlr4/Hello.g4"],
    language = "Cpp",
)

headers(
    name = "headers",
    srcs = [":generated"],
)

sources(
    name = "sources",
    srcs = [":generated"],
)

cc_library(
    name = "Cpp",
    hdrs = [":headers"],
    srcs = [":sources"],
    includes = ["."],
)

But it feels hacky as there is an extra step necessary to filter the output and there does not seem to be proper API support for directories in both output hierarchies (but ctx.new_file(ctx.genfiles_dir, "") works after some fiddling with the includes of cc_library).

I would therefore appreciate if someone could look into it and provide proper support for this (not exactly uncommon) use case. Thanks.

@hlopko
Copy link
Member

hlopko commented May 28, 2018

I think the problem is that in your previous example you put headers into srcs, which probably breaks, and in the second you put them into hdrs.

As currently implemented c++ rules assume that all files in the directory are source files.

Is my guess making sense to you?

@marcohu
Copy link
Contributor Author

marcohu commented May 28, 2018

It makes perfect sense, but that doesn't help :)

While initially my workaround seemed to work, it really doesn't. While cc_library compiles, trying to consume the lib with cc_binary fails (with 0.13.0).

cc_library(
    name = "lib",
    hdrs = [":headers"],
    srcs = [":sources"],
    includes = ["."],
    deps = ["@antlr4_runtimes//:cpp"],
)

cc_binary(
    name = "Cpp",
    srcs = ["src/main.cpp"],
    includes = ["."],
    deps = [":lib"],
)
ERROR: /home/user/Dev/rules_antlr/examples/antlr4/Cpp/BUILD:29:1: undeclared inclusion(s) in rule '//antlr4/Cpp:Cpp':
this rule is missing dependency declarations for the following files included by 'antlr4/Cpp/src/main.cpp':
  'bazel-out/k8-fastbuild/genfiles/antlr4/Cpp/TLexer.h'
  'bazel-out/k8-fastbuild/genfiles/antlr4/Cpp/TParser.h'

There is mention about missing "generated header propagation" for 0.13.0. Does this apply to this use case?

Right now I'm stuck. Any help would be appreciated. Thanks.

BTW, this is how I split the sources.

        sources = ctx.actions.declare_directory(ctx.attr.name + _extension(ctx.attr.language))
        output_dir = sources.path
        # for C/C++ we must split headers from sources
        if ctx.attr.language == "Cpp" or ctx.attr.language == "C":
            artifacts = ctx.actions.declare_directory(ctx.attr.name + ".antlr")
            # TODO headers currently must be stored below bazel-genfiles
            headers = ctx.new_file(ctx.genfiles_dir, "")
#            headers = ctx.actions.declare_directory(ctx.attr.name + ".hh")
            outputs = [sources, headers, artifacts]
        else:
            outputs = [sources]

The generated files tree looks like this:

bazel-bin
├── antlr4
│   └── Cpp
│       ├── generated.antlr
│       │   ├── TLexer.interp
│       │   ├── TLexer.tokens
│       │   ├── TParser.interp
│       │   └── TParser.tokens
│       └── generated.cc
│           ├── TLexer.cpp
│           ├── TParserBaseListener.cpp
│           ├── TParser.cpp
│           └── TParserListener.cpp
bazel-genfiles
└── antlr4
    └── Cpp
        ├── TLexer.h
        ├── TParserBaseListener.h
        ├── TParser.h
        └── TParserListener.h

@hlopko hlopko self-assigned this Jun 20, 2018
@hlopko hlopko added P2 We'll consider working on this in future. (Assignee optional) and removed under investigation labels Jun 20, 2018
@hlopko
Copy link
Member

hlopko commented Jun 22, 2018

Found the culprit for the original issue (srcs + hdrs together). Fix is in the pipeline. I'll still have to look into the second issue after the first fix it's submitted.

bazel-io pushed a commit that referenced this issue Jun 26, 2018
Before, Bazel expected that it can compile whatever appeared in cc_library.srcs
directory artifacts. That is true for C/C++ source files, and for headers when
the C++ toolchain supported header parsing/processing (which used
CppCompileAction). When the toolchain doesn't support header parsing/processing,
Bazel would crash.

Addresses issue #5092. One part of it.
Fixes #5372.

RELNOTES: None.
PiperOrigin-RevId: 202114286
@marcohu
Copy link
Contributor Author

marcohu commented Jun 28, 2018

@mhlopko Thanks much for work!

Just tried with HEAD.

BUILD

load("@rules_antlr//antlr:antlr4.bzl", "antlr4")

antlr4(
    name = "generated",
    srcs = glob(["src/antlr4/*.g4"]),
    language = "Cpp",
)

cc_library(
    name = "lib",
    srcs = [":generated"],
    includes = ["."],
    deps = ["@antlr4_runtimes//:cpp"],
)

That works without issues.

But consuming the library still fails. Not sure whether it's just a configuration problem, though. From the the error message (see below), it looks to me as there might already be a way to make it work, but I was not able to. I'm trying to compile the following source file:

main.cpp

#include <iostream>

#include "antlr4-runtime.h"
#include "TLexer.h"
#include "TParser.h"

using namespace antlr4;

int main(int , const char **) {
  ANTLRInputStream input(u8"🍴 = 🍐 + \"😎\";(((x * π))) * µ + ∰; a + (x * (y ? 0 : 1) + z);");
  TLexer lexer(&input);
  CommonTokenStream tokens(&lexer);

  tokens.fill();
  for (auto token : tokens.getTokens()) {
    std::cout << token->toString() << std::endl;
  }

  TParser parser(&tokens);
  tree::ParseTree* tree = parser.main();

  std::cout << tree->toStringTree(&parser) << std::endl << std::endl;

  return 0;
}

BUILD

...
cc_binary(
    name = "Cpp",
    srcs = ["src/main.cpp"],
    deps = [":lib"],
)

The generated file tree looks like this.

bazel-bin/
└── antlr4
    └── Cpp
        ├── generated.antlr
        │   ├── TLexer.interp
        │   ├── TLexer.tokens
        │   ├── TParser.interp
        │   └── TParser.tokens
        └── generated.cc
            ├── TLexer.cpp
            ├── TLexer.h
            ├── TParserBaseListener.cpp
            ├── TParserBaseListener.h
            ├── TParser.cpp
            ├── TParser.h
            ├── TParserListener.cpp
            └── TParserListener.h
ERROR: /home/user/Dev/rules_antlr/examples/antlr4/Cpp/BUILD:18:1: C++ compilation of rule '//antlr4/Cpp:Cpp' failed (Exit 1): process-wrapper failed: error executing command 
  (cd /home/user/.cache/bazel/_bazel_user/e731f2dfd092596498e1031483dd09c5/execroot/examples && \
  exec env - \
    PATH=/home/user/.local/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games \
    PWD=/proc/self/cwd \
    TMPDIR=/tmp \
  /home/user/.cache/bazel/_bazel_user/e731f2dfd092596498e1031483dd09c5/execroot/examples/_bin/process-wrapper '--timeout=0' '--kill_delay=15' /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -B/usr/bin -B/usr/bin -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer '-std=c++0x' -MD -MF bazel-out/k8-fastbuild/bin/antlr4/Cpp/_objs/Cpp/main.pic.d '-frandom-seed=bazel-out/k8-fastbuild/bin/antlr4/Cpp/_objs/Cpp/main.pic.o' -fPIC -iquote . -iquote bazel-out/k8-fastbuild/genfiles -iquote bazel-out/k8-fastbuild/bin -iquote external/antlr4_runtimes -iquote bazel-out/k8-fastbuild/genfiles/external/antlr4_runtimes -iquote bazel-out/k8-fastbuild/bin/external/antlr4_runtimes -iquote external/bazel_tools -iquote bazel-out/k8-fastbuild/genfiles/external/bazel_tools -iquote bazel-out/k8-fastbuild/bin/external/bazel_tools -isystem external/antlr4_runtimes/runtime/Cpp/runtime/src -isystem bazel-out/k8-fastbuild/genfiles/external/antlr4_runtimes/runtime/Cpp/runtime/src -isystem bazel-out/k8-fastbuild/bin/external/antlr4_runtimes/runtime/Cpp/runtime/src -fno-canonical-system-headers -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -c antlr4/Cpp/src/main.cpp -o bazel-out/k8-fastbuild/bin/antlr4/Cpp/_objs/Cpp/main.pic.o)
antlr4/Cpp/src/main.cpp:16:20: fatal error: TLexer.h: No such file or directory
 #include "TLexer.h"
                    ^

@hlopko
Copy link
Member

hlopko commented Jun 28, 2018

hmmm maybe just precisely placed includes attribute or include_prefix is what's needed :)

werkt pushed a commit to werkt/bazel that referenced this issue Aug 2, 2018
Before, Bazel expected that it can compile whatever appeared in cc_library.srcs
directory artifacts. That is true for C/C++ source files, and for headers when
the C++ toolchain supported header parsing/processing (which used
CppCompileAction). When the toolchain doesn't support header parsing/processing,
Bazel would crash.

Addresses issue bazelbuild#5092. One part of it.
Fixes bazelbuild#5372.

RELNOTES: None.
PiperOrigin-RevId: 202114286
@hlopko hlopko added team-Rules-CPP Issues for C++ rules and removed category: rules > C++ labels Oct 11, 2018
@marcohu
Copy link
Contributor Author

marcohu commented Jun 5, 2019

hmmm maybe just precisely placed includes attribute or include_prefix is what's needed :)

Not quite. It only works if I split headers and sources. With the proper includes cc_binary finally compiles. Not sure if I had tried this last year. I'm currently using Bazel 0.26.

antlr4(
    name = "generated",
    srcs = glob(["src/antlr4/*.g4"]),
    language = "Cpp",
    package = "antlrcpptest",
)

headers(
    name = "headers",
    srcs = [":generated"],
)

sources(
    name = "sources",
    srcs = [":generated"],
)

cc_library(
    name = "lib",
    hdrs = [":headers"],
    srcs = [":sources"],
    includes = ["generated.hh/antlrcpptest"],
    deps = ["@antlr4_runtimes//:cpp"]
)

cc_binary(
    name = "bin",
    srcs = ["src/cpp/main.cpp"],
    deps = [":lib"],
)

This limits the usage scenarios as the includes can't be dynamically determined from grammars (at least I don't see how I could feed this information back to the Skylark rule from my Java binary) and the usage feels a bit awkward, but at least there is a first working solution.

Do you see any chance this could be made simpler?

BTW, this is the ANTLR C++ project I'm trying to port to Bazel: https://github.com/antlr/antlr4/tree/master/runtime/Cpp/demo

@oquenchil oquenchil self-assigned this Jun 6, 2019
@marcohu
Copy link
Contributor Author

marcohu commented Jun 9, 2019

I've been pondering a bit more about the C++ support. I think the includes parameter really is warranted and necessary (and only feels weird to a non-C++ developer), because with ANTLR you want to make the headers available to dependent rules.

And there seems to be a way to simplify the process for the user via a custom toolchain. This is the route that rules_flex takes and as it might become an official Bazel project I guess it's the recommended approach.

@marcohu marcohu closed this as completed Jun 9, 2019
@marcohu
Copy link
Contributor Author

marcohu commented Jun 23, 2019

FYI, the final solution was amazingly simple. You just have to return a CcInfo provider with the compilation context!

In the BUILD file you have to provide the generating rule as a dependency as well as input:

cc_library(
    name = "lib",
    srcs = [":generated"],
    deps = [
        ":generated",
        "@antlr4_runtimes//:cpp",
    ],
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

No branches or pull requests

4 participants