Skip to content

Conversation

@davezarzycki
Copy link
Contributor

Also remove some ancient logic to detect and ignore requests to use LLD. If people want to explicitly use LLD, they probably have a reason and we shouldn't second guess them.

@davezarzycki
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Making the Swift build system more similar to the LLVM options is definitely a good idea.

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 that we should simplify this like the previous case in cmake/modules/AddSwift.cmake

Copy link
Contributor

@drodriguez drodriguez left a comment

Choose a reason for hiding this comment

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

The parts that I know (the Android parts, mostly), looks fine to me. Should be functionally equivalent to the existing code. There's only that one point that should be double checked. Otherwise looks good.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the manipulations around these lines (L418-L422) are the same to the ones in AddSwiftUnittests.cmake, but the modifications are different: some checks are kept here, while they are drop in the other file. Is the divergence intentional?

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fce4f52df219a516c761b4dbe108370352ad46ba

Also remove some ancient logic to detect and ignore requests to use LLD.
If people want to explicitly use LLD, they probably have a reason and we
shouldn't second guess them.
@davezarzycki
Copy link
Contributor Author

davezarzycki commented May 28, 2020

Hi @compnerd and @drodriguez – I've simplified AddSwiftStdlib.cmake more. I think we should make SWIFT_USE_LINKER be as much like LLVM_USE_LINKER as possible. In other words, if it's set, trust it.

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fce4f52df219a516c761b4dbe108370352ad46ba

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - fce4f52df219a516c761b4dbe108370352ad46ba

@davezarzycki
Copy link
Contributor Author

@swift-ci please clean test linux

@compnerd
Copy link
Member

Yeah, I agree on the idea that if the user specified it, we assume correct and honor it.

@davezarzycki
Copy link
Contributor Author

@swift-ci please test windows

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