Skip to content

Conversation

mgovers
Copy link
Member

@mgovers mgovers commented May 14, 2024

cfr. https://youtu.be/xSrEHZ6Sgfg?si=et5_SnnpXRpxXE6A

it is good practice if you have potential users building from source that are not interested in things like tests, compiler warnings, etc. That is: the build should pass with default compiler settings initiated by

cmake -B build/ -S .
cmake --build build/

and it should do so without any compiler warnings. Even more so, if one provides an install prefix or has the correct (admin) privileges for the default install prefix, installing should also pass. E.g.:

cmake -B build/ -S . -DCMAKE_INSTALL_PREFIX=install
cmake --build build/
cmake --build build/ --target install

This also FINALLY disables the -Wunused-attribute warning that ClangCL raises.

TBD: should we have (periodically scheduled?) builds that check whether these default builds pass?

mgovers added 3 commits May 14, 2024 10:14
Signed-off-by: Martijn Govers <[email protected]>
Signed-off-by: Martijn Govers <[email protected]>
@mgovers mgovers requested a review from TonyXiang8787 May 14, 2024 08:22
@mgovers mgovers self-assigned this May 14, 2024
@mgovers mgovers added the improvement Improvement on internal implementation label May 14, 2024
@mgovers
Copy link
Member Author

mgovers commented May 14, 2024

FYI: I did the following tests:

  • Windows
    • default generator (Visual Studio Solution)
      • MSVC
        • cmake -B build -S . and cmake --build build
          • from x64 native tools command prompt
          • from default powershell
        • with installation cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install and cmake --build build --target install
          • from x64 native tools command prompt
      • ClangCL
        • cmake -B build -S . -T ClangCL and cmake --build build
          • from x64 native tools command prompt
          • from default powershell
        • with installation cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install and cmake --build build --target install
          • from x64 native tools command prompt
    • Ninja
      • MSVC
        • cmake -B build -S . -G Ninja and cmake --build build
          • from x64 native tools command prompt
          • from default powershell
      • ClangCL
        • cmake -B build -S . -G Ninja -DCMAKE_C_COMPILER="clang-cl.exe" -DCMAKE_CXX_COMPILER="clang-cl.exe" and cmake --build build
          • from x64 native tools command prompt
          • [ ] from default powershell not possible due to environment
  • Linux (WSL2)
    • default generator (ninja)
      • Clang (via source + CC and CXX environment variables)
        • cmake -B build -S . and cmake --build build
        • with installation cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install and cmake --build build --target install
      • GCC (via source + CC and CXX environment variables)
        • cmake -B build -S . and cmake --build build
        • with installation cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install and cmake --build build --target install

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@mgovers mgovers added this pull request to the merge queue May 14, 2024
Merged via the queue into main with commit 1277eec May 14, 2024
@mgovers mgovers deleted the feature/default-build branch May 14, 2024 10:10
@mgovers mgovers mentioned this pull request Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement on internal implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants