Skip to content

Symlink swift and swiftc to swift-driver, when available #69834

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 4 commits into from
Jan 9, 2024
Merged

Symlink swift and swiftc to swift-driver, when available #69834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7c3f5a
Remove redundant calls to 'add_swift_tool_symlink' on bin executables…
artemcm Nov 14, 2023
f85b180
Symlink 'swift' and 'swiftc' to 'swift-driver', when available
artemcm Nov 14, 2023
7f6152a
Install host-built 'swift-driver' and 'swift-help' when installing th…
artemcm Nov 14, 2023
6a181fb
Add symlinks for the legacy driver invocation (for emergency fallback…
artemcm Nov 14, 2023
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
2 changes: 2 additions & 0 deletions lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ void Driver::parseDriverKind(ArrayRef<const char *> Args) {
llvm::StringSwitch<llvm::Optional<DriverKind>>(DriverName)
.Case("swift", DriverKind::Interactive)
.Case("swiftc", DriverKind::Batch)
.Case("swift-legacy-driver", DriverKind::Interactive)
.Case("swiftc-legacy-driver", DriverKind::Batch)
.Case("sil-opt", DriverKind::SILOpt)
.Case("sil-func-extractor", DriverKind::SILFuncExtractor)
.Case("sil-nm", DriverKind::SILNM)
Expand Down
40 changes: 32 additions & 8 deletions lib/DriverTool/driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ static bool shouldRunAsSubcommand(StringRef ExecName,
// If we are not run as 'swift', don't do anything special. This doesn't work
// with symlinks with alternate names, but we can't detect 'swift' vs 'swiftc'
// if we try and resolve using the actual executable path.
if (ExecName != "swift")
if (ExecName != "swift" && ExecName != "swift-legacy-driver")
return false;

// If there are no program arguments, always invoke as normal.
Expand Down Expand Up @@ -167,14 +167,30 @@ static bool shouldRunAsSubcommand(StringRef ExecName,
static bool shouldDisallowNewDriver(DiagnosticEngine &diags,
StringRef ExecName,
const ArrayRef<const char *> argv) {
// We are not invoking the driver, so don't forward.
if (ExecName != "swift" && ExecName != "swiftc") {
return true;
// We are expected to use the legacy driver to `exec` an overload
// for testing purposes.
if (llvm::sys::Process::GetEnv("SWIFT_OVERLOAD_DRIVER").has_value()) {
return false;
}
StringRef disableArg = "-disallow-use-new-driver";
StringRef disableEnv = "SWIFT_USE_OLD_DRIVER";
auto shouldWarn = !llvm::sys::Process::
GetEnv("SWIFT_AVOID_WARNING_USING_OLD_DRIVER").has_value();

// We explicitly are on the fallback to the legacy driver from the new driver.
// Do not forward.
if (ExecName == "swift-legacy-driver" ||
ExecName == "swiftc-legacy-driver") {
if (shouldWarn)
diags.diagnose(SourceLoc(), diag::old_driver_deprecated, disableArg);
return true;
}

// We are not invoking the driver, so don't forward.
if (ExecName != "swift" && ExecName != "swiftc") {
return true;
}

// If user specified using the old driver, don't forward.
if (llvm::find_if(argv, [&](const char* arg) {
return StringRef(arg) == disableArg;
Expand All @@ -193,7 +209,7 @@ static bool shouldDisallowNewDriver(DiagnosticEngine &diags,

static bool appendSwiftDriverName(SmallString<256> &buffer) {
assert(llvm::sys::fs::exists(buffer));
if (auto driverNameOp = llvm::sys::Process::GetEnv("SWIFT_USE_NEW_DRIVER")) {
if (auto driverNameOp = llvm::sys::Process::GetEnv("SWIFT_OVERLOAD_DRIVER")) {
llvm::sys::path::append(buffer, *driverNameOp);
return true;
}
Expand Down Expand Up @@ -312,8 +328,7 @@ static int run_driver(StringRef ExecName,
subCommandArgs.push_back(DriverModeArg.data());
} else if (ExecName == "swiftc") {
subCommandArgs.push_back("--driver-mode=swiftc");
} else {
assert(ExecName == "swift");
} else if (ExecName == "swift") {
subCommandArgs.push_back("--driver-mode=swift");
}
// Push these non-op frontend arguments so the build log can indicate
Expand Down Expand Up @@ -344,7 +359,16 @@ static int run_driver(StringRef ExecName,
return 2;
}
}


// We are in the fallback to legacy driver mode.
// Now that we have determined above that we are not going to
// forward the invocation to the new driver, ensure the rest of the
// downstream driver execution refers to itself by the appropriate name.
if (ExecName == "swift-legacy-driver")
ExecName = "swift";
else if (ExecName == "swiftc-legacy-driver")
ExecName = "swiftc";

Driver TheDriver(Path, ExecName, argv, Diags);
switch (TheDriver.getDriverKind()) {
case Driver::DriverKind::SILOpt:
Expand Down
2 changes: 1 addition & 1 deletion test/Driver/LegacyDriver/legacy-driver-propagates-response-file-to-new-driver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// REQUIRES: shell
// RUN: %{python} -c 'for i in range(500001): print("-DTEST5_" + str(i))' > %t.resp
// RUN: cp %S/Inputs/print-args.sh %swift-bin-dir/legacy-driver-propagates-response-file.sh
// RUN: env SWIFT_USE_NEW_DRIVER=legacy-driver-propagates-response-file.sh %swiftc_driver_plain %s @%t.resp | %FileCheck %s
// RUN: env SWIFT_OVERLOAD_DRIVER=legacy-driver-propagates-response-file.sh %swiftc_driver_plain -disallow-use-new-driver %s @%t.resp | %FileCheck %s
// RUN: rm %swift-bin-dir/legacy-driver-propagates-response-file.sh

// CHECK: -Xfrontend
Expand Down
2 changes: 1 addition & 1 deletion test/Driver/options-repl-darwin.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

// RUN: %empty-directory(%t/usr/bin/)
// RUN: %empty-directory(%t/usr/lib/)
// RUN: %hardlink-or-copy(from: %swift_driver_plain, to: %t/usr/bin/swift)
// RUN: %hardlink-or-copy(from: %swift_driver_plain-legacy-driver, to: %t/usr/bin/swift)

// RUN: %host-library-env %t/usr/bin/swift -repl -### | %FileCheck -check-prefix=INTEGRATED %s
// RUN: %host-library-env %t/usr/bin/swift -### | %FileCheck -check-prefix=INTEGRATED %s
Expand Down
2 changes: 1 addition & 1 deletion test/Driver/subcommands.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@
// RUN: echo "#!/bin/sh" > %t.dir/swift-foo
// RUN: echo "echo \"exec: \$0\"" >> %t.dir/swift-foo
// RUN: chmod +x %t.dir/swift-foo
// RUN: env PATH=%t.dir %swift_driver_plain foo | %FileCheck -check-prefix=CHECK-SWIFT-SUBCOMMAND %s
// RUN: env PATH=%t.dir SWIFT_USE_OLD_DRIVER=1 %swift_driver_plain foo | %FileCheck -check-prefix=CHECK-SWIFT-SUBCOMMAND %s
// CHECK-SWIFT-SUBCOMMAND: exec: {{.*}}/swift-foo
65 changes: 47 additions & 18 deletions tools/driver/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,40 @@ add_swift_parser_link_libraries(swift-frontend)
# to ensure that `swiftc` forwards to the standalone driver when invoked.
swift_create_early_driver_copies(swift-frontend)

swift_create_post_build_symlink(swift-frontend
SOURCE "swift-frontend${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swift${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")
# If a `swift-driver` executable adjacent to the `swift-frontend` executable exists
# then the `swift` and `swiftc` symlinks should point to it by-default
if(EXISTS "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-driver${CMAKE_EXECUTABLE_SUFFIX}" AND EXISTS "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-help${CMAKE_EXECUTABLE_SUFFIX}")
message(STATUS "Pointing 'swift' and 'swiftc' symlinks at 'swift-driver'.")
swift_create_post_build_symlink(swift-frontend
SOURCE "swift-driver${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swift${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")
swift_create_post_build_symlink(swift-frontend
SOURCE "swift-driver${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swiftc${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")

swift_create_post_build_symlink(swift-frontend
SOURCE "swift-frontend${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swiftc${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")
message(STATUS "Pointing 'swift-legacy-driver' and 'swiftc-legacy-driver' symlinks at 'swift-frontend'.")
swift_create_post_build_symlink(swift-frontend
SOURCE "swift-frontend${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swift-legacy-driver${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")
swift_create_post_build_symlink(swift-frontend
SOURCE "swift-frontend${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swiftc-legacy-driver${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")
else()
message(STATUS "Pointing 'swift' and 'swiftc' symlinks at 'swift-frontend' - no early SwiftDriver build found.")
swift_create_post_build_symlink(swift-frontend
SOURCE "swift-frontend${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swift${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")

swift_create_post_build_symlink(swift-frontend
SOURCE "swift-frontend${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "swiftc${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")
endif()

swift_create_post_build_symlink(swift-frontend
SOURCE "swift-frontend${CMAKE_EXECUTABLE_SUFFIX}"
Expand Down Expand Up @@ -157,15 +182,6 @@ swift_create_post_build_symlink(swift-frontend
DESTINATION "swift-parse-test${CMAKE_EXECUTABLE_SUFFIX}"
WORKING_DIRECTORY "${SWIFT_RUNTIME_OUTPUT_INTDIR}")

add_swift_tool_symlink(swift swift-frontend compiler)
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I can tell what happened here is that #6053 added this and removed the others, but that was somehow lost in the conflict resolution in #10337. I haven't looked at the LLVM closely, but I assume we should probably prefer add_swift_tool_symlink over swift_create_post_build_symlink and swift_install_in_component? Any thoughts @edymtt/@compnerd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One thing about add_swift_tool_symlink is that it requires the "source" be a component. So we can't simply call:

add_swift_tool_symlink(swift swift-driver compiler)

Because that leads to:

CMake Error at /Volumes/Data/GHWorkspace/build/Ninja-Release/llvm-macosx-arm64/lib/cmake/llvm/AddLLVM.cmake:2168 (get_target_property):
  get_target_property() called with non-existent target "swift-driver".

Copy link
Member

Choose a reason for hiding this comment

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

@bnbarham yes, we do prefer the add_swift_tool_symlink because it correctly handles the install of the link (or in the case of Windows, copy 😠). This makes it significantly better for building the toolchain distribution. Without that we will need to add a whole slew of custom install logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, we have that logic already (we have both today):

swift_install_in_component(FILES "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift${CMAKE_EXECUTABLE_SUFFIX}"
                           DESTINATION "bin"
                           COMPONENT compiler)

One thing about add_swift_tool_symlink is that it requires the "source" be a component. So we can't simply call:

We could fix that by adding a custom target for the early swift driver (which does the copy) right? ie. change swift_create_early_driver_copies to instead use add_custom_command + add_custom_target rather than configure_file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already have a slew of custom install logic though, with swift_install_in_component later in this file.

We can pursue a refactor here to replace both swift_create_post_build_symlink and swift_install_in_component with LLVM tooling, but as-is, the logic is already there, and add_swift_tool_symlink would not work for this use-case for the reason described above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, swift_install_in_component does, I believe, already do Windows-specific things (copy instead of symlink) so I don't think this will functionally change much here.

add_swift_tool_symlink(swiftc swift-frontend compiler)
add_swift_tool_symlink(swift-symbolgraph-extract swift-frontend compiler)
add_swift_tool_symlink(swift-api-extract swift-frontend compiler)
add_swift_tool_symlink(swift-autolink-extract swift-frontend autolink-driver)
add_swift_tool_symlink(swift-indent swift-frontend editor-integration)
add_swift_tool_symlink(swift-api-digester swift-frontend compiler)
add_swift_tool_symlink(swift-cache-tool swift-frontend compiler)

add_dependencies(compiler swift-frontend)
swift_install_in_component(FILES "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "bin"
Expand Down Expand Up @@ -193,4 +209,17 @@ add_dependencies(editor-integration swift-frontend)
swift_install_in_component(FILES "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-indent${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "bin"
COMPONENT editor-integration)

if(EXISTS "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-driver${CMAKE_EXECUTABLE_SUFFIX}" AND EXISTS "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-help${CMAKE_EXECUTABLE_SUFFIX}")
swift_install_in_component(PROGRAMS "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-driver${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "bin"
COMPONENT compiler)
swift_install_in_component(PROGRAMS "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-help${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "bin"
COMPONENT compiler)
swift_install_in_component(FILES "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swiftc-legacy-driver${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "bin"
COMPONENT compiler)
swift_install_in_component(FILES "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-legacy-driver${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "bin"
COMPONENT compiler)
endif()