[ISSUE #4737] Separate codeql workflow#4740
Conversation
| # queries: ./path/to/local/query, your-org/your-repo/queries@main | ||
| languages: ${{ matrix.language }} | ||
|
|
||
| - if: matrix.language == 'cpp' || matrix.language == 'csharp' |
There was a problem hiding this comment.
This step seemed like it would never execute, as neither of these languages was in the matrix, so I did not migrate it to the CodeQL workflow.
There was a problem hiding this comment.
You mean L52~L57 task Build C or L50 languages: ${{ matrix.language }}?
There was a problem hiding this comment.
Build C task was added in #4543. It seems intentional to skip the Build C task, and there is further room for optimization (such as submodules: true not added yet) in this area. We have Git submodules in our C SDK, and we want to test these C codes.
There was a problem hiding this comment.
You mean L52~L57 task
Yes, the conditional check for cpp/csharp
Build C task was added in #4543. It seems intentional to skip the Build C task, and there is further room for optimization (such as submodules: true not added yet) in this area. We have Git submodules in our C SDK, and we want to test these C codes.
I'm not quite sure what you mean here, since the way this is currently authored, matrix.language will never be either of these two options. So this is essentially a dead code path as it exists today. But I can add it back if it is an area of future change.
There was a problem hiding this comment.
I have tried to run Build C with an additional submodule: true param and it encountered a compile error: https://github.com/Pil0tXia/eventmesh/actions/runs/7506142011/job/20436880958. Let's keep the original Build C task and I'll address it in #4742 and #4743.
|
|
||
| # https://docs.gradle.org/current/userguide/performance.html | ||
| - name: Build | ||
| run: ./gradlew clean assemble compileTestJava --no-build-cache --parallel --daemon |
There was a problem hiding this comment.
I am not familiar with your usage of CodeQL, so I made the assumption that you used it to scan test code as well. If you do not, the compileTestJava task can be removed here.
There was a problem hiding this comment.
Based on our previous experience, we would like to execute the build task. The build task should already include the assemble and compileTestJava tasks. In your experience, do you think it's necessary to replace the build task with the assemble and compileTestJava tasks?
There was a problem hiding this comment.
build will run everything in the build, most notably the tests. The tests will be verified in the Continuous Integration workflow, so running them here would be redundant (unless I misunderstand and CodeQL needs to see them execute). Running assemble compileTestJava will just compile main and test source set that CodeQL cares about.
If you find it better, we could change this to something like build -x test for a similar result.
There was a problem hiding this comment.
OK, let's keep assemble compileTestJava then. This way, only the source code will be compiled, but CodeQL doesn't need them to run.
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4740 +/- ##
============================================
- Coverage 17.60% 17.59% -0.02%
+ Complexity 1775 1774 -1
============================================
Files 797 797
Lines 29786 29786
Branches 2573 2573
============================================
- Hits 5243 5240 -3
- Misses 24063 24065 +2
- Partials 480 481 +1 ☔ View full report in Codecov by Sentry. |
* Add GRADLE_ENTERPRISE_ACCESS_KEY to a couple of Gradle builds missing it * Update to most recent Gradle Enterprise Gradle Plugin * Move CodeQL verification to its own worfklow * Rename CodeQL workflow to "Analyze" * Rename "Build" workflow to "Docker" * Restore the language matrix with 'java' as the only language in ci workflow * Return the cpp/csharp language step in ci workflow * Rename CodeQL job to "Analyze" rather than "Build"
3 participants
Fixes #4737.
Motivation
The motivation for this PR is to fix an issue where CodeQL is not able to analyze a PR when the Gradle build cache prevents all Java compilation. CodeQL requires the compiler to execute during the workflow in order to perform its analysis, as documented here.
Modifications
This pull request moves the CodeQL verification to its own workflow, independent of the "Continuous Integration" workflow, where Gradle build caching is disabled. This not only addresses the issue, but should also have some improvements to the workflows:
Documentation