-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Simplify C compilation for Fortanix-SGX target #83204
Conversation
r? @kennytm (rust-highfive has picked a reviewer for you, use r? to override) |
.gitattributes
Outdated
src/vendor/** -text | ||
Cargo.lock linguist-generated=false | ||
|
||
# Older git versions try to fix line endings on images, this prevents it. | ||
*.png binary | ||
*.ico binary | ||
*.woff binary | ||
*.woff2 binary |
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.
This seems like an unrelated change bundled into this PR.
CC_x86_64_fortanix_unknown_sgx=clang-11 \ | ||
CFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ | ||
CXX_x86_64_fortanix_unknown_sgx=clang++-11 \ | ||
CXXFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ |
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.
Interesting. In general, I'd be concerned about assuming that system headers will work for a different target (it's always a challenge to build a non-host target using a host compiler while cancelling out any assumptions that hold for one and not the other), but I don't think that's an issue for a highly controlled Docker environment like this.
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.
Not just “system headers” but specifically the x86_64-linux-gnu
headers. I think it's better this way than the previous solution.
Looks reasonable to me. r=me if you split out the unrelated change into a separate PR. |
a5efbe9
to
5bd50ef
Compare
@joshtriplett Thanks, can you delegate to @raoulstrackx I'd like him to take a look as well. |
bors r=me |
@bors r=joshtriplett,raoulstrackx |
📌 Commit 5bd50ef has been approved by |
…lett,raoulstrackx Simplify C compilation for Fortanix-SGX target cc `@raoulstrackx`
Rollup of 11 pull requests Successful merges: - rust-lang#82191 (Vec::dedup_by optimization) - rust-lang#82270 (Emit error when trying to use assembler syntax directives in `asm!`) - rust-lang#82434 (Add more links between hash and btree collections) - rust-lang#83080 (Make source-based code coverage compatible with MIR inlining) - rust-lang#83168 (Extend `proc_macro_back_compat` lint to `procedural-masquerade`) - rust-lang#83192 (ci/docker: Add SDK/NDK level 21 to android docker for 32bit platforms) - rust-lang#83204 (Simplify C compilation for Fortanix-SGX target) - rust-lang#83216 (Allow registering tool lints with `register_tool`) - rust-lang#83223 (Display error details when a `mmap` call fails) - rust-lang#83228 (Don't show HTML diff if tidy isn't installed for rustdoc tests) - rust-lang#83231 (Switch riscvgc-unknown-none-elf use lp64d ABI) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
cc @raoulstrackx