Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang 19 #125

Closed
wants to merge 7 commits into from
Closed

clang 19 #125

wants to merge 7 commits into from

Conversation

eljonny
Copy link
Contributor

@eljonny eljonny commented Feb 19, 2025

This PR does not address the issue in #124, but it updates the Action to use Clang 19.
Some of my changes are fairly opinionated and I wouldn't want you to take them as they are (unless they seem ok to you); you can check out the corresponding commits for some additional information.
Most of the static_analysis.dockerfile changes are based on my research about dockerfiles (since I'm relatively new to them) and helped me to understand, speed up, and follow the build process.
I have confirmed that these changes work here in my first run with the code from this branch in a pre-release version of the marketplace action I published, which uses a docker image built with the updated static_analysis.dockerfile here.
I really didn't have to change very much, but CMake takes a long time to compile so it took a bit to be able to really test it for real with my personal project, in which I'm using your Action for C++ analysis with clang-tidy and cppcheck.

I don't know how to do a PR to a new branch on your repo that doesn't exist yet, as I'm not sure these changes should go into master, but it was the only option.

eljonny and others added 6 commits February 17, 2025 23:11
Update to clang-19
Update to Clang-19
Dockerfile for static_analysis container (not for the GH Action):
 - Fix case Docker build warning, FROM and AS are now both upper case
 - Split ENVs up into per-item, I felt it was easier to read
 - Split RUNs up into per-command (for the most part, there are still a
    few multi-command RUNs), this helps with caching in the Docker
    build process and I feel makes it more clear what is happening and
    when, with cd replaced with WORKDIR so the Docker build process is
    controlling where commands are run.
 - Add --chmod to LLVM script COPY command with permissions so the
    build can run the script and we no longer need a chmod command
    prior to running the script
 - RUN the script with ./ instead of bash -c because for some reason it
    just will not work.

Updated the LLVM install file with the latest from https://apt.llvm.org
 - The updated script added the option to specify all as an option to
    install all LLVM packages as opposed to the base set of LLVM
    packages, which added a set of extra packages behind a conditional
    on the ALL flag (specified as all as a flag), with some additional
    stuff for LLVM 14.
 - Some quotes removed here and there.
 - Updated the base set of LLVM packages to include clang-tidy, as the
    updated base packages set did not include it, so this is different
    compared to the JD version and the previous version.
 - Updated script includes an updated LLVM_VERSION_PATTERNS that added
    versions 19 and 20, with a placeholder for 21.
 - Updated script added || true to the apt-key del AF4F7421 command.
 - Updated script added a workaround for
    llvm/llvm-project#62475
@eljonny
Copy link
Contributor Author

eljonny commented Feb 19, 2025

It looks like the Linter failure is a permissions issue on the repo with the Action.

@eljonny
Copy link
Contributor Author

eljonny commented Feb 19, 2025

The Test Action failure is less clear.
I'm not sure what's going on here.
logs_34571592408.zip

@JacobDomagala
Copy link
Owner

Tools updated, closing this one.

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.

2 participants