bazelbuild / stardoc

Stardoc: Starlark Documentation Generator
Apache License 2.0
108 stars 45 forks source link

Add support for Bzlmod #141

Closed fmeum closed 1 year ago

fmeum commented 2 years ago

Input .bzl files are turned into runfiles of a custom Stardoc binary, which allows reusing the Java runfiles library to load the files while taking repository mappings into account.

fmeum commented 2 years ago

@comius Should be ready for review, the failing test seems to require updating the stardoc parts in Bazel first.

fmeum commented 1 year ago

@comius I pushed a new commit to fix the buildifier warnings.

fmeum commented 1 year ago

@comius Friendly ping. Tests are expected to fail until we carry out https://github.com/bazelbuild/bazel/pull/16775#issuecomment-1337613168.

aaronmondal commented 1 year ago

Friendly ping. This is a blocker for downstream projects to fully migrate away from WORKSPACEs.

tetromino commented 1 year ago

@fmeum - am I correct in understanding that this PR requires using a Stardoc jar built after merging https://github.com/bazelbuild/bazel/pull/16775 or similar? And without changing the doc extractor, we cannot get support for bzlmod?

fmeum commented 1 year ago

@tetromino That's right. I outlined a process that I think should keep CI green at any time at https://github.com/bazelbuild/bazel/pull/16775#issuecomment-1337613168, but it's quite possible I missed something or there is a simpler way.

fmeum commented 1 year ago

I pushed a TMP commit that should make the pipeline green again, please ignore it during import.

fmeum commented 1 year ago

@comius Updating Bazel turns out to be rather painful as it now uses rules_jvm_external. I was thinking whether we could switch to using Bzlmod exclusively for Stardoc development. What do you think?

fmeum commented 1 year ago

I copied over the entire list of Maven deps from Bazel. It is long, but my attempts at doing this piece by piece resulted in a seemingly never-ending game of whack-a-mole.

fmeum commented 1 year ago

The buildifier failure is due to https://github.com/bazelbuild/buildtools/issues/1098, which is unrelated (except that I caused it, sorry :-))

comius commented 1 year ago

@comius Updating Bazel turns out to be rather painful as it now uses rules_jvm_external. I was thinking whether we could switch to using Bzlmod exclusively for Stardoc development. What do you think?

I don't mind using bzlmod by default, but I'm not sure it helps. You'll need bazel, which is not on BCR. Adding it seems the easiest (cc @meteorcloudy @Wyverald ), but not sure if there are any objections to that.

Depending on Bazel via bzlmod via repo rules probably has the same problem.

Is it possible to depend on slightly older commit or disable some transitive tests to just get skydoc?

fmeum commented 1 year ago

We could get it via a git_override, but would need to remove that line for BCR releases as non-root modules can't have overrides.

The commit with the changes for Stardoc landed after the switch to rules_jvm_external, so I don't see a way to depend on an older commit. I could manually prune the list based on query information, but starting from scratch led me to add quite a few deps that I wasn't expecting to be needed. @meteorcloudy Do you have an overview of what should be required here?

comius commented 1 year ago

@tetromino this is now ready for review and/or merge

Wyverald commented 1 year ago

Adding it seems the easiest (cc @meteorcloudy @Wyverald )

Adding Bazel to BCR is a bit weird since Bazel is not exactly a library people should depend on. (Granted, it's hella weird that Stardoc depends on Bazel, but that ship sailed before I even knew what Bazel was.)

I think we should probably just do the git_override plus patch thing. And hope @tetromino's rewrite ships soon (lol).

meteorcloudy commented 1 year ago

Yeah, the prod version of Stardoc doesn't depend on Bazel, so even if we want to check in Stardoc in BCR, we don't need Bazel there. git_override with some patch should be enough.

fmeum commented 1 year ago

@tetromino I found a small issue. I will fix it and submit a PR with a proper BCR test and publishing setup.