Build tool plugins don't work on Windows

dabrahams commented 1 year ago

Description

This PR on a minimal project demonstrates. You can see that builds succeed everywhere but Windows.

This is after I've worked around the SPM Path bugs. It complains that it can't find the executable used by the build tool. Various attempts to fix it by adding .exe to the path are in the preceding commits on the same branch; none of them worked.

Expected behavior

Project works on Windows just like on Mac and Linux.

Actual behavior

No response

Steps to reproduce

No response

Swift Package Manager version/commit hash

Swift Package Manager - Swift 5.8.1

Swift & OS version (output of `swift --version ; uname -a`)

swift-driver version: 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100) Target: arm64-apple-macosx13.0 Darwin DaveA-MBP14.localdomain 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul 5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000 arm64

neonichu commented 1 year ago

2023-08-24T23:59:28.1507676Z  "C:\\Library\\Developer\\Toolchains\\unknown-Asserts-development.xctoolchain\\usr\\bin\\lld-link" "-out:D:\\a\\TestGeneration\\TestGeneration\\.build\\plugins\\cache\\TestGeneratorPlugin.exe" "-libpath:C:\\Library\\Developer\\Toolchains\\unknown-Asserts-development.xctoolchain\\usr\\lib\\clang\\15.0.0\\lib\\x86_64-unknown-windows-msvc" "-libpath:C:\\Library\\Developer\\Platforms\\Windows.platform\\Developer\\SDKs\\Windows.sdk\\usr\\lib\\swift\\windows\\x86_64" "-libpath:C:\\Library\\Developer\\Toolchains\\unknown-Asserts-development.xctoolchain\\usr\\lib\\swift\\pm\\PluginAPI" "-libpath:C:\\Library\\Developer\\Platforms\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows\\x86_64" -nologo "C:\\Library\\Developer\\Platforms\\Windows.platform\\Developer\\SDKs\\Windows.sdk\\usr\\lib\\swift\\windows\\x86_64\\swiftrt.obj" "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\TemporaryDirectory.tTymTZ\\TestGeneratorPlugin-1.o" "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\TemporaryDirectory.tTymTZ\\TestGeneratorPlugin-2.o" PackagePlugin.lib
2023-08-24T23:59:28.1510447Z [1/1] Compiling plugin TestGeneratorPlugin
2023-08-24T23:59:28.1510787Z Building for debugging...
2023-08-24T23:59:28.3271157Z error: couldn't build D:\a\TestGeneration\TestGeneration\.build\plugins\outputs\testgeneration\TestGenerationTests\TestGeneratorPlugin\GeneratedTests.swift because of missing inputs: D:\a\TestGeneration\TestGeneration\.build\x86_64-unknown-windows-msvc\debug\GenerateTests
2023-08-24T23:59:28.5378058Z ##[error]Process completed with exit code 1.

neonichu commented 1 year ago

Hm, I don't see any indication in the log that we compiled anything named GenerateTests at all? So I'm guessing something is wrong with the machinery that is supposed to build executables needed by plugins.

neonichu commented 1 year ago

I see this in PluginTarget.accessibleTools():

return try [.builtTool(name: builtToolName, path: RelativePath(validating: executableOrBinaryTarget.name))]

Probably a problem that this doesn't include .exe

dabrahams commented 1 year ago

Maybe, but as noted in the original report the previous 2 or 3 commits on that branch tried to remedy that problem and failed.

neonichu commented 1 year ago

We have targetTriple.executableExtension for this kind of situation. I don't have a Windows environment, but I wonder if it could be worth it to just change this either way. It is definitely not more wrong than what we have today.

neonichu commented 1 year ago

Maybe, but as noted in the original report the previous 2 or 3 commits on that branch tried to remedy that problem and failed.

I don't think you can remedy it from your code, it seems like it is constructing the name based on the target name?

dabrahams commented 1 year ago

I'm pretty sure I can't work around it, especially because of https://github.com/apple/swift-package-manager/issues/6524

neonichu commented 1 year ago

I think what we're doing is we're producing the the executable with .exe but we write its path to the llbuild manifest without .exe (because of the code snippet pasted above), then when we ask llbuild to produce the output of the plugin, but llbuild tells us it can't find the executable because of the truncated path.

dabrahams commented 1 year ago

I hope that means something to you and leads you to a fix!

neonichu commented 1 year ago

Don't think my theory is actually correct. At least I tried to reproduce by setting executableExtension to ".exe" for macOS (I know, a weird way to do that) and I am seeing the correct things happen despite the issue in PluginTarget.accessibleTools().

dabrahams commented 1 year ago

@neonichu I just set up a Windows dev environment on my M1 Mac with parallels and @compnerd's latest ARM Swift installer. Seems to work fine. Any chance you can get set up with that so you can stop guessing and start fixing?

dabrahams commented 1 year ago

Ah, wait, maybe I can work around it by making a build tool plugin that merely issues shell commands rather than directly running an executable target? (maybe even swift run --scratch-path /tmp/foo theTarget)? I think that's possible, although the examples I've found of build tools don't make it clear how to do it.

dabrahams commented 1 year ago

Ah, wait, maybe I can work around it by making a build tool plugin that merely issues shell commands rather than directly running an executable target?

No, the problem is still that invoking the shell executable is impossible on Windows. I am totally stuck without an SPM fix.

dabrahams commented 1 year ago

FWIW, our project is dropping Windows support because of this. We're already not testing on Windows (just building) because some of our tests are generated by a plugin, but now we want to start generating source files. /cc @compnerd

compnerd commented 1 year ago

I wonder if you can get away with something like:

import Foundation
let path = URL(fileURLWithPath: ...).withUnsafeFileSystemRepresentation { String(cString: $0!) }
return Path(path)

I believe that the core issue is the paths.

dabrahams commented 1 year ago

@compnerd what would you put where the "..." go? A path including ".exe"?

This project is trivial; if you can make the TestGenerationPlugin work, its tests will run on Windows and you can demonstrate the trick in a way I can reproduce it at scale. I pushed my changes to the windows branch where you can see how it fails in CI.

dabrahams commented 1 year ago

I'm actually pretty close, having fixed the off-by-one errors in my use of GetFullPathNameW.

Now on my local VM, the test generation executable runs and generates the appropriate swift file. The problem I'm having now is that the linker seems to be gathering the main from the build tool and the test runner and, of course, failing.

^
warning: 'testgeneration': found 2 file(s) which are unhandled; explicitly declare them as resources or exclude from the target
    C:\Users\dave\src\TestGeneration\Tests\TestGenerationTests\TestCases\two.testgen
    C:\Users\dave\src\TestGeneration\Tests\TestGenerationTests\TestCases\one.testgen
Building for debugging...
[7/7] Linking C:\Users\dave\src\TestGeneration\.build\plugins\tools\debug\GenerateTests.exe
Build complete! (14.13s)
Building for debugging...
lld-link: error: duplicate symbol: main
>>> defined at C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\GenerateTests.build\GenerateTests.swift.o
>>> defined at C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\TestGenerationPackageTests.build\runner.swift.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
[29/30] Linking C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\TestGenerationPackageTests.xctest
error: fatalError
PS C:\Users\dave\src\TestGeneration>

The last command issued by "swift build --build-tests -v" (replacing "C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug" with "Z" for concision) is, FWIW:

 "C:\\Program Files\\Swift\\Toolchains\\0.0.0+Asserts\\usr\\bin\\lld-link" "-out:Z\\TestGenerationPackageTests.xctest" "-libpath:C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.37.32822\\lib\\arm64" "-libpath:C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.37.32822\\atlmfc\\lib\\arm64" "-libpath:C:\\Program Files (x86)\\Windows Kits\\10\\Lib\\10.0.22621.0\\ucrt\\arm64" "-libpath:C:\\Program Files (x86)\\Windows Kits\\10\\Lib\\10.0.22621.0\\um\\arm64" "-libpath:C:\\Program Files\\Swift\\Toolchains\\0.0.0+Asserts\\usr\\lib\\clang\\16.0.0\\lib\\aarch64-unknown-windows-msvc" "-libpath:C:\\Program Files\\Swift\\Platforms\\Windows.platform\\Developer\\SDKs\\Windows.sdk\\usr\\lib\\swift\\windows\\aarch64" "-libpath:Z" "-libpath:C:\\Program Files\\Swift\\Platforms\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows\\aarch64" "-libpath:C:\\Program Files\\Swift\\Toolchains\\0.0.0+Asserts\\usr\\lib" -nologo "C:\\Program Files\\Swift\\Platforms\\Windows.platform\\Developer\\SDKs\\Windows.sdk\\usr\\lib\\swift\\windows\\aarch64\\swiftrt.obj" "Z\\GenerateTests.build\\GenerateTests.swift.o" "Z\\GenerateTests.build\\GenerateTests.swiftmodule.o" "Z\\TestGenerationPackageDiscoveredTests.build\\TestGenerationPackageDiscoveredTests.swiftmodule.o" "Z\\TestGenerationPackageDiscoveredTests.build\\TestGenerationTests.swift.o" "Z\\TestGenerationPackageDiscoveredTests.build\\all-discovered-tests.swift.o" "Z\\TestGenerationPackageTests.build\\TestGenerationPackageTests.swiftmodule.o" "Z\\TestGenerationPackageTests.build\\runner.swift.o" "Z\\TestGenerationTests.build\\GeneratedTests.swift.o" "Z\\TestGenerationTests.build\\TestGenerationTests.swift.o" "Z\\TestGenerationTests.build\\TestGenerationTests.swiftmodule.o" -debug:dwarf

In CI, of course, the results are different. It looks like the SPM there thinks the build tool executable file doesn't end in ".exe" and therefore doesn't find it.

compnerd commented 1 year ago

Hmm, I think that the link error is because of tests for executables? Those don't work on Windows due to the way that they are executed (main is linked duplicately and relies on aliasing to not get picked up). Windows doesn't have any tools to emulate such horrible hacks. This one is on @neonichu.

neonichu commented 1 year ago

I think we have an existing GH issue to disable testing executables on Windows, but we never did? IIRC, @tomerd had some concerns.

dabrahams commented 1 year ago

Hmm, I think that the link error is because of tests for executables?

? Nothing in that reproducer is trying to test an executable.

neonichu commented 1 year ago

Could you try using a 5.9 nightly toolchain and bumping the tools-version of your example to 5.9? I wonder if https://github.com/apple/swift-package-manager/pull/6723 is involved in the linker error you're seeing, it looks like we're incorrectly linking the GenerateTests executable's code into the test bundle.

dabrahams commented 1 year ago

@neonichu I am using a Windows install I got from @compnerd a couple weeks ago. Is that recent enough? If not, @compnerd, I might need some help getting the latest installed.

dabrahams commented 1 year ago

Changing swift-tools-version to 5.9 using my current Swift installation has no apparent effect on the bug. I am running on Arm64, so I can't just grab the latest installer from the website. My current version is:

Swift version 5.9-dev (LLVM ad32770d6738638, Swift 753d54576e043ca) Target: aarch64-unknown-windows-msvc

neonichu commented 1 year ago

Seems like it should be new enough to contain the fix for https://github.com/apple/swift-package-manager/pull/6723, so I don't think that's the linker issue, then.

dabrahams commented 1 year ago

The resource-generation branch of the same project actually demonstrates that the duplicate symbol problem is specific to tests that transitively depend on build tool plugins that depend on executable targets. An executable (which also has a main) that depends on the same library as the test and thus transitively on the same things, can be built without problems but if you try to run the tests, no dice.

dabrahams commented 1 year ago

…of course, occasionally, seemingly randomly, everything works:

PS C:\Users\dave\src\TestGeneration> swift test               
warning: 'testgeneration': found 2 file(s) which are unhandled; explicitly declare them as resources or exclude from the target
    C:\Users\dave\src\TestGeneration\Sources\LibWithResource\Resources\Test1.in
    C:\Users\dave\src\TestGeneration\Sources\LibWithResource\Resources\Test2.in
Building for debugging...

[5/5] Linking C:\Users\dave\src\TestGeneration\.build\plugins\tools\debug\GenerateResource.exe
Build complete! (0.72s)
Building for debugging...
lld-link: warning: C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\ResourceGenerationPackageDiscoveredTests.build\ResourceGenerationTests.swift.o: locally defined symbol imported: $s23ResourceGenerationTestsAAC6testItyyKF (defined in C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\ResourceGenerationTests.build\ResourceGenerationTests.swift.o) [LNK4217]
lld-link: warning: C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\ResourceGenerationPackageDiscoveredTests.build\ResourceGenerationTests.swift.o: locally defined symbol imported: $s23ResourceGenerationTestsAACMn (defined in C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\ResourceGenerationTests.build\ResourceGenerationTests.swift.o) [LNK4217]
lld-link: warning: C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\ResourceGenerationPackageTests.build\runner.swift.o: locally defined symbol imported: $s40ResourceGenerationPackageDiscoveredTests05__alldE0Say6XCTest0G4CaseCm04testH5Class_SaySS_yAEKctG0fE0tGyF (defined in C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\ResourceGenerationPackageDiscoveredTests.build\all-discovered-tests.swift.o) [LNK4217]
lld-link: warning: C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\AppWithResource.build\AppWithResource.swift.o: locally defined symbol imported:lld-link: warning:  $s15LibWithResource14resourceBundle10Foundation0E0Cvau (defined in C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\LibWithResource.build\LibWithResource.swift.o) [LNK4217]
[40/40] Linking C:\Users\dave\src\TestGeneration\.build\aarch64-unknown-windows-msvc\debug\AppWithResource.exe
Build complete! (7.50s)
Test Suite 'All tests' started at 2023-09-14 18:43:20.921
Test Suite 'debug.xctest' started at 2023-09-14 18:43:20.937
Test Suite 'ResourceGenerationTests' started at 2023-09-14 18:43:20.937
Test Case 'ResourceGenerationTests.testIt' started at 2023-09-14 18:43:20.937
Test Case 'ResourceGenerationTests.testIt' passed (0.0 seconds)
Test Suite 'ResourceGenerationTests' passed at 2023-09-14 18:43:20.937
         Executed 1 test, with 0 failures (0 unexpected) in 0.0 (0.0) seconds
Test Suite 'debug.xctest' passed at 2023-09-14 18:43:20.937
Executed 1 test, with 0 failures (0 unexpected) in 0.0 (0.0) seconds
Test Suite 'All tests' passed at 2023-09-14 18:43:20.937
         Executed 1 test, with 0 failures (0 unexpected) in 0.0 (0.0) seconds

dabrahams commented 1 year ago

I finally found a set of workarounds that portably allows tests to depend (indirectly) on build tool plugins that use Swift executables. I've factored and commented the code extensively if you care to study it, in the resource-generation branch. Almost everything of interest is here.

lynchsft commented 12 months ago

I'm encountering this same issue Windows in Swift 5.9.1 when trying to link tests that transiently depend on a macro.

Building for debugging...
lld-link: error: duplicate symbol: main
>>> defined at C:\\Users\\lynchsft\\e3\\.build\\x86_64-unknown-windows-msvc\\debug\\SymphonyPackageTests.build\\runner.swift.o
>>> defined at C:\\Users\\lynchsft\\e3\\.build\\x86_64-unknown-windows-msvc\\debug\\WoodwindMacros.build\\Plugin.swift.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
[1/2] Linking C:\Users\lynchsft\e3\.build\x86_64-unknown-windows-msvc\debug\SymphonyPackageTests.xctest
error: fatalError
PS C:\Users\lynchsft\e3>

As stated above, the main product links correctly and the tests compile. It's the final linking of the test runner that fails. This product builds and is deployed on Mac, Linux and Windows. We're excited to use the new macros feature but we won't be able to if we can't test the product on Windows. :(

Thank you in advance for your time and attention.

dabrahams commented 12 months ago

I have workarounds for these problems; see https://github.com/dabrahams/SPMBuildToolSupport (don't use the .swift executable option yet; I'm still trying to get that part to work on Windows)

lynchsft commented 12 months ago

Thank you for your response @dabrahams . I've read your solution twice, once before and once after posting. My reading says your solution applies to plugins but does not offer any support for macros. Am I misunderstanding?

dabrahams commented 12 months ago

There's no specific support for macros. Do they interact with SPM somehow? My solution only addresses the title of this issue. (I've removed the option to run a swift script from the plugin; the swift interpreter simply doesn't work on Windows, and this is the nail in the coffin of any workaround).

lynchsft commented 11 months ago

@neonichu Would you like to see this issue ticketed separately for macros?

The locus of this problem is on Windows platform, but I conclude that it indirectly negatively affects the entire Swift community. As macros proliferate through common libraries, all swift-on-windows tools will either have to abandon testing or wholly abandon windows as a platform. Neither of these outcomes speaks well of the "swift is a cross platform language" story.

MaxDesiatov commented 11 months ago

What issues with macros do you see on Windows? Macros are not build tool plugins, a separate issues filed with self-contained repro steps would be much appreciated.

lynchsft commented 11 months ago

As shown above I'm seeing the duplicate main symbol when linking a test runner that indirectly depends on a macro. I comprehend that macros != plugins but they share the trait that they compile to separate executables not intended to be linked into the test runner. I'll aim to create a reproducible test repo as soon as possible.

dabrahams commented 11 months ago

@lynchsft the duplicate main issue I've been seeing only shows up on Windows. Is that the same as with your case? I can explain how I worked around that problem and maybe(?) you can figure out how to apply a similar workaround for macros.

neonichu commented 11 months ago

I'm not sure we need a repro case for the macro issue tbh, I believe the issues here are well-known:

the "testable executables" features (that is also applied to macros) makes it impossible to differentiate between using and testing a macro, so macros will always be linked into test targets
additionally the "testable executables" feature doesn't work at all on Windows, so we end up with conflicting main functions

The combination of both leads to a pretty bad experience here.

lynchsft commented 11 months ago

This is a trivial repo that reproduces the issue: https://github.com/lynchsft/swift-windows-macro-testing

@dabrahams Yes, the problem arises only on Windows.

@neonichu The experience is bad (can't test my product on a supported Platform). Particularly, I'm asserting that this is a rapidly worsening problem because library authors that don't usually concern themselves with Windows are eagerly adopting macros feature. Soon, the libraries that Windows projects currently rely on will be un-ingestiable, effectively locking our dependences at the last-macro-free version. This in turn, is a security problem as code-currency (up-to-date dependencies) are a crucial topic for averting and responding to security threats.

lynchsft commented 11 months ago

@neonichu, @MaxDesiatov During construction of that ^^ trivial reproduction repo, I actually encountered a success case, where I WAS able to ingest a macro and use it on Windows. The linker error did not arise. Would this state be a useful resource to have?

You can find the successful (can link tests on Windows) example in a branch of the same repo: Branch: success_case Repo: https://github.com/lynchsft/swift-windows-macro-testing

dabrahams commented 11 months ago

This is a trivial repo that reproduces the issue: https://github.com/lynchsft/swift-windows-macro-testing

@lynchsft

Can you make it fail without the unused swift-syntax dependency in Package.swift?
IMO this case is hardly trivial, since it uses a dependency package that is pretty complex. A more useful reproducer would be self-contained.
Don't you want to open a new issue for this problem, since it has nothing to do with build tool plugins? I worry that a pile of unrelated comments will cause maintainers to ignore this issue.

lynchsft commented 11 months ago

Thank you for the cleanliness reminder dabrahms. Link to new ticket: https://github.com/apple/swift-package-manager/issues/7174

My argument is that common libraries (the two used in this example are linked from the new Swift Package Index in the "macros" section) will force Windows development into an untenable position.

swiftlang / swift-package-manager