Skip to content

Add CI checks with LLVM sysroot#858

Open
GuillaumeGomez wants to merge 2 commits intorust-lang:masterfrom
GuillaumeGomez:llvm-sysroot
Open

Add CI checks with LLVM sysroot#858
GuillaumeGomez wants to merge 2 commits intorust-lang:masterfrom
GuillaumeGomez:llvm-sysroot

Conversation

@GuillaumeGomez
Copy link
Member

Fixes #850.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we still need the -Zcodegen-backend flag even when using the LLVM sysroot.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we'll see soon enough. :3

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, the test will just use cg_llvm without this, so this is not what we want.

Comment on lines 316 to +317
use_system_gcc: bool,
with_llvm_sysroot: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use 1 or 2 enums here to make the caller code clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case, the added value is pretty minimal imo.

run_command_with_output_and_env(&command, None, Some(&env))?;

args.config_info.setup(&mut env, false)?;
args.config_info.setup(&mut env, false, false)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment to mention that we always build the sysroot with cg_gcc and we don't need to build the sysroot with cg_llvm since it's already distributed by rustup and we can just not provide --sysroot to use it.

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.

Add CI where we test with a sysroot compiled by LLVM

2 participants