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

Set default compiler to clang on apple hosts #407

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spl
Copy link
Contributor

@spl spl commented May 30, 2019

The change in this PR (615c53b) seems to be the right choice to make. clang/clang++ should be considered the default on macOS, and it allows the ToolFamily to be chosen correctly based on the name.

Many tests fail, however, and I believe that's because (a) they are duplicated for Linux and macOS targets and (b) each uses the same target and host. For example, consider this test from tests/test.rs:

#[test]
fn gnu_x86_64() {
    for vendor in &["unknown-linux-gnu", "apple-darwin"] {
        let target = format!("x86_64-{}", vendor);
        let test = Test::gnu();
        test.gcc()
            .target(&target)
            .host(&target)
            .file("foo.c")
            .compile("foo");
        test.cmd(0).must_have("-fPIC").must_have("-m64");
    }   
}

So, first, is my premise about choosing the default compiler on macOS is correct? If so, how should the tests change?

@spl
Copy link
Contributor Author

spl commented May 30, 2019

This is how one test for the target and host x86_64-unknown-linux-gnu fails on Azure Pipelines:

test gnu_i686 ... cargo:warning=clang: error: no such file or directory: 'foo.c'
cargo:warning=clang: error: no input files


error occurred: Command "clang" "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=i686-apple-darwin" "-Wall" "-Wextra" "-o" "/tmp/gcc-test.TJJ1vYB38xne/foo.o" "-c" "foo.c" with args "clang" did not execute successfully (status code exit code: 1).

@alexcrichton
Copy link
Member

Seems reasonable to me! I'm not sure why CI didn't fire here, but this plus with fixed tests sounds reasonableto me.

@spl
Copy link
Contributor Author

spl commented Jun 1, 2019

What's the best way to fix the tests?

It's not clear to me why the host and target are both set to the same or even why the host is set at all? For one thing, I think you'd want any current or future logic in cc-rs to be able to assume the host has the expected environment, whereas using x86_64-apple-darwin as the host on a Linux machine does not fit that expectation.

Would it be acceptable to let the tests assume the actual host instead of the target as the host?

@spl
Copy link
Contributor Author

spl commented Jun 3, 2019

Okay, I have better understanding of the test infrastructure. Ignore my musings above. I'm currently performing some renovations to the tests.

@alexcrichton
Copy link
Member

Ok thanks for that!

I wonder if in the meantime that #408 is a better solution for this? I'm somewhat wary of changing defaults as that can have weird unintended rippling effects, but if we can detect that /usr/bin/cc is a symlink to a binary called clang that seems like a pretty clear indicator that it's clang-by-default rather than gcc

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

This seems reasonable, if someone wants to use GCC on these targets, they can set CC or set the compiler via the API, but I think we shouldn't ever default to it on Apple targets.

That said, this needs a rebase, and I'll probably go over it again after that's done.

@thomcc thomcc added the O-apple Apple targets and toolchains label Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-apple Apple targets and toolchains
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants