Skip to content

Use build-std-features instead of rlibc #298

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

Merged
merged 3 commits into from
Oct 18, 2021

Conversation

YtvwlD
Copy link
Contributor

@YtvwlD YtvwlD commented Oct 9, 2021

I've built a UEFI application with this library this year and it worked great, thank you!

But there's one thing I've noticed in the test-runner and in the application template:

# When building using Cargo's `build-std` feature, the `mem` feature of `compiler-builtins`
# does not automatically get enabled. Therefore, we have to manually add support for
# the memory functions.
rlibc = "1.0.0"

So, I wondered, why not directly enable the mem feature of compiler-builtins? It seems to work both in my application and in the test-runner, so I thought, I'd submit this as a PR. If there's something I'm missing, feel free to close this PR. :)

There's one last thing about the BUILDING.md I stumbled upon while creating this PR, but it's not really important:

Cargo feature: `cargo +nightly build -Z build-std --target x86_64-unknown-uefi`.

Why is the command line that long? -Z build-std is already specified in .cargo/config, so it's not really needed. (The same applies to -Z build-std-features=compiler-builtins-mem.)

@YtvwlD
Copy link
Contributor Author

YtvwlD commented Oct 12, 2021

I guess this error is not my fault. It also occurs with the current nightly compiler on master.
With nightly-2021-10-07, 08 and 09, the tests work.

BUILDING.md Outdated
@@ -24,7 +24,7 @@ application. Copy it to a new directory to get started.

- Build using a `nightly` version of the compiler and activate the
[`build-std`](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#build-std)
Cargo feature: `cargo +nightly build -Z build-std --target x86_64-unknown-uefi`.
Cargo features: `cargo +nightly build -Z build-std -Z build-std-features=compiler-builtins-mem --target x86_64-unknown-uefi`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are correct that this command is unnecessarily long, could you change it to just cargo +nightly build --target x86_64-unknown-uefi?

@nicholasbishop
Copy link
Member

Thanks for the PR! I don't know about the history of why that wasn't previously in .cargo/config, but I think this looks like a nice simplification. Left one comment on BUILDING.md but otherwise looks good.

I guess this error is not my fault. It also occurs with the current nightly compiler on master.

Confirmed, that's #299. I disabled the failing test for now though so tests should pass if you rebase.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@nicholasbishop
Copy link
Member

Looks good, thanks!

@nicholasbishop nicholasbishop merged commit 55de130 into rust-osdev:master Oct 18, 2021
@YtvwlD YtvwlD deleted the rlibc branch October 19, 2021 08:23
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.

None yet

2 participants