Make sure we actually use `swift-driver` #5842

neonichu · 2022-10-25T23:10:36Z

In many cases, we're directly executing swiftc which means we are relying on the old driver. This switches us to calling swift-driver --driver-mode=swiftc instead if available.

neonichu · 2022-10-25T23:11:21Z

Sources/Build/LLBuildManifestBuilder.swift

+            throw StringError("empty commandline for Swift compilation")
+        }
+
+        // TODO: `--driver-mode=swiftc` needs to be the first argument, but going through `SwiftCompilerTool` does not allow that


To fix this we'd need to move off llbuild's built-in tool and instead create a shell command (which is probably better anyway).

neonichu · 2022-10-25T23:11:31Z

@swift-ci please smoke test

neonichu · 2022-10-25T23:27:20Z

@swift-ci please smoke test

neonichu · 2022-10-25T23:37:16Z

@swift-ci please smoke test

tomerd · 2022-10-26T00:00:03Z

Sources/PackageModel/Toolchain.swift

        extraFlags.swiftCompilerFlags
    }
+
+    private static func commandLineForCompilation(compilerPath: AbsolutePath, fileSystem: FileSystem) -> [String] {


nit: reverse order of arguments

tomerd · 2022-10-26T00:00:30Z

Sources/SPMTestSupport/Toolchain.swift

    }
-
-    /// Helper function to determine whether serialized diagnostics work properly in the current environment.
-    public func supportsSerializedDiagnostics(otherSwiftFlags: [String] = []) -> Bool {


not needed?

That's the hope/goal since this was needed because we were using the old driver in CI contexts (which apparently has a bug triggered by some of our tests).

neonichu · 2022-10-26T17:03:19Z

@swift-ci please smoke test

neonichu · 2022-10-27T01:05:42Z

Looks like right now we're not even installing the driver, so we'd need to change that in the preset / CI job first.

neonichu · 2022-10-27T18:13:37Z

swiftlang/swift#61766
@swift-ci please smoke test

neonichu · 2022-10-27T21:25:34Z

swiftlang/swift#61766
@swift-ci please smoke test

neonichu · 2022-10-27T23:35:18Z

swiftlang/swift#61766
@swift-ci please smoke test

neonichu · 2022-10-28T06:19:04Z

Doesn't look like swift-driver is present inside the toolchain installation directory?

--- bootstrap: note: Symlinking /Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift-driver
--- bootstrap: note: Available executables: ['swift-api-extract', 'swift-build-tool', 'swift-symbolgraph-extract', 'clang', 'llvm-cov', 'swift-demangle', 'clangd', 'clang-13', 'swift-frontend', 'clang-cl', 'dsymutil', 'clang-cache', 'sdk-module-lists', 'swift-stdlib-tool', 'swift-api-checker.py', 'swift', 'clang++', 'swiftc', 'clang-cpp', 'llvm-profdata', 'swift-autolink-extract', 'swift-api-digester']

finagolfin · 2022-10-28T14:43:16Z

What happened to the integrated swift driver that was added last year, #2736, did that not work out? Seems a waste to include that when building SPM and then not use it.

neonichu · 2022-10-31T17:24:42Z

What happened to the integrated swift driver that was added last year, #2736, did that not work out? Seems a waste to include that when building SPM and then not use it.

I'm not 100% sure (cc @artemcm), but either way we cannot use it for manifest compilation / plugin loading since they happen separately from the build system.

neonichu · 2022-11-01T00:04:14Z

I think my Swift PR might be incorrect and install swift-driver after building SwiftPM, so that's why it is missing here.

finagolfin · 2022-11-01T06:24:12Z

Yes, SPM is built and installed first because it is then used to build and install swift-driver, at least on linux and Android.

neonichu · 2022-11-02T18:55:32Z

swiftlang/swift#61766
@swift-ci please smoke test

neonichu · 2022-11-03T00:11:49Z

OK, I added a install-swift-driver before swiftpm in the preset, but it doesn't seem to have affected things a whole lot? Still doesn't seem to be present in the installation directory.

--- bootstrap: note: Symlinking /Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/build/buildbot_incremental/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift-driver
--- bootstrap: note: Available executables: ['swift-api-extract', 'swift-build-tool', 'swift-symbolgraph-extract', 'clang', 'llvm-cov', 'swift-demangle', 'clangd', 'clang-13', 'swift-frontend', 'clang-cl', 'dsymutil', 'clang-cache', 'sdk-module-lists', 'swift-stdlib-tool', 'swift-api-checker.py', 'swift', 'clang++', 'swiftc', 'clang-cpp', 'llvm-profdata', 'swift-autolink-extract', 'swift-api-digester']

finagolfin · 2022-11-03T04:39:27Z

The order of the flags is irrelevant, as SPM is used to build swift-driver, so it is always built and tested before.

neonichu · 2022-11-03T16:33:01Z

The order of the flags is irrelevant, as SPM is used to build swift-driver, so it is always built and tested before.

Ah interesting, that seems unfortunate. We're actually already building the driver as part of the first bootstrapping stage, maybe we can just change things to re-use the built products from that.

finagolfin · 2022-11-03T17:51:12Z

Only the macOS CI builds the early swift-driver, the linux CI doesn't as it doesn't have a prebuilt Swift compiler installed.

finagolfin · 2022-11-03T17:52:34Z

Oh, if you mean when bootstrapping SPM itself, not sure what executables that generates.

neonichu · 2022-11-03T21:45:06Z

So I think a viable path here could be:

add the ability to specify a custom swift-driver (@compnerd also asked for this) and point it to the one anyway produced by building swift-driver with CMake during bootstrap
tell the driver to use the swiftc we're already using (might already be happening)

This tries to use the swift-driver built during the first-stage for the "fake" toolchain we're constructing for bootstrapping. Basically a variation on #5842. Trying first whether it is sufficient to just have the symlink, if not we should be able to combine the approach from #5842 and this together.

In many cases, we're directly executing `swiftc` which means we are relying on the old driver. This switches us to calling `swift-driver --driver-mode=swiftc` instead if available.

neonichu · 2022-11-09T01:32:09Z

@swift-ci please smoke test

neonichu · 2022-12-08T17:56:11Z

@swift-ci please smoke test

neonichu · 2022-12-08T20:30:58Z

Probably time to abandon this, actually. Failures on self-hosted are proving that even explicitly using swift-driver doesn't help us:

Using compiler command line: ["/Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift-driver", "--driver-mode=swiftc"]

neonichu self-assigned this Oct 25, 2022

neonichu commented Oct 25, 2022

View reviewed changes

neonichu force-pushed the use-driver branch from 5094c27 to 5ae1544 Compare October 25, 2022 23:27

neonichu force-pushed the use-driver branch from 5ae1544 to eb67a6d Compare October 25, 2022 23:36

tomerd reviewed Oct 26, 2022

View reviewed changes

neonichu force-pushed the use-driver branch from eb67a6d to 06d7317 Compare October 26, 2022 17:02

neonichu force-pushed the use-driver branch from 06d7317 to 9bb6a5d Compare October 27, 2022 21:25

neonichu force-pushed the use-driver branch from 9bb6a5d to 33b49c4 Compare October 27, 2022 23:35

neonichu mentioned this pull request Nov 3, 2022

Install swift-driver as part of SwiftPM builds swiftlang/swift#61766

Closed

neonichu mentioned this pull request Nov 8, 2022

Make sure we actually use swift-driver #5885

Closed

Make sure we actually use swift-driver

f593faf

In many cases, we're directly executing `swiftc` which means we are relying on the old driver. This switches us to calling `swift-driver --driver-mode=swiftc` instead if available.

neonichu force-pushed the use-driver branch from 33b49c4 to f593faf Compare November 9, 2022 01:31

neonichu closed this Dec 8, 2022

neonichu deleted the use-driver branch December 8, 2022 20:31

rauhul mentioned this pull request Nov 7, 2023

Update mixin_swiftpm_base to include swift-driver swiftlang/swift#69696

Closed

Make sure we actually use swift-driver #5842

Make sure we actually use swift-driver #5842

Uh oh!

Conversation

neonichu commented Oct 25, 2022

Uh oh!

neonichu Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

neonichu commented Oct 25, 2022

Uh oh!

neonichu commented Oct 25, 2022

Uh oh!

neonichu commented Oct 25, 2022

Uh oh!

tomerd Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

tomerd Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

neonichu Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

neonichu commented Oct 26, 2022

Uh oh!

neonichu commented Oct 27, 2022

Uh oh!

neonichu commented Oct 27, 2022

Uh oh!

neonichu commented Oct 27, 2022

Uh oh!

neonichu commented Oct 27, 2022

Uh oh!

neonichu commented Oct 28, 2022

Uh oh!

finagolfin commented Oct 28, 2022

Uh oh!

neonichu commented Oct 31, 2022

Uh oh!

neonichu commented Nov 1, 2022

Uh oh!

finagolfin commented Nov 1, 2022

Uh oh!

neonichu commented Nov 2, 2022

Uh oh!

neonichu commented Nov 3, 2022

Uh oh!

finagolfin commented Nov 3, 2022

Uh oh!

neonichu commented Nov 3, 2022

Uh oh!

finagolfin commented Nov 3, 2022

Uh oh!

finagolfin commented Nov 3, 2022

Uh oh!

neonichu commented Nov 3, 2022

Uh oh!

neonichu commented Nov 9, 2022

Uh oh!

neonichu commented Dec 8, 2022

Uh oh!

neonichu commented Dec 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Make sure we actually use `swift-driver` #5842

Make sure we actually use `swift-driver` #5842