aminya / setup-cpp

Install all the tools required for building and testing C++/C projects.
Apache License 2.0
195 stars 31 forks source link

The setup-cpp action wrongly installs Windows LLVMs to the same folder #249

Closed FeignClaims closed 1 month ago

FeignClaims commented 5 months ago

From the log of (windows-2022, llvm-16.0.6, , ) jobs, for the setup-cpp action specified as

- name: Setup cpp
  uses: aminya/setup-cpp@v0.37.0
  with:
    compiler: llvm-16.0.6
    vcvarsall: true

    cmake: true
    ninja: true
    ccache: true
    python: true

    cppcheck: true
    clangtidy: true

    gcovr: 7.2 # The default version 5.2 stucks on macos gcc
    OPENCPPCOVERAGE: TRUE

clang-tidy triggers an installation of llvm 17.0.6 (see Setup cpp#472). After that, llvm 16.0.6 is triggered to be installed to the same folder (see Setup cpp#554), which is, however, not installed according to the log.

To further ensure that llvm 16.0.6 is not installed, I directly specify the clang version to 16.0.6 in the corresponding conan profile, but cmake still uses llvm-17.0.6 according to Configure cmake#76.

Upvote & Fund

@aminya is using Polar.sh so you can upvote and help fund this issue. The funding is received once the issue is completed & confirmed by you.

Thank you in advance for helping prioritize & fund our backlog!


Fund with Polar

aminya commented 5 months ago

true triggers the default version which is 17. But when you specify the compiler, it installs 16. So, either remove the clang-tidy specification or use the same version for both.

FeignClaims commented 5 months ago

true triggers the default version which is 17. But when you specify the compiler, it installs 16.

That's what I intended to do, and worked well on (ubuntu-22.04, llvm-16.0.6, , ). Therefore I opened this issue to know whether the behavior of intalling different versions of Windows LLVM to the same folder is intentional.

So, either remove the clang-tidy specification or use the same version for both.

I thought the separated compiler: llvm-17.0.6 and clang-tidy: true options implies that these options are independent from each other. That's why I used different LLVM versions on purpose.

Could this be fixed by installing Windows LLVMs to different folders, or simply by reporting an error when the clang-tidy and llvm version don't match?

aminya commented 5 months ago

The code here hands syncing the version for these tools. It can be improved to use the specific version. I think the reason it doesn't work is that compiler: llvm-17.0.6 doesn't populate the llvm field in the options.

https://github.com/aminya/setup-cpp/blob/34bb7838132719f0db8a21d1b1c805d39f21977d/src/main.ts#L59

We should probably parse the compiler field and inform the sync logic about that. Then later install the parsed information. A tuple of compiler + version and separating this switch case into a separate function should be good enough https://github.com/aminya/setup-cpp/blob/34bb7838132719f0db8a21d1b1c805d39f21977d/src/compilers.ts#L44