Skip to content

ci: replace cpp-linter-action with cpp-linter CLI#838

Open
LuciferYang wants to merge 2 commits into
apache:mainfrom
LuciferYang:replace-cpp-linter-action
Open

ci: replace cpp-linter-action with cpp-linter CLI#838
LuciferYang wants to merge 2 commits into
apache:mainfrom
LuciferYang:replace-cpp-linter-action

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What

cpp-linter/cpp-linter-action is frozen at v2.15.1. Its v2.16 release pulled in
an untrusted dependency and was blocked by ASF Infra
(apache/infrastructure-actions#325), and the ASF gateway ignores newer versions.
The pinned action still runs, but it can no longer be updated.

How

The action is a thin wrapper around the cpp-linter PyPI package, and the block
does not touch that package. This installs it plus matching clang tools with
pip and calls the cpp-linter CLI with the same options the action used. The
checks-failed output and the Fail fast step stay as they were. Because it is a
run: step and not a uses: reference, it adds no new third-party action to the
ASF allowlist.

I also looked at emitting SARIF and uploading it with
github/codeql-action/upload-sarif, which is allowed since it is a github/*
action. That route loses the thread comment and needs an extra conversion step,
so calling the CLI directly stays closer to what we have now.

Verified

I ran the CLI locally against this repo's .clang-format and .clang-tidy on a
file containing a NULL and a mis-named member. Both clang-format and
clang-tidy ran, emitted ::warning annotations, and wrote checks-failed to
GITHUB_OUTPUT. The PR also carries a temporary probe commit that trips the
linter on purpose so CI exercises the new path; I will drop it before merge.


This contribution was authored with assistance from Claude Opus 4.8, in line with
the Iceberg guidelines for AI-assisted contributions
(https://iceberg.apache.org/contribute/#guidelines-for-ai-assisted-contributions).
All changes were reviewed and tested by the author.

Closes #336

cpp-linter/cpp-linter-action is frozen at v2.15.1: v2.16+ introduced an
untrusted dependency and was blocked by ASF Infra
(apache/infrastructure-actions#325), and the ASF gateway ignores newer
versions. The pinned action still runs but can no longer be updated.

The action is a thin wrapper around the cpp-linter PyPI package, which is not
affected by the block. Install that package plus matching clang tools with pip
and call the cpp-linter CLI with the same options the action used, keeping the
checks-failed output and the Fail fast step. This is a run: step rather than a
uses: reference, so it adds no new third-party action to the ASF allowlist.

Fixes apache#336.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the C++ linter workflow to stop using the frozen cpp-linter/cpp-linter-action wrapper and instead install and run the cpp-linter PyPI CLI directly, keeping the existing “checks-failed” / fail-fast behavior while avoiding adding a new third‑party action reference.

Changes:

  • Replace cpp-linter/cpp-linter-action with a setup-python + pip install + cpp-linter CLI invocation in the CI workflow.
  • Add a temporary probe C++ source file intended to intentionally trigger clang-tidy/naming warnings to exercise the new linter path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/iceberg/temp_linter_probe.cc Adds a temporary intentionally-noncompliant file to force the linter to report findings.
.github/workflows/cpp-linter.yml Switches from the GitHub Action wrapper to installing and running the cpp-linter CLI with equivalent arguments.

Comment thread src/iceberg/temp_linter_probe.cc Outdated
Comment on lines +1 to +2
// TEMP verification file — will be dropped before merge.
// Intentionally violates .clang-tidy to confirm the new cpp-linter CLI runs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a temporary test for lint validation and will be deleted after testing.

@LuciferYang LuciferYang Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

It looks like the check is taking effect. I will revert this file and run the tests again.

Comment thread .github/workflows/cpp-linter.yml Outdated
Comment on lines +94 to +98
- uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
if: github.event_name == 'pull_request'
- name: Install cpp-linter and clang tools
if: github.event_name == 'pull_request'
run: pip install "cpp-linter==1.13.0" "clang-format==22.1.8" "clang-tidy==22.1.8"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, done. Pinned setup-python to 3.13 and switched to python -m pip install so it uses the interpreter the action set up. I kept the run step as cpp-linter (its console entry point) since the package has no __main__, so it can't be called with python -m; it's installed into the same interpreter, so they stay in sync.

Comment thread src/iceberg/temp_linter_probe.cc Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 06:51
@LuciferYang
LuciferYang force-pushed the replace-cpp-linter-action branch from 7889803 to 88d5abc Compare July 22, 2026 06:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

.github/workflows/cpp-linter.yml:98

  • actions/setup-python is invoked without specifying python-version, which makes the job dependent on the runner’s preinstalled Python (and which pip it provides). That can change over time and can also result in pip resolving to a different interpreter than the one setup-python put on PATH. Specify a Python version (consistent with other workflows) and use python -m pip for the installs.
      - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
        if: github.event_name == 'pull_request'
      - name: Install cpp-linter and clang tools
        if: github.event_name == 'pull_request'
        run: pip install "cpp-linter==1.13.0" "clang-format==22.1.8" "clang-tidy==22.1.8"

Address review feedback: pin setup-python to 3.13 so the runner's default
Python cannot drift, and install with python -m pip so it targets the
interpreter the action set up rather than a bare pip on PATH.
Copilot AI review requested due to automatic review settings July 22, 2026 06:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines +94 to +98
- uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
if: github.event_name == 'pull_request'
with:
python-version: '3.13'
- name: Install cpp-linter and clang tools

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#838 (comment)

have previously tested this by introducing a faulty temporary file.

@LuciferYang

Copy link
Copy Markdown
Contributor Author

cc @wgtmac

@wgtmac wgtmac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! Thanks a lot, @LuciferYang!

python-version: '3.13'
- name: Install cpp-linter and clang tools
if: github.event_name == 'pull_request'
run: python -m pip install "cpp-linter==1.13.0" "clang-format==22.1.8" "clang-tidy==22.1.8"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we parameterize these versions somewhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Find alternative to cpp-linter/cpp-linter-action

3 participants