Skip to content
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

Replace xbuild with cargo build-std #99

Merged
merged 3 commits into from
Nov 5, 2020

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Nov 4, 2020

The build-std feature of cargo now allows to specify the panic-immediate-abort feature. See: https://github.com/rust-osdev/cargo-xbuild#alternative-the-build-std-feature-of-cargo.

So we can use that (unstable) feature instead of depending on xbuild

Produces the same size wasm output as with xbuild e.g. flipper:

image

@athei
Copy link
Contributor

athei commented Nov 4, 2020

Regarding the TODO: I think passing hardcoded flags to cargo as it is done in this PR is fine. I wouldn't generate/mess with a .cargo/config that a user might want to edit manually.

What is not fine is hardcoding RUSTFLAGS. As far as I see it we just set the env variable to some hardcoded value. But that should be fixed in the mentioned issue. Not here.

@ascjones
Copy link
Collaborator Author

ascjones commented Nov 4, 2020

I wouldn't generate/mess with a .cargo/config that a user might want to edit manually.

The idea would be that it would set the default hardcoded rustflags iff they are not specified by the user (like we do with the [profile.release] section. And also would ensure (since the env var overrides the value from .cargo/config) that a user supplied RUSTFLAGS is not ignored.

But that should be fixed in the mentioned issue. Not here.

Anyway I agree that this should be fixed separately with a little more thought.

@ascjones ascjones marked this pull request as ready for review November 4, 2020 15:17
@ascjones
Copy link
Collaborator Author

ascjones commented Nov 4, 2020

CI currently broken, just waiting for a new image to rebuild before merging

@ascjones ascjones merged commit 4c25e3a into master Nov 5, 2020
@ascjones ascjones deleted the aj-replace-xbuild-with-build-std branch November 5, 2020 09:38
@ascjones ascjones mentioned this pull request Nov 27, 2020
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