search

ziglang / zig-bootstrap

take off every zig
368 stars 86 forks source link

Update cmake required version #126

Closed david-vanderson closed 1 year ago

david-vanderson commented 1 year ago

I ran into lots of bugs with cmake 3.18.4 (3.18.4-2+deb11u1 to be exact). The change to: -DCMAKE_C_COMPILER="$ZIG;cc;-fno-sanitize=all;-s;-target;$TARGET;-mcpu=$MCPU"

seems to have caused the bugs I saw, which were all related to cmake not recognizing "zig cc" properly.

Everything worked when I upgraded to cmake 3.24.2, which is just the latest version I got from cmake.org. Probably earlier versions work as well.

motiejus commented 1 year ago

Yes please. I spent more time than I would like to admit on this one.

motiejus commented 1 year ago

If we want compatibility with older cmake versions, we can just make wrappers of cc and c++, which just invoke zig cc/c++ with the right args.

motiejus commented 1 year ago

I tested with cmake 3.22.3, so the required version can be at most this number. Probably it can be lower than that.

It does not work with 3.18, and I see this in 3.19 changelog:

The CMAKE__COMPILER variable may now be used to store "mandatory" compiler flags like the CC and other environment variables.

As an aside, requiring a newer cmake is unfortunate; this project really should try to be compatible with 3.13.4. I'm not sure how to solve the problem, however, because the environment variable equivalent for CMAKE_ASM_COMPILER did not respect spaces.

Just passing CC, CXX and ASM via env vars still works: https://github.com/ziglang/zig-bootstrap/pull/132

david-vanderson commented 1 year ago

Sounds like going with #132 is the best option combined with bumping the required cmake version to 3.19.

@andrewrk if that is okay with you please close this pull. Thanks!