Fixed cmake_minimum_required directive

mhx / dwarfs

A fast high compression read-only file system for Linux, Windows and macOS

GNU General Public License v3.0

2.16k stars 58 forks source link

Fixed cmake_minimum_required directive #175

Closed maxirmx closed 1 year ago

maxirmx commented 1 year ago

Hi, @mhx

Thank you the great tool. A small improvement proposal:

cmake_minimum_required shall preceed project Otherwise cmake generates very long and annoying warning
add_subdirectory(folly EXCLUDE_FROM_ALL SYSTEM) actually requires cmake 3.25, not 3.24

Thank you

cc: @ronaldtse This is a contribution from Tebako (GitHub), a product of Ribose (GitHub).

mhx commented 1 year ago

Thanks!

mhx commented 1 year ago

This somehow managed to break the Windows build. I suspect the change from 3.24 to 3.25 rather than moving the cmake_minimum_required statement. I'll dig into that tomorrow.

maxirmx commented 1 year ago

You can leave it to me if you wish

I am going to integrate Windows build in our pipeline

mhx commented 1 year ago

I suspect the change from 3.24 to 3.25 rather than moving the cmake_minimum_required statement.

Au contraire. It's exactly the cmake_minimum_required move that's causing trouble. I've reverted that part of the PR in 6cfce3b for now.

You can leave it to me if you wish

That'd be great, I don't have the time right now to dig deeper into this. Somehow the change affects how static vs dynamic linking is handled on Windows.

I am going to integrate Windows build in our pipeline

Thanks!