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

Separate generating documentation per module #202

Merged
merged 28 commits into from
Mar 25, 2022
Merged

Separate generating documentation per module #202

merged 28 commits into from
Mar 25, 2022

Conversation

fricklerhandwerk
Copy link
Member

@fricklerhandwerk fricklerhandwerk commented Mar 16, 2022

closes #197

this is a huge diff, unfortunately. most of the noise comes from unavoidable redundancy in WORKSPACE files per module for backwards compatibility, and the rendered documentation checked into version control. tried to keep a narrative in commit history. it probably makes sense to just look at the result, possibly excluding the last commit, which only updates the READMEs. to inspect motivation per line one has to resort to git blame, sorry for the inconvenience.

key takeaways:

  • documentation is rendered per module
  • rendering code is shrinked and simplified significantly, and in one place
  • top-level README.md renders from the same data
  • full backwards compatibility

@fricklerhandwerk fricklerhandwerk force-pushed the issue-197 branch 7 times, most recently from 9573bec to 768df13 Compare March 21, 2022 09:08
@fricklerhandwerk fricklerhandwerk marked this pull request as ready for review March 21, 2022 09:08
@fricklerhandwerk fricklerhandwerk force-pushed the issue-197 branch 2 times, most recently from 28cc36e to bb267c9 Compare March 21, 2022 10:24
Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, good work teasing all of this apart!

A couple comments.

docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
core/.bazelrc Outdated Show resolved Hide resolved
core/WORKSPACE Outdated Show resolved Hide resolved
core/WORKSPACE Show resolved Hide resolved
core/BUILD.bazel Outdated Show resolved Hide resolved
core/BUILD.bazel Show resolved Hide resolved
toolchains/java/WORKSPACE Outdated Show resolved Hide resolved
core/docs/stardoc.bzl Outdated Show resolved Hide resolved
toolchains/cc/README.md Outdated Show resolved Hide resolved
rename its internal source collection `nixpkgs`.
ideally we would already operate within `bzlmod` here, but we do not
even have all dependencies present in [BCR][bcr] as of today. it is
another can of worms detailed in [#181][181].

here we just isolate `rules_nixpkgs_core` as a proper self-contained
repository as far as possible.

[bcr]: https://github.com/bazelbuild/bazel-central-registry/tree/92bb87fc41d549a47297af67c4135111308b7b03
[181]: #181
remove unused README templates, as rendering is now self-contained.

motivation: `docs` package is for external use by other modules.
modules will have their own documentation rules, and we do not want to
introduce artificial packages in each just for that.

this change requires clearly distinguishing between the transient,
generated file and the source file that we create with `copy_files`.
that was not necessary before, as the transient and source files lived
in different directories. since Bazel does not have a way to express
this distinction, encode the distinction with (arbitrarily) differing
file names.

for now decide against encoding this in the rules, as the mechanism
would be obfuscated.
preserve existing helpers in case we ever need to do more file copying
in a similar fashion.
also a few cleanups to clarify internal dependencies.
This reverts commit 3754098.

taking a different approach to render module documentation...
invocations `bazel run` within `toolchains/cc` for some reason are not
idempotent (at least on Darwin) and on the second run throw this error:

rules_nixpkgs/toolchains/cc/BUILD.bazel:38:9: Generating proto for Starlark doc for __README.md failed: missing input file '//:bazel-out/_tmp/actions/stderr-12'

run `bazel clean` to avoid this. it may also be possible to work around
this with adding to `.bazelignore`.
these files are already made accessible by `file_group("srcs")`.
letting Bazel ignores them prevents issues with idempotency of builds

note that `.bazelignore` has to be a proper file, symlinks do not behave
as expected.
otherwise the file must already be present to do the build
render documentation for original workspace based on separate modules,
only using `stardoc` templates.

this lets us get rid of additional template files and a lot of noisy
helper code.

add rule aliases for backwards compatibility.
this removes the requirement of having the generated file already
present in the source tree. otherwise, if it does not exist, `cp` will
fail with "Permission denied".

note that we had to explicitly make use of `POSIX_SH` provided by the
build itself, as `--no-preserve` is unique to `coreutils` and not
available on e.g. BSD or Darwin.
this is to have as little repetition as currently possible, and while it
looks ugly, it is intended to be only temporary until the
sub-repositories are proper `bzlmod` modules.

`WORKSPACE`-based documentation is still rendered the same way, and will
not be changed, to keep compatibility. at some point, once we have
modules, we will remove the temporary sub-module `WORKSPACE` files, and
the top level will import their local dependencies as packages again.
lumping these together forced strict toolchain-specific dependencies on
all users, while these are normally only needed for development (in this
case, rendering documentation only).
do not generate for Go as that would require aliasing the Go library,
which in turn would introduce a strict dependency on
`@io_bazel_rules_go`. instead link to the existing module `README.md`.
@fricklerhandwerk
Copy link
Member Author

@aherrmann Fixed all the comments and issues.

Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, this looks great!

Only one comment: #202 (comment)

@aherrmann aherrmann added the merge-queue merge on green CI label Mar 25, 2022
@mergify mergify bot merged commit aeeabee into master Mar 25, 2022
@mergify mergify bot deleted the issue-197 branch March 25, 2022 12:50
@mergify mergify bot removed the merge-queue merge on green CI label Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate reference documentation for each toolchain module
2 participants