stardoc icon indicating copy to clipboard operation
stardoc copied to clipboard

deps in bzl_library() incorrect handled in stardoc()

Open lisaonduty opened this issue 3 years ago • 6 comments

We have a structure similar to this:

[workspace]/
    WORKSPACE
    BUILD
    macro/
        BUILD
        macro.bzl
    docs/
        BUILD

The macro.bzl use string_flag() and therefore have this load statement.

load(
    "@bazel_skylib//rules:common_settings.bzl",
    "string_flag",
)

To be able to generate documentation for the macros in macro.bzl we must take care of the dependency so we have made 2 bzl_library() targets in macro/BUILD:

load("@bazel_skylib//:bzl_library.bzl", "bzl_library")

bzl_library(
    name = "macro-deps",
    srcs = [
        "@bazel_skylib//rules:common_settings.bzl",
    ],
    visibility = ["//visibility:public"],
)

bzl_library(
    name = "macro-code",
    srcs = [
        "macro.bzl",
    ],
    visibility = ["//visibility:public"],
)

In BUILD below docs we have gathered all stardoc() rules. For the macro above we have the following rule:

load("@bazel_stardoc//stardoc:stardoc.bzl", "stardoc")

stardoc(
    name = "config-bool-docs",
    out = "config-bool.md",
    input = "//macro:macro-code",
    symbol_names = ["config_option_bool"],
    deps = ["//macro:macro-deps"],
)

This works BUT what we actually would like to have is this: macro/BUILD:

load("@bazel_skylib//:bzl_library.bzl", "bzl_library")

bzl_library(
    name = "macro-deps",
    srcs = [
        "@bazel_skylib//rules:common_settings.bzl",
    ],
)

bzl_library(
    name = "macro-code",
    srcs = [
        "macro.bzl",
    ],
    deps= [
        "macro-deps",
    ],
    visibility = ["//visibility:public"],
)

And in docs/BUILD:

load("@bazel_stardoc//stardoc:stardoc.bzl", "stardoc")

stardoc(
    name = "config-bool-docs",
    out = "config-bool.md",
    input = "//macro:macro-code",
    symbol_names = ["config_option_bool"],
)

So that we have the deps in macro-code target instead of the stardoc() target. Shouldn't that work? It doesn't build but I think that it should and that this is a bug. It doesn't follow the usual Bazel way of treating the dependencies

As stated above in docs/BUILD we have gathered all stardoc() rules (also for more than the above docs/macro ) . We think that all dependencies should be stated in BUILD's where we have the code and those BUILD's shall provide the public bzl_library() targets that can be used in the stardoc() rules in docs/BUILD. The targets below docs should not need to keep track of the dependencies.

lisaonduty avatar Oct 01 '20 14:10 lisaonduty

How about:

load("@bazel_skylib//:bzl_library.bzl", "bzl_library")

bzl_library(
    name = "macro-deps",
    srcs = [
        "@bazel_skylib//rules:common_settings.bzl",
    ],
    visibility = ["//visibility:public"],
)

bzl_library(
    name = "macro-code",
    srcs = [
        "macro.bzl",
    ],
    deps = ":macro-deps",
    visibility = ["//visibility:public"],
)

with:

load("@bazel_stardoc//stardoc:stardoc.bzl", "stardoc")

stardoc(
    name = "config-bool-docs",
    out = "config-bool.md",
    input = "//macro:macro-code",
    symbol_names = ["config_option_bool"],
    deps = ["//macro:macro-code"],
)

This is close to your first example, except that macro-code is depending on macro-deps via deps of bzl_library, and thus the stardoc target depends transitively on your macro's dependencies.

It's indeed unfortunate that one needs to specify macro-code as an input and separately as an entry in deps -- we should fix that. But the workaround here seems close to equivalent to what you want.

c-parsons avatar Oct 01 '20 15:10 c-parsons

Thanks for the proposal :-) but I don't get that working either. :-(

ERROR:docs/BUILD:3:8: in input attribute of stardoc rule //docs:config-bool-docs: '//macro:config-code' must produce a single file

lisaonduty avatar Oct 01 '20 15:10 lisaonduty

Whoops. I guess you need:

load("@bazel_stardoc//stardoc:stardoc.bzl", "stardoc")

stardoc(
    name = "config-bool-docs",
    out = "config-bool.md",
    input = "//macro:macro.bzl",
    symbol_names = ["config_option_bool"],
    deps = ["//macro:macro-code"],
)

(You'll need to export macro.bzl in that package if it's not already using export_files)

We should definitely clean this up, but this should solve the issue that the stardoc target need not be aware of macro.bzl's dependencies

c-parsons avatar Oct 01 '20 15:10 c-parsons

We wanted to avoid to also make an export of macro.bzl and only use the bzl_library(). It works to generate the documentation as it is now but it would be appreciated if it was possible to specify all the dependencies in the "code directories" and only have one public bazel target that are used in the "documentation directory". Then we follow the same clean dependency principle also for documentation as we have for our Bazel build and test.

It seems like the input parameter can only contain one file (it can be the file or a bzl_library() with one file). It is stated in the documentation that the input parameter to stardoc() is optional. But it seems to be mandatory. If symbol_names is given then you could think that it should be possible to have only one bzl_library() in deps instead and leave the input empty. But that isn't possible either.

Thanks for the attention, it's appreciated! :-)

lisaonduty avatar Oct 02 '20 08:10 lisaonduty

While I wouldn't go so far to say current behavior is "incorrect," I agree that Stardoc ought accept multiple inputs (a bzl_library) and produce multiple outputs.

This is the most natural way to think about it....I've got this Starlark library that I want to document.

pauldraper avatar Nov 18 '20 23:11 pauldraper

I can't see why Stardoc shouldn't follow the same behavior as Bazel does for transitive dependencies?! Stardoc is tailor-made for Bazel so to follow Bazel must be something that is wanted and from that perspective I would say it is incorrect.

lisaonduty avatar Nov 20 '20 10:11 lisaonduty