Add diff_test asserting that docs are up-to-date

[alexeagle]

it prints a convenient 'bazel run' command to update them, replacing the shell script

Follow-up to #297 (comment)

[tetromino]

Thank you for the PR!

First, a nit: please rename tag from "no-bazelci-windows" to just "no_windows" - simpler, more descriptive (it's not just Bazel CI that we care about - the test would also fail on Windows if run manually) and it's the tag name used for the same purpose in other places.

Second, major stylistic request: we try to keep BUILD files friendly for automated tools (think mass invocation of buildozer across a monorepo). This means: repetition in a BUILD file (unlike a .bzl file) is expected and normal; there shouldn't be loops; the bulk of the BUILD file should look like a list of target initializations, each one having a name.

If you want to avoid some repetition, I would suggest introducing a helper macro combining one stardoc and corresponding diff_test (say, stardoc_with_diff_test) and explicitly invoking that macro on each doc:

stardoc_with_diff_test(
    name = "build_test_docs",
    out = "build_test_doc_gen.md",
    input = "//rules:build_test.bzl",
    deps = ["//rules:build_test"],
)

stardoc_with_diff_test(
    name = "analysis_test_docs",
    out = "analysis_test_doc_gen.md",
    input = "//rules:analysis_test.bzl",
    deps = ["//rules:analysis_test.bzl"],
)

...

And //docs:update in this case might use native.existing_rules to obtain the names of any stardoc targets (ideally via another helper macro). Maybe something like this:

def update_above_docs(name):
    content = ["#!/usr/bin/env bash", "cd ${BUILD_WORKSPACE_DIRECTORY}"]
    data = []
    for r in native.existing_rules().values():
        if r["kind"] == "stardoc":
            doc_gen = r["out"]
            if doc_gen.startswith(":"):
                doc_gen = doc_gen[1:]
            doc_dst = doc_gen.replace("_gen.md", ".md")
            data.append(doc_gen)
            content.append("cp -fv bazel-bin/docs/{0} docs/{1}".format(doc_gen, doc_dst))

    update_script = name + ".sh"
    write_file(
        name = name + "_gen",
        out = update_script,
        content = content,
    )

    native.sh_binary(
        name = name,
        srcs = [update_script],
        data = data,
    )

[alexeagle]

I agree in theory that BUILD files should be machine-edited.

However in this case, there is no likely machine-editor in sight. This repo isn't big enough to require one-off buildozer commands, and there is no gazelle plugin to help maintain the file.

If it's always going to be hand-edited, would you still want to expand the file to the amount of boilerplate you suggest?

In addition the overall complexity is higher when you need to create that macro and (very cool) way of finding other rules in the package.

OTOD you've already got the edits in context so WDYT about making that change yourself?

[alexeagle]

Hey @tetromino in the time since you suggested that, we built a full-featured rule to support exactly your proposal
https://github.com/aspect-build/bazel-lib/blob/main/docs/docs.md
example usage:
https://github.com/aspect-build/rules_js/blob/main/docs/BUILD.bazel

[tetromino]

@alexeagle - the aspect_build/bazel-lib rule looks great for skylib's internal use.

I'm hesitant to export it as is as a public api from skylib because compared to a Google internal implementation of the same idea, it's missing some features which non-Google users probably also want (e.g. autoformatting of markdown output, keeping docs in a separate directory tree from .bzl sources, embedding comments in the markdown output), and the "package:" tag feels like bit of a hack.

Are you interested in updating this PR based on aspect_build/bazel-lib stardoc_with_diff_test without the "package:" tag (which isn't needed for internal functionality)?

[tetromino]

Or alternatively, do you want me to do it?

[alexeagle]

Sure, I can do it. I'd really like to have more collaboration with you, I think we are doing a lot of the same work.

https://github.com/aspect-build/bazel-lib/blob/6f7469dd03dd1a19697a3d7e33d7ba8972eac034/lib/private/docs.bzl has the implementation you want, prior to switching it to be based on our write_source_file rule. That rule has the nice property that it gives the user a nice error when the source file to diff against doesn't exist, but it's not a feature you need.

/cc @kormide

[alexeagle]

ping @tetromino PTAL

[tetromino]

LGTM and thank you!

I took the liberty to fix a couple of tiny nits - added some comments to remind that update_docs needs to go at the bottom of the BUILD file, and renamed the tag to follow the same naming scheme as other projects.