Skip to content

build: bump minimum CMake requirement#606

Merged
rauhul merged 1 commit into
apple:mainfrom
compnerd:version
Dec 7, 2023
Merged

build: bump minimum CMake requirement#606
rauhul merged 1 commit into
apple:mainfrom
compnerd:version

Conversation

@compnerd
Copy link
Copy Markdown
Collaborator

@compnerd compnerd commented Dec 7, 2023

Update the CMake requirement to 3.19 as Swift requires 3.19.x to build. The upgrade allows us to remove workarounds that we were carrying for older CMake compatibility.

Update the CMake requirement to 3.19 as Swift requires 3.19.x to build.
The upgrade allows us to remove workarounds that we were carrying for
older CMake compatibility.
@compnerd
Copy link
Copy Markdown
Collaborator Author

compnerd commented Dec 7, 2023

@swift-ci please test

@compnerd
Copy link
Copy Markdown
Collaborator Author

compnerd commented Dec 7, 2023

CC: @natecook1000 @rauhul

@rauhul rauhul merged commit 11c580b into apple:main Dec 7, 2023
@compnerd compnerd deleted the version branch December 7, 2023 22:00
@rauhul
Copy link
Copy Markdown
Collaborator

rauhul commented Dec 7, 2023

FWIW I have no issues with you directly merging cmake changes

Comment thread CMakeLists.txt
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
endif()
list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules)
Copy link
Copy Markdown

@bnbarham bnbarham Dec 8, 2023

Choose a reason for hiding this comment

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

Isn't this too late now @compnerd? SwiftSupport is within cmake/modules and is included before this now. CC @xedin

I'm confused as to how this passed CI though 🤔

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, that is a good point 🤔. How did CI get away with this?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah, looking at logs CI is using SPM, not CMake

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh lovely :(

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