Skip to content

Set default linker to lld for Amazon Linux 2023 #72049

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.

Sign up for GitHub

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
Apr 10, 2024
Merged

Set default linker to lld for Amazon Linux 2023 #72049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,10 @@ if(XCODE)
swift_common_xcode_cxx_config()
endif()

# Check what linux distribution is being used.
# This can be used to determine the default linker to use.
cmake_host_system_information(RESULT DISTRO_NAME QUERY DISTRIB_PRETTY_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be reverted.

The minimum CMake version is 3.19.6 (says so at the top of the file).

This key only appeared on CMake 3.22.

Copy link
Contributor Author

@futurejones futurejones Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@drodriguez @finagolfin @etcwilde
The minimum cmake version required for the swift toolchain build has been "cmake": "v3.24.2" since version 5.10

https://github.com/apple/swift/blob/main/utils/update_checkout/update-checkout-config.json#L232

#67018

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the version used by the build-script for CI, and only on Linux and BSD. The CMake files should be compatible with the version required by the cmake_minimum_required instruction. This is failing for a macOS which a 3.20 version of CMake (enough to build LLVM and Swift using only CMake).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the best option here is to update the minimum cmake version to match the version used in the CI.
This was supposed to have been done already and it makes no sense to be using a cmake version as old as v3.19.6

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 @etcwilde plans to significantly increase the minimum CMake version this year anyway, so it would make sense to increase it to 3.22 now itself.

Evan, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

But things need to be done in certain order, and not as a reaction to introducing a dependency to check for a specific Linux distribution, and have a broken CMakeLists.txt at the root of the project.

Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming #73001 fixes the problem for now, we can discuss the CMake version separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

One CMake version bump that should be very easy to justify is to 3.20.0, since that's what LLVM needs, and anybody trying to build would need at least that.

Going beyond that is probably a question of distribution support. macOS is an outlier: if one uses Homebrew you are fine, but if some other packing system is used, people will need to upgrade. I think the problem in Linux and BSD distros is worked around with the compiled CMake version (at least when using build-script).

IMO, for any platform, in any case, it would be good if the actual needed CMake version was detailed in the main CMakeLists.txt file.


# Which default linker to use. Prefer LLVM_USE_LINKER if it set, otherwise use
# our own defaults. This should only be possible in a unified (not stand alone)
# build environment.
Expand All @@ -959,6 +963,8 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows" AND NOT CMAKE_HOST_SYSTEM_NAME STREQ
set(SWIFT_USE_LINKER_default "lld")
elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(SWIFT_USE_LINKER_default "")
elseif(DISTRO_NAME STREQUAL "Amazon Linux 2023")
set(SWIFT_USE_LINKER_default "lld")
else()
set(SWIFT_USE_LINKER_default "gold")
endif()
Expand Down
19 changes: 18 additions & 1 deletion lib/Driver/UnixToolChains.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//

#include <fstream>

#include "ToolChains.h"

#include "swift/Basic/LLVM.h"
Expand Down Expand Up @@ -108,9 +110,24 @@ ToolChain::InvocationInfo toolchains::GenericUnix::constructInvocation(

return II;
}
// Amazon Linux 2023 requires lld as the default linker.
bool isAmazonLinux2023Host() {
std::ifstream file("/etc/os-release");
std::string line;

while (std::getline(file, line)) {
if (line.substr(0, 12) == "PRETTY_NAME=") {
if (line.substr(12) == "\"Amazon Linux 2023\"") {
file.close();
return true;
}
}
}
return false;
}

std::string toolchains::GenericUnix::getDefaultLinker() const {
if (getTriple().isAndroid())
if (getTriple().isAndroid() || isAmazonLinux2023Host())
return "lld";

switch (getTriple().getArch()) {
Expand Down