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

bazel: bazelize docs/generated/redact_safe.md generation #67923

Closed
rickystewart opened this issue Jul 22, 2021 · 3 comments · Fixed by #68836
Closed

bazel: bazelize docs/generated/redact_safe.md generation #67923

rickystewart opened this issue Jul 22, 2021 · 3 comments · Fixed by #68836
Assignees
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf

Comments

@rickystewart
Copy link
Collaborator

Epic CRDB-8035

@rickystewart rickystewart added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-build-system T-dev-inf labels Jul 22, 2021
@rickystewart
Copy link
Collaborator Author

	@(echo "The following types are considered always safe for reporting:"; echo; \
	  echo "File | Type"; echo "--|--") >[email protected] || { rm -f [email protected]; exit 1; }
	@git grep --recurse-submodules -n '^func \(.*\) SafeValue\(\)' | \
	  grep -v '^vendor/github.com/cockroachdb/redact' | \
	  sed -E -e 's/^([^:]*):[0-9]+:func \(([^ ]* )?(.*)\) SafeValue.*$$/\1 | \`\3\`/g' | \
	  sort >>[email protected] || { rm -f [email protected]; exit 1; }
	@git grep --recurse-submodules -n 'redact\.RegisterSafeType' | \
	  grep -vE '^([^:]*):[0-9]+:[ 	]*//' | \
	  grep -v '^vendor/github.com/cockroachdb/redact' | \
	  sed -E -e 's/^([^:]*):[0-9]+:.*redact\.RegisterSafeType\((.*)\).*/\1 | \`\2\`/g' | \
	  sort  >>[email protected] || { rm -f [email protected]; exit 1; }

This is easily something that we can't bazelize -- we can't pass the entire source tree into a Bazel genrule to grep the sources, let alone the entire tree of all the dependencies that we depend on.

If we still want this file in-tree and we still want it generated along with the rest of our documentation, then we'll either have to find a more sensible (incremental, not monolithic) way to construct this file, or we'll have to accept that we can't use Bazel to build it.

cc @knz

@knz
Copy link
Contributor

knz commented Jul 27, 2021

The function of this is twofold.

One is to ensure we get a GitHub PR notification when a type is added to this list.

Another is to document the list of these types, for folk who want to know how the type they're looking at will be treated.

I'm not sure what "incremental" would look like?

In any case, this is more or less the same challenge as ensuring our linter tests can run. What does Bazel think about the linters?

@rickystewart
Copy link
Collaborator Author

I'm not sure what "incremental" would look like?

(This is hypothetical, I haven't designed this and don't think that trying to implement this makes sense given the ROI.) We'd need a way to compute the list of "redact-safe" types for an individual Go package, and then a way to union all those redact-safe types across all of our Go packages and our dependencies. i.e., the same thing, but computed incrementally rather than doing a grep over the entire codebase including dependencies.

In any case, this is more or less the same challenge as ensuring our linter tests can run. What does Bazel think about the linters?

Yeah, so the linters don't really run "inside" the Bazel sandbox. You can use Bazel to build a test binary containing the linter tests and then run it from within the workspace, but that's meaningfully different from asking Bazel to run a test itself, in which case all the dependencies for the test would have to be captured in the sandbox -- and the "dependencies" for the linter include the entire source tree, which is a non-starter. Similarly, we can't capture the entire source tree as a dependency for redact_safe.md.

I think the way this is going to go is that whatever command we have people run to generate documentation (dev generate docs or whatever) is going to run a bazel build to build the majority of the documentation, and then it'll probably "cheat" and just do the git grep thing directly to generate redact_safe.md.

@rickystewart rickystewart self-assigned this Aug 12, 2021
rickystewart added a commit to rickystewart/cockroach that referenced this issue Aug 12, 2021
This file can't properly be bazelfied because generating it requires
access to the entire source tree, which we can't easily pass into the
Bazel sandbox. Instead it gets special non-Bazel support in `dev` and a
special validation in CI.

Closes cockroachdb#67923

Release note: None
rickystewart added a commit to rickystewart/cockroach that referenced this issue Aug 13, 2021
This file can't properly be bazelfied because generating it requires
access to the entire source tree, which we can't easily pass into the
Bazel sandbox. Instead it gets special non-Bazel support in `dev` and a
special validation in CI.

Closes cockroachdb#67923

Release note: None
craig bot pushed a commit that referenced this issue Aug 16, 2021
68836: dev: generate `docs/generated/redact_safe.md` in `dev` r=rail a=rickystewart

This file can't properly be bazelfied because generating it requires
access to the entire source tree, which we can't easily pass into the
Bazel sandbox. Instead it gets special non-Bazel support in `dev` and a
special validation in CI.

Closes #67923

Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
@craig craig bot closed this as completed in 246e68c Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants