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

Stardoc fails in Bazel version 4.0.0 if load of @rules_cc in .bzl files #92

Open
lisaonduty opened this issue Feb 16, 2021 · 7 comments
Assignees

Comments

@lisaonduty
Copy link

Hi,

We were working on upgrading our Bazel version from 3.7.0 to 4.0.0 when our Stardoc document rules failed to build. The only information we received was that it was a Starlark evaluation error:

ERROR: /stardoc_example/cpp/BUILD:12:8: Generating proto for Starlark doc for cc_library_macro_doc failed: (Exit 1): stardoc failed: error executing command 
  (cd bazel_build_root/testbaselib/sandbox/processwrapper-sandbox/5/execroot/bbi && \
  exec env - \
  bazel-out/host/bin/external/bazel_stardoc/stardoc/stardoc '--input=//stardoc_example/cpp:hello.bzl' '--workspace_name=' '--symbols=cc_library_macro' '--dep_roots=.' '--dep_roots=external/' '--output=bazel-out/k8-fastbuild/bin/stardoc_example/cpp/cc_library_macro_doc.raw')
...
Use --sandbox_debug to see verbose messages from the sandbox
Stardoc documentation generation failed: Starlark evaluation error
Target //stardoc_example/cpp:cc_library_macro_doc failed to build

Fortunate we had some documentation rules that still could build so from that we realized (after digging for hours) that it failed for the documents that used cc-rules in the .bzl files. These contained load statement of rules_cc:

load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")

So to resolve this and be able to upgrade our Bazel version we needed to remove all these load statements and use native calls instead.

I made a general .bzl rule in a local copy of Bazel cpp example (bazel/example/cpp)
which I share below.

In 3.7.0 it works fine to have it like this:

We added the following in the BUILD file:

load("@bazel_stardoc//stardoc:stardoc.bzl", "stardoc")
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
load("hello.bzl","cc_library_macro")
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")

cc_library_macro(
    name = "hello-lib-macro",
    srcs = ["hello-lib.cc"],
    hdrs = ["hello-lib.h"],
)
stardoc(
    name = "cc_library_macro_doc",
    out = "cc_library_macro_doc.md",
    input = "hello.bzl",
    deps = ["rules_cc"],
)

bzl_library(
    name = "rules_cc",
    srcs = [
        "@rules_cc//cc:action_names.bzl",
        "@rules_cc//cc:defs.bzl",
        "@rules_cc//cc:find_cc_toolchain.bzl",
        "@rules_cc//cc/private/rules_impl:cc_flags_supplier.bzl",
        "@rules_cc//cc/private/rules_impl:cc_flags_supplier_lib.bzl",
        "@rules_cc//cc/private/rules_impl:compiler_flag.bzl",
    ],
    visibility = ["//visibility:public"],
)

And made a new .bzl file with the following content:

load("@rules_cc//cc:defs.bzl", "cc_library")

def cc_library_macro(
    name,
    srcs,
    hdrs):
    """
    Macro that wrapper cc_library rule.

    **cc_library_macro** is a macro around Bazel
    [cc_library()](https://docs.bazel.build/be/c-cpp.html#cc_library)

    ## Output

    The rule will produce a static library (.a file).

    Args:
        name: Type: String

            A unique name for this rule.
        srcs: Type: List of labels

            The list of C and C++ files that are processed to create the target.
        hdrs: Type: List of labels

            Public headers in this static library.

    """
    cc_library(
    name = name,
    srcs = srcs,
    hdrs = hdrs,
    )

And the markdown document was successfully generated when executing the following command:

bazel build cc_library_macro_doc

But when building with Bazel version 4.0.0 it fails. To resolve this we needed to remove the load statement in hello.bzl:

load("@rules_cc//cc:defs.bzl", "cc_library")

And use the native call instead:

native.cc_library(

And then we could also remove the target "rules_cc" from deps.

On the side note we would also like to mention that we think that it's unfortunate that we needed to specify all internal dependencies for rules_cc but that was the only way we found out to get successful builds.

We use Stardoc version 0.4.0

We would be grateful to get your feedback on this and to confirm if this is a bug.
Since the Bazel version 4.0.0 is considered as a long term supported version (LTS) it feels extra important that Stardoc is compatible with that version.

Thanks and Regards
Lisa

@brandjon
Copy link
Member

Hi Lisa, that certainly sounds problematic. The lack of an informative error message should be fixed as of bazelbuild/bazel@9f60686, but that commit isn't in 4.0.0 (see bazelbuild/bazel#13077). Can you try running with Bazel at head and see if you get a more useful error?

@lisaonduty
Copy link
Author

Thanks @brandjon for your quick reply!

We will try with the patch you refer to and see what error message we get then.

@pilkjaer
Copy link

@brandjon It seems that commit that improves error messages was merged to bazel/master 7/1 and Bazel 4.0.0 was released 21/1. In order words, that commit is already present in 4.0.0 and example provided by @lisaonduty already contains that fix. If you want/can continue this discussion we probably should make a separate ticket for this.

@brandjon
Copy link
Member

Bazel 4.0.0 has baseline bazelbuild/bazel@37a429a, from Nov 18. The error message commit came during the (rather long, it seems) canarying period.

@pilkjaer
Copy link

@brandjon All right. Thank you for your clarification. In that case 4.0.0 does not contain that fix. But our local Bazel uplift that we call "4.0.0" is based on that release commit. So in reality it's 4.0.0 plus some more commits. That error fix commit is part of our baseline so we already running it.

@brandjon
Copy link
Member

So to confirm, a build of Bazel containing the Jan 7 fix for better error reporting in Stardoc still fails to give a useful error message for why Stardoc now fails on .bzl files that load from @rules_cc? Then the next will be to repro this and improve the error handling path. I'm a little busy at the moment but will try to look within the next few days.

@brandjon brandjon added P2 and removed P3 labels Feb 24, 2021
@brandjon brandjon self-assigned this Feb 24, 2021
@pilkjaer
Copy link

@brandjon Yes. Your summary is correct. Sorry for the confusion. We were able to find the root of the problem as well as fix it so for us there is no urgency. You can follow instructions provided by @lisaonduty so you can both reproduce the error that she reported as we ll to see if error message is something you can improve upon.

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