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

Cache: failure when the download is aborted and the download dir is empty #548

Open
gerhardol opened this issue Mar 13, 2024 · 3 comments
Open

Comments

@gerhardol
Copy link
Contributor

This is related to cached packages. No cache or direct CMake FetchContent is probably not affected, as download is tried every time.

The cache download directory is created before the download is started.
If a download is aborted for any reason (ctrl-c, server issue etc) or the hash check fails, the job may fail (not really investigated if all occurrences are handled).
The real problem comes the next time, the download directory already exists and a new download is not attempted. You will just get a cryptic message when the directory is empty.

Some proposals. All of these assumes that a failure in the download, unpack, hash check, patching will fail a job (this is not checked really in CMake I assume).

  • A complete would be to add a crc on the unpacked and patched download. This could be configured or automatic. Not an option to me, would be too slow.
  • Some marker file in the download to check when dir exists (and when downloaded). A simple patch below. (Will also handle "undetected" failures.
  • Some marker file created after download and install is done.

A very simple patch. Should assume CMakeLists.txt if the variable is defined but empty.

diff --git a/CMakeConfig/cpm/CPM.cmake b/CMakeConfig/cpm/CPM.cmake
index ed75dd9..353717a 100644
--- a/CMakeConfig/cpm/CPM.cmake
+++ b/CMakeConfig/cpm/CPM.cmake
@@ -769,6 +769,7 @@ function(CPMAddPackage)
         BINARY_DIR "${${CPM_ARGS_NAME}_BINARY_DIR}"
       )
 
+      CPMCheckDownloadContents(${CPM_ARGS_NAME})
     else()
       # Enable shallow clone when GIT_TAG is not a commit hash. Our guess may not be accurate, but
       # it should guarantee no commit hash get mis-detected.
@@ -809,6 +810,8 @@ function(CPMAddPackage)
     if(CPM_SOURCE_CACHE AND download_directory)
       file(LOCK ${download_directory}/../cmake.lock RELEASE)
     endif()
+    CPMCheckDownloadContents(${CPM_ARGS_NAME})
+
     if(${populated})
       cpm_add_subdirectory(
         "${CPM_ARGS_NAME}"
@@ -827,6 +830,12 @@ function(CPMAddPackage)
   cpm_export_variables("${CPM_ARGS_NAME}")
 endfunction()
 
+macro(CPMCheckDownloadContents PACKAGE)
+  if(NOT $ENV{MY_CPM_CHECK_CONTENT_FILE} STREQUAL "" AND NOT EXISTS "${${PACKAGE}_SOURCE_DIR}/$ENV{MY_CPM_CHECK_CONTENT_FILE}")
+    message(FATAL_ERROR "CPM: Package ${PACKAGE} was not downloaded properly, missing: ${${PACKAGE}_SOURCE_DIR}/$ENV{MY_CPM_CHECK_CONTENT_FILE}. Delete the directory and try again.")
+  endif()
+endmacro()
+
 # Fetch a previously declared package
 macro(CPMGetPackage Name)
   if(DEFINED "CPM_DECLARATION_${Name}")
@jdumas
Copy link

jdumas commented May 3, 2024

+1 for this issue. This happens pretty frequently in our Jenkins CI/CD setup. We try to leverage a shared cache, but sometimes jobs get aborted and leave the cache in a sorry state.

@jhurliman
Copy link

We encounter this issue frequently as well, in both CI and local developer machines.

@gerhardol
Copy link
Contributor Author

#559 added (I saw the comments above after submitting). That should be safer than the quick patch above and has an optional checksum.

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

No branches or pull requests

3 participants