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

Bump timsort to 3.0.0 #22516

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Bump timsort to 3.0.0 #22516

wants to merge 6 commits into from

Conversation

Morwenn
Copy link
Contributor

@Morwenn Morwenn commented Jan 23, 2024

Specify library name and version: timsort/3.0.0

Version 3.0.0 requires C++20, so the automatic update was not enough to automatically bump the library version.


@CLAassistant
Copy link

CLAassistant commented Jan 23, 2024

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@toge
Copy link
Contributor

toge commented Jan 24, 2024

@Morwenn
To solve CI errors, this changes can help.
https://github.com/conan-io/conan-center-index/pull/22517/files

these changes might help to solve CI errors.

  • validate compiler version
  • specify c++ version on CMakeLists.txt of test_package

@Morwenn
Copy link
Contributor Author

Morwenn commented Jan 24, 2024

Thanks, I'm going to try those later tonight.

@conan-center-bot

This comment has been minimized.

@Morwenn
Copy link
Contributor Author

Morwenn commented Jan 25, 2024

I think it doesn't work because <ranges> is only truly available in libc++ 15, it's experimental before - so it could work with Clang < 15 but only if it uses libstdc++. I supposed that it failed there because AppleClang defaults to libc++. Is there anyway I can detect that to fix the recipe?

@toge
Copy link
Contributor

toge commented Jan 26, 2024

@Morwenn
How about incrementing apple-clang version:

                "apple-clang": "14",

In cpp reference, apple-clang seems to support ranges since 14.

I confirmed timsort/3.0.0 works well on apple-clang 15.

@Morwenn
Copy link
Contributor Author

Morwenn commented Jan 27, 2024

Hum, I forgot that libstdc++ was considered deprecated with AppleClang, so you're right, there's probably no need to try to be smart about it.

@conan-center-bot

This comment has been minimized.

elseif(gfx-timsort_VERSION VERSION_LESS "3.0.0")
target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_11)
else()
target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_20)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since cxx_std_20 requires 3.12, would you update cmake_minimum_required?

https://cmake.org/cmake/help/latest/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about it again maybe these lines aren't be useful: shouldn't the conan-generated targets assign the correct compile features for the standard version?

I introduced them on your suggestion to fix the previous problem, but maybe constraining the compiler versions was enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Conan 2.x is all solved, the CMakeDeps, etc. will require 3.15. Plus, cppstd is defined by default there, so passing target_compile_features should no longer needed. I guess it's okay keeping the current, otherwise it would fail to build with Conan 2.x in case running an older version of cmake

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "the current", do you mean as it was before this MR, or the target_compile_features currently in the MR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get errors when removing target_compile_features apparently, guess I incorrectly interpreted your message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump because it's been half a year and I'm still unsure what you meant.

@conan-center-bot

This comment has been minimized.

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry the huge delay! I would suggest only adding c++20 in test package. For Conan 2.x it will be managed by cppstd, so it should be safe.

Please, update the test_package/CMakeLists.txt to require version 3.15 (minimum require by Conan 2.x) and will cover cxx_std_20

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ❌

Failure in build 1 (f68d79894f1ec0063f3c3d94881e702454164547):

  • timsort/2.1.0:
    Didn't run or was cancelled before finishing

  • timsort/2.0.2:
    Didn't run or was cancelled before finishing

  • timsort/2.0.1:
    Didn't run or was cancelled before finishing

  • timsort/2.0.0:
    Didn't run or was cancelled before finishing

  • timsort/1.3.0:
    Didn't run or was cancelled before finishing

  • timsort/1.2.1:
    Didn't run or was cancelled before finishing

  • timsort/1.2.0:
    CI failed to create some packages (All logs)

    Logs for packageID 5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9:
    [settings]
    arch=x86_64
    build_type=Release
    compiler=gcc
    compiler.libcxx=libstdc++11
    compiler.version=5
    os=Linux
    
    [...]
    -----------------
    -- The C compiler identification is GNU 5.4.0
    -- The CXX compiler identification is GNU 5.4.0
    -- Detecting C compiler ABI info
    -- Detecting C compiler ABI info - done
    -- Check for working C compiler: /usr/bin/gcc - skipped
    -- Detecting C compile features
    -- Detecting C compile features - done
    -- Detecting CXX compiler ABI info
    -- Detecting CXX compiler ABI info - done
    -- Check for working CXX compiler: /usr/bin/g++ - skipped
    -- Detecting CXX compile features
    -- Detecting CXX compile features - done
    -- Conan: called by CMake conan helper
    -- Conan: Adjusting output directories
    -- Conan: Using cmake targets configuration
    -- Conan: Adjusting default RPATHs Conan policies
    -- Conan: Adjusting language standard
    -- Conan: Compiler GCC>=5, checking major version 5
    -- Conan: Checking correct version: 5
    -- Conan: C++ stdlib: libstdc++11
    -- Configuring incomplete, errors occurred!
    See also "/home/conan/workspace/prod-v1/bsr/cci-b38e2403/recipes/timsort/all/test_v1_package/build/aa812f66cc25d52e62cde1f093beffcc1fbfe3f8/CMakeFiles/CMakeOutput.log".
    CMake Error at /home/conan/workspace/prod-v1/bsr/cci-b38e2403/recipes/timsort/all/test_package/CMakeLists.txt:8 (target_compile_features):
      target_compile_features The compiler feature "cxx_std_20" is not known to
      CXX compiler
    
      "GNU"
    
      version 5.4.0.
    
    
    WARN: *** Conan 1 is legacy and on a deprecation path ***
    WARN: *** Please upgrade to Conan 2 ***
    timsort/1.2.0 (test package): WARN: 
         ************************************************
         The 'cmake_find_package_multi' generator is deprecated.
         Please update your code and remove it.
         *************************************************
    
    timsort/1.2.0 (test package): WARN: 
         ************************************************
         The 'cmake' generator is deprecated.
         Please update your code and remove it.
         *************************************************
    
    timsort/1.2.0 (test package): WARN: **** The 'from conans import CMake' helper is deprecated. Please update your code and remove it. ****
    ERROR: timsort/1.2.0 (test package): Error in build() method, line 11
    	cmake.configure()
    	ConanException: Error 1 while executing cd '/home/conan/workspace/prod-v1/bsr/cci-b38e2403/recipes/timsort/all/test_v1_package/build/aa812f66cc25d52e62cde1f093beffcc1fbfe3f8' && cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE="Release" -DCONAN_IN_LOCAL_CACHE="OFF" -DCONAN_COMPILER="gcc" -DCONAN_COMPILER_VERSION="5" -DCONAN_CXX_FLAGS="-m64" -DCONAN_SHARED_LINKER_FLAGS="-m64" -DCONAN_C_FLAGS="-m64" -DCONAN_LIBCXX="libstdc++11" -DCMAKE_INSTALL_PREFIX="/home/conan/workspace/prod-v1/bsr/cci-b38e2403/recipes/timsort/all/test_v1_package/build/aa812f66cc25d52e62cde1f093beffcc1fbfe3f8/package" -DCMAKE_INSTALL_BINDIR="bin" -DCMAKE_INSTALL_SBINDIR="bin" -DCMAKE_INSTALL_LIBEXECDIR="bin" -DCMAKE_INSTALL_LIBDIR="lib" -DCMAKE_INSTALL_INCLUDEDIR="include" -DCMAKE_INSTALL_OLDINCLUDEDIR="include" -DCMAKE_INSTALL_DATAROOTDIR="share" -DCMAKE_PREFIX_PATH="/home/conan/workspace/prod-v1/bsr/cci-b38e2403/recipes/timsort/all/test_v1_package/build/aa812f66cc25d52e62cde1f093beffcc1fbfe3f8" -DCMAKE_MODULE_PATH="/home/conan/workspace/prod-v1/bsr/cci-b38e2403/recipes/timsort/all/test_v1_package/build/aa812f66cc25d52e62cde1f093beffcc1fbfe3f8" -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY="ON" -DCONAN_EXPORTED="1" -Wno-dev '/home/conan/workspace/prod-v1/bsr/cci-b38e2403/recipes/timsort/all/test_v1_package'
    
  • timsort/3.0.0:
    Didn't run or was cancelled before finishing

  • timsort/1.2.2:
    Didn't run or was cancelled before finishing


Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 1 (f68d79894f1ec0063f3c3d94881e702454164547):

  • timsort/2.1.0:
    All packages built successfully! (All logs)

  • timsort/2.0.1:
    All packages built successfully! (All logs)

  • timsort/3.0.0:
    All packages built successfully! (All logs)

  • timsort/2.0.2:
    All packages built successfully! (All logs)

  • timsort/1.3.0:
    All packages built successfully! (All logs)

  • timsort/1.2.1:
    All packages built successfully! (All logs)

  • timsort/1.2.0:
    All packages built successfully! (All logs)

  • timsort/2.0.0:
    All packages built successfully! (All logs)

  • timsort/1.2.2:
    All packages built successfully! (All logs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants