Skip to content

[Testing] Do not assume unqualified path to clang++ #25972

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 1 commit into from
Jul 4, 2019

Conversation

davezarzycki
Copy link
Contributor

Without this change, unified builds are broken on my Linux workstation (Fedora 30, x86-64).

Without this change, unified builds are broken on my Linux workstation
(Fedora 30, x86-64).
@davezarzycki davezarzycki requested a review from compnerd July 4, 2019 06:44
@davezarzycki
Copy link
Contributor Author

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 06fd9c4 into swiftlang:master Jul 4, 2019
@davezarzycki davezarzycki deleted the unbreak_unified_builds branch July 4, 2019 08:17
@compnerd
Copy link
Member

compnerd commented Jul 4, 2019

@davezarzycki, I believe that the path should be unqualified, as that is what the test is testing. The point here is to ensure that we explicitly are passing clang++ and relying on PATH to find the tool.

@davezarzycki
Copy link
Contributor Author

I don't think my setup is doing anything weird here, and the fully qualified path made its way through to the test. How can I help you debug this?

@jrose-apple
Copy link
Contributor

I think someone else changed the search to find files in the same directory as the compiler, to make it easier to build full custom toolchain setups. That means this test will never work for a unified build. Maybe it can hard-link the compiler to another location, like some of the other tests in the Driver/ folder?

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.

4 participants