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

Add clang tidy workflow #1031

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add clang tidy workflow #1031

wants to merge 1 commit into from

Conversation

mcbarton
Copy link

@mcbarton mcbarton commented Nov 29, 2024

@vgvassilev here is the PR to try and add the clang tidy workflow to this repo. Can you activate the workflow to see whether it works?

@mcbarton
Copy link
Author

@vgvassilev I got the workflow to start, but it couldn't complete as it couldn't find header files (I believe this may be fixed with the bash set_conda_env_vars.sh command). Can you approve the workflow to run so I can see if this fixes it?

@mcbarton
Copy link
Author

mcbarton commented Nov 29, 2024

@anigamova can you point me to the instructions on how to compile using Conda? I get the following error trying to add in the clang tidy workflow

Error while trying to load a compilation database:
Line filter for clang-tidy:
Could not auto-detect compilation database from directory "build"

Do I need to pass an option to cmake so it can find the compilation database?

@mcbarton
Copy link
Author

mcbarton commented Nov 29, 2024

@anigamova can you point me to the instructions on how to compile using Conda? I get the following error trying to add in the clang tidy workflow

Error while trying to load a compilation database:
Line filter for clang-tidy:
Could not auto-detect compilation database from directory "build"

Do I need to pass an option to cmake so it can find the compilation database?

@anigamova @vgvassilev I realised this wasn't the error I needed to fix. I forgot to install wget to able to download micromamba. Are you able to approval it for the workflow to run?

@vgvassilev
Copy link

I can't, it's @anigamova's land ;)

@mcbarton
Copy link
Author

mcbarton commented Nov 29, 2024

https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/actions/runs/12089671964/job/33717942489?pr=1031#step:7:932 - Not entirely sure what has gone wrong here in activating the conda environment. Will investigate tomorrow.

For reference here is the same point in the xeus-cpp clang tidy workflow, which also using a conda environment - https://github.com/compiler-research/xeus-cpp/actions/runs/12087483631/job/33709016293?pr=188#step:6:790

@vgvassilev
Copy link

Maybe it needs to be merged to be able to comment…

@mcbarton
Copy link
Author

mcbarton commented Nov 29, 2024

@vgvassilev no that's not the issue. If you look at the output from the clang tidy review workflow running at the moment it fails on the following line micromamba activate HIGGSANALYSIS-COMBINED-LIMIT . It doesn't get to the point yet where it produces the report at the moment, in order to be able to comment.

To look at the failure message expand the part of the workflow called run clang-tidy, and then expand this section (line 134 of the run clang-tidy part of the workflow)

Running cmake: cmake . -B build -DINSTALL_PYTHON=FALSE -DCMAKE_EXPORT_COMPILE_COMMANDS=ON || true &&  
rm -rf build &&  set -x &&  
wget https://github.com/mamba-org/micromamba-releases/releases/download/1.5.10-0/micromamba-linux-64 && 
 mv ./micromamba-linux-64 ./micromamba &&  
 eval "$(micromamba shell hook -s posix)" &&  
 micromamba create -n HIGGSANALYSIS-COMBINED-LIMIT -y --log-level warning -f $GITHUB_WORKSPACE/conda_env.yml &&  
 micromamba activate HIGGSANALYSIS-COMBINED-LIMIT &&  
 bash set_conda_env_vars.sh &&  
 cmake . -B build -DINSTALL_PYTHON=FALSE -DCMAKE_EXPORT_COMPILE_COMMANDS=ON || true

@mcbarton
Copy link
Author

@anigamova can you approve the workflow run?

1 similar comment
@mcbarton
Copy link
Author

mcbarton commented Dec 2, 2024

@anigamova can you approve the workflow run?

@mcbarton
Copy link
Author

mcbarton commented Dec 2, 2024

@anigamova I have put in a debug commit to try and see exactly what is the line of the script it is complaining about.

@mcbarton
Copy link
Author

mcbarton commented Dec 2, 2024

@anigamova I think the issue at the moment is the warning about gcc already being present in the environment, so I am trying to remove it before we create and activate the environment.

@mcbarton
Copy link
Author

mcbarton commented Dec 3, 2024

@vgvassilev @anigamova After some debugging (effectively have to just avoid trying to activate the created environment in this workflow for some unknown reason) I finally got something which would start installing the necessary packages to build this repo, but the ROOT it installs is missing a particular component.

  CMake Error at /github/home/micromamba/envs/clang-tidy-review/cmake/ROOTConfig.cmake:159 (message):
  -- Configuring incomplete, errors occurred!
    ROOT component RooFitHS3 not found
  Call Stack (most recent call first):

What do I need to add to get this component of ROOT?

@vgvassilev
Copy link

RooFitHS3

That's back to @guitargeek...

@mcbarton
Copy link
Author

@guitargeek ping (about the RooFitHS3 question)

@guitargeek
Copy link
Contributor

Hi @mcbarton! The RooFitHS3 library should not even be linked. Can you try what happens if you apply the same changes as in this PR?

#1034

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.

3 participants