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

deps in bzl_library() incorrect handled in stardoc() #72

Open
lisaonduty opened this issue Oct 1, 2020 · 6 comments
Open

deps in bzl_library() incorrect handled in stardoc() #72

lisaonduty opened this issue Oct 1, 2020 · 6 comments

Comments

@lisaonduty
Copy link

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.

@c-parsons
Copy link
Collaborator

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.

@lisaonduty
Copy link
Author

lisaonduty commented Oct 1, 2020

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

@c-parsons
Copy link
Collaborator

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

@lisaonduty
Copy link
Author

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! :-)

@pauldraper
Copy link

pauldraper commented Nov 18, 2020

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.

@lisaonduty
Copy link
Author

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.

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

4 participants