Fix builds #9
Fix builds #9
Conversation
Reviewer's GuideReplaces the legacy master-build workflow with a unified GitHub Actions pipeline (build.yml) featuring multi-platform CMake builds, OS-specific dependency installation, testing, packaging, artifact uploads, and conditional automated prerelease creation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
General comments:
- The release notes use placeholder "?? Windows x64" etc—consider programmatically listing actual platforms or replacing those placeholders to keep the notes accurate.
- The test step is marked continue-on-error, so failing tests won’t break the workflow—remove that flag if you want test failures to fail the build.
- Consider adding caching (using actions/cache) for CMake state and package installs (apt/homebrew) to significantly speed up CI builds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The release notes use placeholder "?? Windows x64" etc—consider programmatically listing actual platforms or replacing those placeholders to keep the notes accurate.
- The test step is marked continue-on-error, so failing tests won’t break the workflow—remove that flag if you want test failures to fail the build.
- Consider adding caching (using actions/cache) for CMake state and package installs (apt/homebrew) to significantly speed up CI builds.
## Individual Comments
### Comment 1
<location> `.github/workflows/build.yml:92` </location>
<code_context>
+ - name: Build
+ shell: bash
+ run: |
+ cmake --build build --config Release --parallel $(nproc 2>/dev/null || echo 4)
+
+ - name: List Build Output
</code_context>
<issue_to_address>
The use of $(nproc 2>/dev/null || echo 4) for parallel builds may not work on macOS or Windows runners.
Since nproc is not available on macOS and this command fails on Windows, please use a more portable method for setting parallelism, such as a fixed value or leveraging GitHub Actions environment variables.
</issue_to_address>
### Comment 2
<location> `.github/workflows/build.yml:141` </location>
<code_context>
+ # Create archive
+ cd ..
+ if [ "${{ runner.os }}" == "Windows" ]; then
+ 7z a audio-level-fixer-${{ matrix.config.artifact_name }}.zip audio-level-fixer-${{ matrix.config.artifact_name }}/*
+ else
+ tar -czf audio-level-fixer-${{ matrix.config.artifact_name }}.tar.gz audio-level-fixer-${{ matrix.config.artifact_name }}/
</code_context>
<issue_to_address>
The workflow assumes 7z is available on Windows runners for zipping artifacts.
Please add a step to install 7-Zip on Windows runners, or switch to a built-in zip utility to ensure compatibility.
</issue_to_address>
### Comment 3
<location> `.github/workflows/build.yml:112` </location>
<code_context>
+ else
+ echo "No tests found - skipping"
+ fi
+ continue-on-error: true
+
+ - name: Package Artifacts
</code_context>
<issue_to_address>
Tests are allowed to fail without failing the workflow.
Consider removing 'continue-on-error' or making it conditional if test failures should prevent releases.
</issue_to_address>
## Security Issues
### Issue 1
<location> `.github/workflows/build.yml:45` </location>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>
### Issue 2
<location> `.github/workflows/build.yml:67` </location>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>
### Issue 3
<location> `.github/workflows/build.yml:187` </location>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
.github/workflows/build.yml
Outdated
| - name: Build | ||
| shell: bash | ||
| run: | | ||
| cmake --build build --config Release --parallel $(nproc 2>/dev/null || echo 4) |
There was a problem hiding this comment.
issue (performance): The use of $(nproc 2>/dev/null || echo 4) for parallel builds may not work on macOS or Windows runners.
Since nproc is not available on macOS and this command fails on Windows, please use a more portable method for setting parallelism, such as a fixed value or leveraging GitHub Actions environment variables.
.github/workflows/build.yml
Outdated
| # Create archive | ||
| cd .. | ||
| if [ "${{ runner.os }}" == "Windows" ]; then | ||
| 7z a audio-level-fixer-${{ matrix.config.artifact_name }}.zip audio-level-fixer-${{ matrix.config.artifact_name }}/* |
There was a problem hiding this comment.
issue (bug_risk): The workflow assumes 7z is available on Windows runners for zipping artifacts.
Please add a step to install 7-Zip on Windows runners, or switch to a built-in zip utility to ensure compatibility.
.github/workflows/build.yml
Outdated
| else | ||
| echo "No tests found - skipping" | ||
| fi | ||
| continue-on-error: true |
There was a problem hiding this comment.
question (testing): Tests are allowed to fail without failing the workflow.
Consider removing 'continue-on-error' or making it conditional if test failures should prevent releases.
.github/workflows/build.yml
Outdated
| submodules: recursive | ||
|
|
||
| - name: Setup CMake | ||
| uses: lukka/get-cmake@latest |
There was a problem hiding this comment.
security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha): An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
Source: opengrep
.github/workflows/build.yml
Outdated
|
|
||
| - name: Setup MSVC (Windows) | ||
| if: runner.os == 'Windows' | ||
| uses: microsoft/setup-msbuild@v2 |
There was a problem hiding this comment.
security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha): An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
Source: opengrep
.github/workflows/build.yml
Outdated
| echo "> **Note:** This is an automated development build. The project is under active development." >> release-notes.md | ||
|
|
||
| - name: Create Development Release | ||
| uses: softprops/action-gh-release@v1 |
There was a problem hiding this comment.
security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha): An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
Source: opengrep
Summary by Sourcery
Add a comprehensive CMake-based GitHub Actions workflow for cross-platform builds, tests, packaging, and artifact publishing, and remove the obsolete master-build.yml.
Enhancements:
Chores: