Cargo check before get the metadata #289
Conversation
Fixes google#225 The reason `set -e` failed to interrupt the workflow is due to the output redirection to `jq`. Since `jq` returns a value of 0, indicating success, the script continues to execute. You can try running these scripts locally to gain insight into the issue. To illustrate: Script without interruption: ```sh set -e function pkg-meta { cargo metadata --format-version 1 | jq -r ".packages[] | select(.name\ == \"zerocopy\").$1" } ZC_TOOLCHAIN=\"$(pkg-meta 'metadata.ci.\"pinned-nightly\"')\" echo \"Discovered that the nightly toolchain is $ZC_TOOLCHAIN\" echo \"ZC_TOOLCHAIN=$ZC_TOOLCHAIN\" ``` Script with interruption: ```sh set -e function pkg-meta { cargo metadata --format-version 1" } ZC_TOOLCHAIN=\"$(pkg-meta 'metadata.ci.\"pinned-nightly\"')\" echo \"Discovered that the nightly toolchain is $ZC_TOOLCHAIN\" echo \"ZC_TOOLCHAIN=$ZC_TOOLCHAIN\" ``` Additionally, you can display the return value after the command execution. This output will confirm the successful execution of the command. ```sh set -e function pkg-meta { cargo metadata --format-version 1 | jq -r \".packages[] | select(\ .name == \"zerocopy\").$1\" echo echo \$? } ZC_TOOLCHAIN=\"$(pkg-meta 'metadata.ci.\"pinned-nightly\"')\" echo \"Discovered that the nightly toolchain is $ZC_TOOLCHAIN\" echo \"ZC_TOOLCHAIN=$ZC_TOOLCHAIN\" ``` Therefore, I've incorporated `cargo check` to validate the package retrieval. Subsequent to this check, there's no necessity to fetch the package again using `cargo metadata`. This modification should rectify the point of failure in the Git action.
|
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. |
.github/workflows/ci.yml
Outdated
| @@ -86,6 +86,7 @@ jobs: | |||
| run: | | |||
| set -e | |||
|
|
|||
| cargo check | |||
There was a problem hiding this comment.
Can you change this to cargo metadata --format-version 1 > /dev/null? cargo check may fail for other reasons (ie, there's an issue with the code uploaded in a PR).
There was a problem hiding this comment.
Hello @joshlf,
I've assessed that both cargo check and cargo metadata aren't appropriate solutions for addressing this issue. They don't align with the workflow of this job. The root problem lies in the job's failure to pause when a command encounters an error, and then pass that outcome to the subsequent command in the pipeline.
Given this, I am proposing a solution that revolves around adjusting the set command. To be more precise, by introducing the -o pipefail option, we can make the command proficient at recognizing failures. This, in turn, empowers the -e flag to conclude the process. Consequently, the refined set command would appear as follows: set -eo pipefail. For further elaboration on the set command, additional details can be found using set --help.
Additionally, I've noticed that other jobs also use set -e and command piping, which might have similar issues. Would it be acceptable to apply these changes to all those jobs?
Your guidance on this matter would be greatly appreciated.
There was a problem hiding this comment.
Yeah, setting set -eo pipefail everywhere would be great!
|
This build failure illustrates that your technique works perfectly! |
After evaluating the situation, it's clear that both `cargo check` and `cargo metadata` aren't suitable solutions for addressing this issue. They don't align with the workflow of this job. The core problem lies in the job's inability to pause when a command encounters an error and then propagate that outcome to the subsequent command in the pipeline. With this understanding, I'm suggesting a solution that centers around modifying the `set` command. To be more precise, by incorporating the `-o pipefail` option, we can enhance the command's ability to detect failures. This, in turn, empowers the `-e` flag to terminate the process. As a result, the refined `set` command would take the form: `set -eo pipefail`. For a deeper understanding of the `set` command, you can refer to additional information available using `set --help`.
To bolster reliability, incorporate the -o pipefail option, which designates a command pipeline's overall return status as failed if any individual command within it fails. Following this, integrating the -e option can empower the script with the ability to promptly halt execution.
Fixes #225
The reason
set -efailed to interrupt the workflow is due to the output redirection tojq. Sincejqreturns a value of 0, indicating success, the script continues to execute. You can try running these scripts locally to gain insight into the issue.To illustrate:
Script without interruption:
Script with interruption:
Additionally, you can display the return value after the command execution. This output will confirm the successful execution of the command.
Therefore, I've incorporated
cargo checkto validate the package retrieval. Subsequent to this check, there's no necessity to fetch the package again usingcargo metadata. This modification should rectify the point of failure in the Git action.