-
Notifications
You must be signed in to change notification settings - Fork 223
Try to restore some build system conventions in a low-impact way #270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Seems reasonable. I wonder if we should just remove the |
|
I don't the github actions running here... any idea whats up with that? |
|
Looks like github actions might be down/struggling: https://www.githubstatus.com/ |
|
|
||
| # Set the target. | ||
| CFLAGS = $(WASM_CFLAGS) --target=$(TARGET_TRIPLE) | ||
| CFLAGS += --target=$(TARGET_TRIPLE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If someone overwrites CFLAGS on the make command line, does it override the --target=$(TARGET_TRIPLE) part too, or just the -O2 -DNDEBUG part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just the O2 -DNDEBUG part, but will check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunfishcode I tested it, and it indeed the CLI was overriding everything. I added a bunch of override so that this is no longer the case, and just the -O2 -DNDEBUG part gets overridden.
7657a2d to
000c9ec
Compare
000c9ec to
59008cc
Compare
|
The CI failure has errors like "cc: error: unrecognized command line option ‘--target=wasm32-wasi’"; it looks like it's using the host |
59008cc to
1f11d55
Compare
|
Oops forgot about CI and the README too. I did a grep and there should be no more pesky |
|
The CI is now getting "fatal error: 'wasi/api.h' file not found" errors, and it's not immediately obvious what the problem is. |
|
@sunfishcode I think I am using |
96808c1 to
c28fd53
Compare
c28fd53 to
965088e
Compare
This is less of a sledgehammer, and prepares the way for WebAssembly#270.
This is less of a sledgehammer, and prepares the way for #270.
965088e to
040b95e
Compare
Progress towards WebAssembly#270
Progress towards WebAssembly#270
Progress towards WebAssembly#270
Progress towards WebAssembly#270
Progress towards WebAssembly#270
Fixes WebAssembly#269 The "don't move the variables independently if you want to `make install`" part is a gross, but as neither `wasi-sdk` or Nixpkgs actually needs `make install` (we both can just build right into the directories we want) I figured it wasn't actually so heinous in practice. Also reorganize the the github actions CI slightly.
040b95e to
68cfb34
Compare
|
Closing since I broke up into bits. |
Fixes #269
The "don't move the variables independently if you want to
make install" part is a gross, but as neitherwasi-sdkor Nixpkgs actuallyneeds
make install(we both can just build right into the directorieswe want) I figured it wasn't actually so heinous in practice.