You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "philo-he (via GitHub)" <gi...@apache.org> on 2026/08/07 23:21:32 UTC

[PR] [VL] Run clang-tidy in the native build job and narrow its check list [gluten]

philo-he opened a new pull request, #12729:
URL: https://github.com/apache/gluten/pull/12729

   <!--
   Thank you for submitting a pull request! Here are some tips:
   
   1. For first-time contributors, please read our contributing guide:
      https://github.com/apache/gluten/blob/main/CONTRIBUTING.md
   2. If necessary, create a GitHub issue for discussion beforehand to avoid duplicate work.
   3. If the PR is specific to a single backend, include [VL] or [CH] in the PR title to indicate the
      Velox or ClickHouse backend, respectively.
   4. If the PR is not ready for review, please mark it as a draft.
   -->
   
   ## What changes are proposed in this pull request?
   The clang-tidy check ran in cpp_clang_tidy.yml, a workflow_run job that consumed the native build artifact from another workflow. That could not work: only cpp/build/ is uploaded, so the compilation database's -I paths into ep/build-velox/ and dev/vcpkg/vcpkg_installed/ did not exist in the downstream job.
   
   The check now runs in the existing `cpp-test-udf-test` job, which builds the native libraries in the same container and workspace, so the database and full header tree are already present.
   
   <!--
   Provide a clear and concise description of the changes introduced in this PR.
   Ensure the PR description aligns with the code changes, especially after updates.
   If applicable, include "Fixes #<GitHub_Issue_ID>" to automatically close the corresponding issue
   when the PR is merged.
   -->
   
   ## How was this patch tested?
   Depends on CI.
   <!--
   Describe how the changes were tested, if applicable.
   Include new tests to validate the functionality, if necessary.
   For UI-related changes, attach screenshots to demonstrate the updates.
   -->
   
   ## Was this patch authored or co-authored using generative AI tooling?
   Generated-by: Claude Opus 5
   
   <!--
   If generative AI tooling has been used in the process of authoring this patch, please include the
   phrase: 'Generated-by: ' followed by the name of the tool and its version.
   If no, write 'No'.
   Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [VL] Run clang-tidy in the native build job and narrow its check list [gluten]

Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan merged PR #12729:
URL: https://github.com/apache/gluten/pull/12729


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [VL] Run clang-tidy in the native build job and narrow its check list [gluten]

Posted by "prestodb-ci (via GitHub)" <gi...@apache.org>.
prestodb-ci commented on PR #12729:
URL: https://github.com/apache/gluten/pull/12729#issuecomment-5264135279

   Kicking off rebase job, please wait...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [VL] Run clang-tidy in the native build job and narrow its check list [gluten]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12729:
URL: https://github.com/apache/gluten/pull/12729#discussion_r3741854684


##########
dev/run-clang-tidy.py:
##########
@@ -128,12 +83,15 @@ def tidy(args):
     fix = "--fix" if args.fix == "fix" else ""
     files = args.files
 
+    # Checks come from 'cpp/.clang-tidy'; passing '--checks' here would override it.
+    # CLANG_TIDY_EXTRA_ARGS forwards toolchain details the compilation database omits.
+    extra_args = os.environ.get("CLANG_TIDY_EXTRA_ARGS", "").strip()
+

Review Comment:
   `CLANG_TIDY_EXTRA_ARGS` is interpolated directly into a shell command (`shell=True`). If it contains shell metacharacters or newlines, it can break the invocation (and, outside CI, becomes a command-injection footgun). Consider normalizing it via `shlex.split` and re-quoting each token before formatting it into the command string.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [VL] Run clang-tidy in the native build job and narrow its check list [gluten]

Posted by "philo-he (via GitHub)" <gi...@apache.org>.
philo-he commented on PR #12729:
URL: https://github.com/apache/gluten/pull/12729#issuecomment-5244291351

   cc @zhouyuan


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org