ci: Update workflow to use pre-built docker containers - Part 2 #895
Conversation
.github/workflows/distcheck.yaml
Outdated
|
|
||
| - name: Call `autoreconf -ivf` | ||
| shell: bash |
There was a problem hiding this comment.
why do we suddenly need this?
There was a problem hiding this comment.
I added "shell: bash" because matrix variable comparisons weren't working correctly in the container environment. For example:
if [ "${{ matrix.sdk }}" = "neuron" ]
wasn't being evaluated properly with the container's default shell.
But I moved this setting to workflow-level default instead of specifying it per step:
defaults:
run:
shell: bash
There was a problem hiding this comment.
- what's the default shell?
- What changed that this is a problem?
There was a problem hiding this comment.
-
What's the default shell?
In GitHub runners (Ubuntu 22.04): Default is/bin/bash
In Docker containers: Default is/bin/sh -
What changed that this is a problem?
Previously, we were running directly on GitHub runners where/bin/bashwas the default shell. When we moved to containerized builds, we're now running in containers where /bin/sh is the default shell. This becomes a problem because:
The string comparison == operator works in bash but not in sh
Our matrix variable evaluations like:
if [ "${{ matrix.sdk }}" == "neuron" ]
were using bash-specific syntax that isn't POSIX-compliant
The solution was to either:
- Explicitly specify shell: bash to maintain the same shell environment we had before
- Use POSIX-compliant syntax (= instead of ==)
Here I have add shell: bash to maintain consistency with our previous environment and ensure all bash features are available.
There was a problem hiding this comment.
Ack. I have made the change (2).
arunkarthik-akkart
force-pushed
the
ci/containerize-ci
branch
4 times, most recently
from
f9b7125 to
342a453
Compare
|
Also, please add the obligatory "Signed-off-by" to your commit message. |
Currently the github actions workflow installs all the packages during runtime. Instead we want to install all the packages except aws-ofi-nccl plugin within a docker container and use this container during runtime. This commit updates the PR-CI to use the pre-built containers instead of installing the stack during runtime. Signed-off-by: Arun Karthik <[email protected]>
arunkarthik-akkart
force-pushed
the
ci/containerize-ci
branch
from
342a453 to
220cad2
Compare
Done |
|
bot:aws:retest |
|
bot:aws:retest |
| env: | ||
| CC: clang | ||
| CXX: clang++ | ||
| run: | | ||
| # Ensure exit on real errors, but continue on exit code 2 | ||
| set +e | ||
|
|
||
| # Create directories first | ||
| mkdir -p /workspace/codechecker_Results | ||
| mkdir -p /workspace/codechecker_Results_HTML | ||
| mkdir -p ./codechecker_Results_HTML | ||
|
|
||
| # Step 1: Log command | ||
| CodeChecker log \ | ||
| -o /workspace/codechecker_Results.json \ | ||
| -b "make V=1" | ||
| if [ $? -ne 0 ]; then exit 1; fi | ||
|
|
||
| # Step 2: Analyze command | ||
| CodeChecker analyze \ | ||
| /workspace/codechecker_Results.json \ | ||
| -o /workspace/codechecker_Results \ | ||
| --jobs "$(nproc)" \ | ||
| --config .github/codechecker.yaml \ | ||
| --ctu \ | ||
| --ctu-all | ||
| if [ $? -ne 0 ]; then exit 1; fi | ||
|
|
||
| # Step 3: Parse command - allow exit code 2 | ||
| CodeChecker parse \ | ||
| /workspace/codechecker_Results \ | ||
| --export html \ | ||
| --output /workspace/codechecker_Results_HTML | ||
| parse_exit=$? | ||
| if [ $parse_exit -ne 0 ] && [ $parse_exit -ne 2 ]; then | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
I understand removing setup codes as that is now embedded into our docker image, but how come there's a need to add additional run's specifically for the CodeChecker workflow?
Is the new docker image incompatible with whisperity/codechecker-analysis-action?
Description of changes:
Currently the github actions workflow installs all the packages during runtime. Instead we want to install all the packages except aws-ofi-nccl plugin within a docker container and use this container during runtime.
This commit updates the PR-CI to use the pre-built containers instead of installing the stack during runtime.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.