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

Revert "Go back to CMake 3.25.2 (#4496)" #4503

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Feb 8, 2023

This reverts PR #4496 (b8f2855).

CMake bug report (suspected regression): https://gitlab.kitware.com/cmake/cmake/-/issues/24398

Description

Suggested changelog entry:

@bradking
Copy link

bradking commented Feb 9, 2023

Add a preceding commit with this patch to get more verbose information about the connection failure:

diff --git a/tools/FindCatch.cmake b/tools/FindCatch.cmake
index 57bba58b..ef120bc8 100644
--- a/tools/FindCatch.cmake
+++ b/tools/FindCatch.cmake
@@ -36,10 +36,16 @@ endfunction()
 function(_download_catch version destination_dir)
   message(STATUS "Downloading catch v${version}...")
   set(url https://github.com/philsquared/Catch/releases/download/v${version}/catch.hpp)
-  file(DOWNLOAD ${url} "${destination_dir}/catch.hpp" STATUS status)
+  file(DOWNLOAD ${url} "${destination_dir}/catch.hpp" STATUS status LOG log)
   list(GET status 0 error)
   if(error)
-    message(FATAL_ERROR "Could not download ${url}")
+    string(REPLACE "\n" "\n  " log "  ${log}")
+    message(FATAL_ERROR
+      "Could not download URL:\n"
+      "  ${url}\n"
+      "Log:\n"
+      "${log}"
+      )
   endif()
   set(CATCH_INCLUDE_DIR
       "${destination_dir}"

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Feb 9, 2023
@rwgk rwgk force-pushed the back_to_cmake_latest branch from 6a139e1 to 8c88d04 Compare February 9, 2023 20:36
@rwgk
Copy link
Collaborator Author

rwgk commented Feb 9, 2023

Thanks a lot @bradking, I'd say your patch is a keeper!

Here is one log:

https://github.com/pybind/pybind11/actions/runs/4138219644/jobs/7154367988

IIUC it reads a bunch of ~1k blocks successfully but then runs into errors:

   ...
    [1330 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1339 bytes data]
    [1057 bytes data]
    [1362 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [731 bytes data]
    [1362 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    [1362 bytes data]
    [1371 bytes data]
    [1371 bytes data]
    SSLRead() return error -50
    Failed receiving HTTP2 data
    SSLWrite() returned error -9806
    Failed sending HTTP2 data
    Connection #1 to host objects.githubusercontent.com left intact

@rwgk
Copy link
Collaborator Author

rwgk commented Feb 10, 2023

The curl (in CMake) issue was fixed: https://gitlab.kitware.com/cmake/cmake/-/issues/24398

The fix will be in 3.26.0-rc3

Link to releases (for checking if the rc3 release happened already): https://github.com/Kitware/CMake/releases

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Feb 15, 2023
@rwgk rwgk force-pushed the back_to_cmake_latest branch from 8c88d04 to 10dd074 Compare February 15, 2023 18:18
@rwgk rwgk marked this pull request as ready for review February 15, 2023 20:45
@rwgk rwgk requested a review from henryiii as a code owner February 15, 2023 20:45
@rwgk
Copy link
Collaborator Author

rwgk commented Feb 15, 2023

@henryiii This is working now with CMake 3.26.0-rc3. I think it'll be best to also merge @bradking's much more informative reporting of the underlying download error.

@henryiii henryiii force-pushed the back_to_cmake_latest branch from 10dd074 to 08bf54c Compare February 16, 2023 14:55
@henryiii henryiii merged commit 08a4a47 into pybind:master Feb 16, 2023
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Feb 16, 2023
@rwgk
Copy link
Collaborator Author

rwgk commented Feb 16, 2023

Thanks @henryiii!

@rwgk rwgk deleted the back_to_cmake_latest branch February 16, 2023 15:57
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Feb 16, 2023
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