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

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

Open
FeignClaims opened this issue May 15, 2024 · 3 comments
Open
Labels
bug Something isn't working

Comments

@FeignClaims
Copy link

FeignClaims commented May 15, 2024

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 aminya added bug Something isn't working and removed bug Something isn't working labels May 15, 2024
@aminya
Copy link
Owner

aminya commented May 15, 2024

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
Copy link
Author

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
Copy link
Owner

aminya commented May 15, 2024

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.

if (!syncVersions(opts, ["llvm", "clangtidy", "clangformat"])) {

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

switch (compiler) {

@aminya aminya added the bug Something isn't working label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants