ci: fix Solidity action output handling#6
ci: fix Solidity action output handling#6AdekunleBamz wants to merge 1 commit intobifrost-platform:masterfrom
Conversation
Fixes a bug where the compile output was written to a literal file named OUTPUT_PATH instead of the configured output path. Also runs the workflow on pull requests and updates checkout to v4.
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in the custom Solidity CI action where the solc compiler output was being redirected to a literal file named OUTPUT_PATH instead of using the configured output file path stored in the $OUTPUT_PATH variable. This bug could hide compiler errors and make CI checks unreliable.
Changes:
- Fixed the critical variable expansion bug in solc output redirection
- Added initialization/clearing of output files (
errors,codesize,OUTPUT_PATH) at script start - Improved error handling with
|| exit 1and|| truewhere appropriate - Enhanced path and variable quoting throughout the script for robustness
- Updated GitHub Actions workflow to trigger on pull requests in addition to pushes
- Updated checkout action from v2 to v4
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| actions/solidity-0.6/entrypoint.sh | Fixed critical OUTPUT_PATH bug, added output file initialization, improved quoting and error handling throughout the shell script |
| .github/workflows/solidity-actions.yml | Added pull_request trigger and updated checkout action to v4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "$1" >> codesize | ||
| solc --optimize --combined-json bin-runtime "$1" --allow-paths "$SOURCE_DIR" > "$1".json | ||
| CODESIZE=$(cat "$1".json | jq '.contracts[] | select(."bin-runtime" != "") | ."bin-runtime"' | tail -1 | wc -m) | ||
| echo $CODESIZE >> codesize |
There was a problem hiding this comment.
The variable CODESIZE should be quoted for consistency with the quoting improvements made elsewhere in this script. While this line works correctly because echo is used, maintaining consistent quoting practices helps prevent potential issues if the code is modified in the future.
| echo $CODESIZE >> codesize | |
| echo "$CODESIZE" >> codesize |
| echo $CODESIZE >> codesize | ||
| CODESIZE=`expr $CODESIZE / 2 - 1` | ||
| PERCENT=`expr $CODESIZE \* 100 / 24576` | ||
|
|
||
| echo $PERCENT\% >> codesize |
There was a problem hiding this comment.
The variables PERCENT and CODESIZE should be quoted for consistency with the quoting improvements made elsewhere in this script. While these lines work correctly in their current context, maintaining consistent quoting practices helps prevent potential issues if the code is modified in the future.
| echo $CODESIZE >> codesize | |
| CODESIZE=`expr $CODESIZE / 2 - 1` | |
| PERCENT=`expr $CODESIZE \* 100 / 24576` | |
| echo $PERCENT\% >> codesize | |
| echo "$CODESIZE" >> codesize | |
| CODESIZE=`expr $CODESIZE / 2 - 1` | |
| PERCENT=`expr $CODESIZE \* 100 / 24576` | |
| echo "$PERCENT%" >> codesize |
This PR fixes an issue in the custom Solidity CI action where solc output was redirected to a literal file named
OUTPUT_PATHinstead of the configured output file, which could hide compiler errors and make CI unreliable.Changes:
$OUTPUT_PATHfileerrors,codesize, and$OUTPUT_PATHfor each runpull_requestin addition topushactions/checkout@v4