Skip to content

Fix CMake CMP0135 warnings for ExternalProject downloads#5083

Open
mvanhorn wants to merge 3 commits intoponylang:mainfrom
mvanhorn:fix-cmake-warnings
Open

Fix CMake CMP0135 warnings for ExternalProject downloads#5083
mvanhorn wants to merge 3 commits intoponylang:mainfrom
mvanhorn:fix-cmake-warnings

Conversation

@mvanhorn
Copy link
Copy Markdown

Sets cmake_policy(SET CMP0135 NEW) in lib/CMakeLists.txt so that
ExternalProject_Add URL downloads use extraction-time timestamps instead
of archive timestamps. This silences both dev warnings from the gbenchmark
and googletest downloads reported in #4852.

The NEW behavior is the recommended default since CMake 3.24 and ensures
that code depending on extracted contents is correctly rebuilt when the URL
changes.

Fixes #4852

Set cmake_policy(SET CMP0135 NEW) so that ExternalProject_Add URL
downloads use extraction-time timestamps instead of archive timestamps.
This silences the dev warnings from gbenchmark and googletest downloads.

Fixes ponylang#4852

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Mar 27, 2026
@SeanTAllen
Copy link
Copy Markdown
Member

Thanks for the PR!

The cmake_policy(SET CMP0135 NEW) approach has a compatibility problem: CMP0135 was introduced in CMake 3.24, but our minimum is 3.21. Calling cmake_policy(SET CMP0135 NEW) on CMake versions that don't know about that policy would be an error, so we'd have to bump the minimum to 3.24 to make this work.

Since the fix only applies to the two ExternalProject_Add calls in lib/CMakeLists.txt (gbenchmark on line 34 and googletest on line 41), I'd rather just add DOWNLOAD_EXTRACT_TIMESTAMP TRUE to each of them. That silences the warning without forcing a minimum version bump.

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Mar 27, 2026
Replace cmake_policy(SET CMP0135 NEW) with per-call
DOWNLOAD_EXTRACT_TIMESTAMP TRUE on both ExternalProject_Add
invocations. Avoids requiring CMake 3.24+ since the minimum
is 3.21.
@mvanhorn
Copy link
Copy Markdown
Author

Good catch on the minimum version. Switched to DOWNLOAD_EXTRACT_TIMESTAMP TRUE on both ExternalProject_Add calls in 2ccbf0e.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Mar 27, 2026
@SeanTAllen
Copy link
Copy Markdown
Member

And I just noticed that DOWNLOAD_EXTRACT_TIMESTAMP is also Cmake 3.24. Ugh.

Some more searching suggests that something like

  if(POLICY CMP0135)
      cmake_policy(SET CMP0135 NEW)
  endif()

at the top of the file is "idiomatic" for handling this. but i haven't validated that.

also i might have gotten my reading of DOWNLOAD_EXTRACT_TIMESTAMP wrong and now I think maybe it would have been FALSE not TRUE. that we would have wanted. Not that it was the right thing but looks like I double-wronged it.

Replace per-target DOWNLOAD_EXTRACT_TIMESTAMP with a top-level
policy guard that works on CMake 3.21+ (the project minimum).
@mvanhorn
Copy link
Copy Markdown
Author

Switched to the if(POLICY CMP0135) guard in 7bf880d. Removed the per-target DOWNLOAD_EXTRACT_TIMESTAMP since the policy applies globally to both ExternalProject_Add calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discuss during sync Should be discussed during an upcoming sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CMake warnings when building

3 participants