ziglang / zig-bootstrap

take off every zig
368 stars 86 forks source link

ASM/CC/CXX: pass over environment variables #132

Closed motiejus closed 1 year ago

motiejus commented 1 year ago

ASM is now required by llvm. Commit 50aa139d14 moved CC, CXX to cmake's variables and introduced CMAKE_ASM_COMPILER. However, as noted in the build script, it requires cmake 3.191, which is not yet ubiquitous.

Roll back to env vars until, say, 2025, when ubuntu 20.04 gets EOLd? The cutoff date is up for negotiation; I just picked something that my inner circle still uses quite a bit.

david-vanderson commented 1 year ago

Can you bump the required cmake version in the readme to 3.19?

andrewrk commented 1 year ago

With this PR the cmake version required remains at 3.13.4.

andrewrk commented 1 year ago

I'm confused - in https://github.com/ziglang/zig-bootstrap/commit/50aa139d14db69b62b4e7ce1ae03befe30876b69, I wrote "cmake does not respect spaces in the ASM environment variable". But it apparently works on your computer? I suppose I can test it again just to be sure.

david-vanderson commented 1 year ago

Sorry I'm confused too. I thought @motiejus was saying that cmake 3.19 fixed the "spaces in ASM env var" problem?

motiejus commented 1 year ago

Sorry, I tested it on the wrong cmake. I will rerun my tests on 3.18 and report back.

motiejus commented 1 year ago

Indeed this fails on cmake 3.18.

I have a diff that creates wrappers zig-cc and zig-c++, and now am testing those. Will report back.

andrewrk commented 1 year ago

Let's talk about bumping the cmake requirement to 3.19. It's not ideal but is it a show stopper? I think it would be less complicated than relying on more shell script logic to create wrappers.

motiejus commented 1 year ago

Let's talk about bumping the cmake requirement to 3.19. It's not ideal but is it a show stopper?

It's a nuisance.

I think it would be less complicated than relying on more shell script logic to create wrappers.

I somehow thought that env vars for ASM will work, they don't. I agree the hacks aren't worth it, at least for me.

david-vanderson commented 1 year ago

@motiejus Wait wait - we need to do something here. Do the env vars for ASM work with cmake 3.19?

andrewrk commented 1 year ago

@david-vanderson the end result of this conversation was 336e925113d3c20cbd8f35a6004ada88048e63b8 - same as your pull request except with the correct version number, which is 3.19.

david-vanderson commented 1 year ago

Ah thank you! I've also confirmed that cmake 3.19 works with the env vars.