-
Notifications
You must be signed in to change notification settings - Fork 4.4k
docs: add example for --workspace_status_command #28462
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
base: master
Are you sure you want to change the base?
docs: add example for --workspace_status_command #28462
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request adds an example for --workspace_status_command to the user manual. While adding an example is a great improvement, the current implementation has some issues with markdown formatting and clarity that could confuse users. I've provided a suggestion to restructure the example for better readability and correctness.
| #### Example | ||
|
|
||
| The following example shows how `--workspace_status_command` can be used | ||
| to generate build metadata and consume it during a build. | ||
|
|
||
| Create a workspace status script: | ||
|
|
||
| ```sh | ||
| #!/usr/bin/env sh | ||
| echo "BUILD_USER $(whoami)" | ||
| echo "BUILD_TIMESTAMP $(date +%s)" | ||
|
|
||
| Make the script executable: | ||
| chmod +x status.sh | ||
|
|
||
|
|
||
| bazel build //:gen_metadata --workspace_status_command=./status.sh | ||
|
|
||
| genrule( | ||
| name = "gen_metadata", | ||
| outs = ["metadata.txt"], | ||
| cmd = "cat bazel-out/volatile-status.txt > $@", | ||
| stamp = 1, | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example for --workspace_status_command has several issues that make it confusing and will cause rendering problems:
- Markdown Syntax Error: There is a missing closing ```` for the code block, which will cause the rest of the document to be rendered as code.
- Incorrect Content in Code Block: The
shcode block contains user instructions (Make the script executable:) and agenruledefinition (which is Python/Starlark syntax), neither of which are part of the shell script itself. - Confusing Structure: The example mixes file content, shell commands, and build rule definitions without clear separation or explanation of how they relate. For instance, the
bazel buildcommand appears before thegenruleit's supposed to build.
I've suggested a rewrite that structures the example as a clear, step-by-step guide and fixes the formatting issues.
#### Example
The following example shows how `--workspace_status_command` can be used
to generate build metadata and consume it during a build.
First, create a workspace status script named `status.sh`:
```sh
#!/usr/bin/env sh
echo "BUILD_USER $(whoami)"
echo "BUILD_TIMESTAMP $(date +%s)"Next, make the script executable:
chmod +x status.shThen, create a BUILD file with a genrule that consumes the status information. The stamp = 1 attribute is required for the rule to see the status files.
genrule(
name = "gen_metadata",
outs = ["metadata.txt"],
cmd = "cat bazel-out/volatile-status.txt > $@",
stamp = 1,
)Finally, run the build, pointing to your script with the --workspace_status_command flag:
bazel build //:gen_metadata --workspace_status_command=./status.sh
This PR adds a simple, self-contained example demonstrating how to use
--workspace_status_command in the Bazel user manual.
It includes:
This addresses the request in issue #17163 to make the documentation clearer
and easier for new users to understand.
Following the suggestion to proceed without formal assignment.
Fixes #17163