Skip to content

Commit a7694c4

Browse files
greenc-FNALgoogle-labs-jules[bot]
authored and
knoepfel
committed
Fix workflow issues caught by actionlint (Framework-R-D#139)
* fix(workflows): Resolve actionlint warnings and logic in CI This commit addresses multiple `actionlint` warnings and improves the robustness of the CI workflows. - **clang-tidy-fix.yaml**: Mitigates a script injection warning by passing the untrusted `github.event.comment.body` through a sanitized environment variable before use in the `run` script. - **cmake-build.yaml**: Refactors the job structure to resolve property warnings and a logic flaw that prevented manual triggers from running correctly. The change separates change-detection and matrix-generation into two distinct, lightweight jobs, ensuring the build runs correctly for all trigger types. * Fix variable/command output splitting with double quotes --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent 95fc681 commit a7694c4

7 files changed

Lines changed: 33 additions & 29 deletions

File tree

‎.github/workflows/clang-tidy-check.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ jobs:
8181
shell: bash
8282
run: |
8383
. /entrypoint.sh
84-
cd $GITHUB_WORKSPACE/phlex-build
84+
cd "$GITHUB_WORKSPACE/phlex-build"
8585
8686
echo "➡️ Running clang-tidy checks..."
8787
cmake --build . --target clang-tidy-check -- --export-fixes clang-tidy-fixes.yaml > clang-tidy.log 2>&1 || true

‎.github/workflows/clang-tidy-fix.yaml‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ jobs:
2727
steps:
2828
- id: parse_comment
2929
name: Parse comment for tidy checks
30+
env:
31+
COMMENT_BODY: ${{ github.event.comment.body }}
3032
run: |
31-
checks_line=$(echo "${{ github.event.comment.body }}" | sed -nE 's/^@phlexbot[[:space:]]+tidy-fix[[:space:]]+(.*)/\1/p' | tr -d '\r')
33+
checks_line=$(echo "$COMMENT_BODY" | sed -nE 's/^@phlexbot[[:space:]]+tidy-fix[[:space:]]+(.*)/\1/p' | tr -d '\r')
3234
if [ -n "$checks_line" ]; then
33-
tidy_checks=$(echo "$checks_line" | tr ',' ' ' | xargs -n1 | paste -sd, -)
34-
echo "tidy_checks=$tidy_checks" >> $GITHUB_OUTPUT
35+
tidy_checks="$(echo "$checks_line" | tr ',' ' ' | xargs -n1 | paste -sd, -)"
36+
echo "tidy_checks=$tidy_checks" >> "$GITHUB_OUTPUT"
3537
fi
3638
- name: Get PR Info
3739
id: get_pr
@@ -69,7 +71,7 @@ jobs:
6971
PHLEX_TIDY_CHECKS: ${{ needs.parse-command.outputs.tidy_checks }}
7072
run: |
7173
. /entrypoint.sh
72-
cd $GITHUB_WORKSPACE/phlex-build
74+
cd "$GITHUB_WORKSPACE/phlex-build"
7375
7476
echo "Applying clang-tidy fixes using CMake target..."
7577
cmake --build . --target clang-tidy-fix -- --export-fixes clang-tidy-fixes.yaml || true

‎.github/workflows/cmake-build.yaml‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,13 @@ jobs:
7878
needs: pre-check
7979
if: >
8080
needs.pre-check.result == 'success' &&
81-
github.event_name != 'workflow_dispatch' &&
81+
(github.event_name == 'pull_request' || github.event_name == 'push') &&
8282
needs.pre-check.outputs.is_act != 'true'
8383
runs-on: ubuntu-latest
8484
permissions:
8585
contents: read
86-
packages: read
8786
outputs:
8887
has_changes: ${{ steps.filter.outputs.matched }}
89-
matrix: ${{ steps.generate.outputs.matrix }}
9088

9189
steps:
9290
- name: Check out source code
@@ -100,8 +98,8 @@ jobs:
10098
uses: Framework-R-D/phlex/.github/actions/detect-relevant-changes@main
10199
with:
102100
repo-path: phlex-src
103-
base-ref: ${{ github.event_name == 'issue_comment' && fromJSON(needs.pre-check.outputs.pr_details).base_sha || github.event.pull_request.base.sha || github.event.before }}
104-
head-ref: ${{ github.event_name == 'issue_comment' && fromJSON(needs.pre-check.outputs.pr_details).sha || github.event.pull_request.head.sha || github.sha }}
101+
base-ref: ${{ github.event.pull_request.base.sha || github.event.before }}
102+
head-ref: ${{ github.event.pull_request.head.sha || github.sha }}
105103
file-type: |
106104
cpp
107105
cmake
@@ -116,25 +114,28 @@ jobs:
116114
echo "::endgroup::"
117115
fi
118116
117+
generate-matrix:
118+
needs: pre-check
119+
if: needs.pre-check.result == 'success'
120+
runs-on: ubuntu-latest
121+
outputs:
122+
matrix: ${{ steps.generate.outputs.matrix }}
123+
steps:
119124
- id: generate
120125
uses: Framework-R-D/phlex/.github/actions/generate-build-matrix@main
121126
with:
122127
user-input: ${{ github.event.inputs.build-combinations }}
123128
comment-body: ${{ github.event.comment.body }}
124129

125130
build:
126-
needs: [pre-check, detect-changes]
131+
needs: [pre-check, detect-changes, generate-matrix]
127132
if: >
128133
needs.pre-check.result == 'success' &&
129134
(
130135
github.event_name == 'workflow_dispatch' ||
136+
github.event_name == 'issue_comment' ||
131137
needs.pre-check.outputs.is_act == 'true' ||
132138
(needs.detect-changes.result == 'success' && needs.detect-changes.outputs.has_changes == 'true')
133-
) &&
134-
(
135-
github.event_name != 'issue_comment' ||
136-
github.event.comment.author_association == 'COLLABORATOR' ||
137-
github.event.comment.author_association == 'OWNER'
138139
)
139140
runs-on: ubuntu-24.04
140141
strategy:
@@ -149,7 +150,8 @@ jobs:
149150
uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0
150151
with:
151152
path: phlex-src
152-
ref: ${{ github.event_name == 'issue_comment' && fromJSON(needs.build-pre-check.outputs.pr_details).sha || github.ref }}
153+
ref: ${{ needs.pre-check.outputs.sha }}
154+
repository: ${{ needs.pre-check.outputs.repo }}
153155

154156
- name: Setup build environment
155157
uses: Framework-R-D/phlex/.github/actions/setup-build-env@main
@@ -175,11 +177,11 @@ jobs:
175177
if: matrix.sanitizer != 'valgrind'
176178
run: |
177179
. /entrypoint.sh
178-
cd $GITHUB_WORKSPACE/phlex-build
180+
cd "$GITHUB_WORKSPACE/phlex-build"
179181
180182
echo "➡️ Running tests..."
181183
echo "::group::Running ctest"
182-
if ctest --progress --output-on-failure -j $(nproc); then
184+
if ctest --progress --output-on-failure -j "$(nproc)"; then
183185
echo "::endgroup::"
184186
echo "✅ All tests passed."
185187
else
@@ -192,7 +194,7 @@ jobs:
192194
if: matrix.sanitizer == 'valgrind'
193195
run: |
194196
. /entrypoint.sh
195-
cd $GITHUB_WORKSPACE/phlex-build
197+
cd "$GITHUB_WORKSPACE/phlex-build"
196198
197199
echo "➡️ Running Valgrind tests..."
198200
echo "::group::Running ctest -T memcheck"

‎.github/workflows/coverage.yaml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ jobs:
151151
- name: Run tests with coverage
152152
run: |
153153
. /entrypoint.sh
154-
cd $GITHUB_WORKSPACE/phlex-build
154+
cd "$GITHUB_WORKSPACE/phlex-build"
155155
156156
echo "➡️ Running tests with coverage..."
157157
PROFILE_ROOT="$GITHUB_WORKSPACE/phlex-build/test/profraw"
@@ -161,7 +161,7 @@ jobs:
161161
export LLVM_PROFILE_FILE="$PROFILE_ROOT/%m-%p.profraw"
162162
163163
echo "::group::Running ctest for coverage"
164-
if ctest --progress --output-on-failure -j $(nproc); then
164+
if ctest --progress --output-on-failure -j "$(nproc)"; then
165165
echo "::endgroup::"
166166
echo "✅ All tests passed."
167167
else
@@ -176,7 +176,7 @@ jobs:
176176
shell: bash
177177
run: |
178178
. /entrypoint.sh
179-
cd $GITHUB_WORKSPACE/phlex-build
179+
cd "$GITHUB_WORKSPACE/phlex-build"
180180
181181
echo "➡️ Generating coverage reports for GCC..."
182182
echo "::group::Running coverage-gcov target"
@@ -195,7 +195,7 @@ jobs:
195195
shell: bash
196196
run: |
197197
. /entrypoint.sh
198-
cd $GITHUB_WORKSPACE/phlex-build
198+
cd "$GITHUB_WORKSPACE/phlex-build"
199199
200200
echo "➡️ Generating coverage reports for Clang..."
201201
echo "::group::Running coverage-llvm target"

‎test/form/data_products/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ add_library(
33
${FORM_DATA_PROD_LIB_NAME} SHARED track_start.cpp
44
) # REFLEX_GENERATE_DICTIONARY doesn't work trivially without making a shared
55
# library
6-
target_link_libraries(${FORM_DATA_PROD_LIB_NAME})
6+
target_link_libraries(${FORM_DATA_PROD_LIB_NAME} ROOT::RIO)
77

88
if(FORM_USE_ROOT_STORAGE)
99
find_package(ROOT REQUIRED COMPONENTS Core RIO)

‎test/form/reader.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ int main(int /* argc*/, char** /* argv[]*/)
4343
// Processing per sub-event
4444
std::vector<float> const* track_start_x = nullptr;
4545
char seg_id_text[64];
46-
sprintf(seg_id_text, seg_id, nevent, nseg);
46+
snprintf(seg_id_text, 64, seg_id, nevent, nseg);
4747
std::string const creator = "Toy_Tracker";
4848
mock_phlex::product_base pb = {
4949
"trackStart", seg_id_text, track_start_x, std::type_index{typeid(std::vector<float>)}};
@@ -87,7 +87,7 @@ int main(int /* argc*/, char** /* argv[]*/)
8787
std::cout << "PHLEX: Read Event segments done " << nevent << std::endl;
8888

8989
char evt_id_text[64];
90-
sprintf(evt_id_text, evt_id, nevent);
90+
snprintf(evt_id_text, 64, evt_id, nevent);
9191
std::string const creator = "Toy_Tracker_Event";
9292
mock_phlex::product_base pb = {
9393
"trackStartX", evt_id_text, track_x, std::type_index{typeid(std::vector<float>)}};

‎test/form/writer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ int main(int /*argc*/, char** /* argv[]*/)
7272
// done, phlex call write(mock_phlex::product_base)
7373
// sub-event writing called by phlex
7474
char seg_id_text[64];
75-
sprintf(seg_id_text, seg_id, nevent, nseg);
75+
snprintf(seg_id_text, 64, seg_id, nevent, nseg);
7676
std::vector<mock_phlex::product_base> batch;
7777
std::string const creator = "Toy_Tracker";
7878
mock_phlex::product_base pb = {
@@ -122,7 +122,7 @@ int main(int /*argc*/, char** /* argv[]*/)
122122

123123
// event writing, current framework, will also write references
124124
char evt_id_text[64];
125-
sprintf(evt_id_text, evt_id, nevent);
125+
snprintf(evt_id_text, 64, evt_id, nevent);
126126
std::string const creator = "Toy_Tracker_Event";
127127
mock_phlex::product_base pb = {
128128
"trackStartX", evt_id_text, &track_x, std::type_index{typeid(std::vector<float>)}};

0 commit comments

Comments
 (0)