swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.71k stars 1.34k forks source link

Detect `link` tool automatically on Windows, and fallback to `lld-link` if not found #5771

Open stevapple opened 2 years ago

stevapple commented 2 years ago

Description

In Swift 5.7, we can use SwiftPM in regular CMD or PowerShell environment, but after #5720 we're getting error: toolchain is invalid: could not find link. This is not desirable, and we should both try to detect link and fallback to lld-link if link can't be found.

Expected behavior

SwiftPM could work without special developer environment.

Actual behavior

It just throws error: toolchain is invalid: could not find link.

Steps to reproduce

  1. Open a regular CMD or PowerShell window;
  2. Change to the root of any Swift package;
  3. Run swift build.

Swift Package Manager version/commit hash

e1e80d7024d0a2c62dee37841d242de7560cfd75

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

swift.org Swift version 5.8-dev (LLVM efc29aad7a9faa8, Swift f926816e5105fca) Target: x86_64-unknown-windows-msvc

Windows 10 Pro Workstation 21H1 Visual Studio 17.3.4

stevapple commented 2 years ago

Additional Context

tomerd commented 2 years ago

cc @compnerd I think you worked on this specific change?

compnerd commented 2 years ago

I'm not convinced that this is the right thing to do in general. If this was meant specifically for static libraries, then I would say that it makes sense. However, in that case, this is basically duplicating #5719 and #5761. I would be slightly against the idea of the vswhere dependency. If the build tools are installed, I believe that vswhere is not going to be installed and additional dependencies on the setup sound like a poor choice given that the current dependency set is already difficult. I'm not sure why swift-driver is being brought up here though.

stevapple commented 2 years ago

If the build tools are installed, I believe that vswhere is not going to be installed and additional dependencies on the setup sound like a poor choice given that the current dependency set is already difficult.

That’s exactly not the case — vswhere is guaranteed to exist on every machine that has Visual Studio installed, so we don’t need to vendor it.

However, in that case, this is basically duplicating #5719 and #5761.

I certainly missed #5761, but #5719 is completely different and irrelevant. Since LLVM variation of tools like lld-link and llvm-ar are designed for drop-in substitution, I believe falling back to LLVM tools totally makes sense.

I'm not sure why swift-driver is being brought up here though.

Swift Driver also needs to find link for static linking, but the problem is slighter because at least it works everywhere link is not actually required.

I don’t know whether MSVC toolset detection should be implemented in TSC or not (since I agree that’s something we want to get rid of), but certainly both Swift Driver and SwiftPM could make use of such utility.

compnerd commented 2 years ago

That’s exactly not the case — vswhere is guaranteed to exist on every machine that has Visual Studio installed, so we don’t need to vendor it.

That is the point - you don't need to have Visual Studio installed, the build tools are sufficient. The question is, does that include vswhere? I don't recall it doing so in the past.

I certainly missed #5761, but #5719 is completely different and irrelevant. Since LLVM variation of tools like lld-link and llvm-ar are designed for drop-in substitution, I believe falling back to LLVM tools totally makes sense.

I disagree - #5719 is entirely the same and relevant. The point is to be able to cascade back to llvm tools - aka fallback to lld-link.

stevapple commented 2 years ago

The question is, does that include vswhere? I don't recall it doing so in the past.

vswhere.exe stays at determined path %ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe as part of Visual Studio installer. As a developer tool, what we should do is to simplify the setup for most user in a manageable way. We’re not going to fully rely on vswhere of course. Path and vswhere as two paths to find MSVC tools should fall back to one another, and the priority could be further discussed and even changed afterwards.

I disagree - #5761 is entirely the same and relevant. The point is to be able to cascade back to llvm tools - aka fallback to lld-link.

If I understand correctly, the key point of #5761 is the ability to manually pick librarian & improvements to ignore missing librarian if it’s not actually used. The fallback solution is, at least, not mentioned in the original issue.

In a word, #5761 is about removing the bar of having a librarian to use SwiftPM, and this issue focuses on the discovery and substitution mechanism where capable librarians exist but can’t be discovered at the point.

compnerd commented 2 years ago

Sorry, I had typo'ed the issue, it is #5719. It is not the ability to manually pick the librarian - we have that already: the AR environment variable should be honored. This is about the default librarian preferring link whenever possible, unless -use-ld=lld is used, where lld-link should be preferred over link. However, if link is not found, it makes sense to fallback to lld-link as that should be available and I don't see any reason to believe that using lld-link as the librarian would cause any type of issue (unlike using lld-link as the linker over link).

As to controlling the librarian, I believe that the environment variable AR does grant us that control. Not requiring a librarian unless used is an interesting idea, though I do not know how to implement that properly off the top of my head.

lxbndr commented 1 year ago

That is the point - you don't need to have Visual Studio installed, the build tools are sufficient. The question is, does that include vswhere? I don't recall it doing so in the past.

BTW, I mostly use Build Tools, and vswhere is always present. Build Tools is considered another kind of product though, making it a bit tricky to query it the right way.

neonichu commented 1 year ago

Don't know anything about development on Windows, but it sounds like vswhere is similar to xcrun on macOS? One option for a fallback would be if we don't find a certain tool via PATH and vswhere exists, we use it to query for a fallback. I think that's something that would be reasonable as an addition to UserToolchain.

gregcotten commented 1 year ago

Hi, I'm running into this issue on the latest windows dev snapshot - what do I need to do differently? I must say it was very convenient being able to run 5.7.x in a normal cmd or powershell without any fiddling.

@compnerd, what is the workaround or fix for error: toolchain is invalid: could not find link?

compnerd commented 1 year ago

Running from the VSDevCmd shell ("x64 Native Developer Command Prompt") is the preferred workaround. You can also just run VsDevCmd or you can set AR. https://github.com/apple/swift-package-manager/pull/5981 should allow you to run without the configuration I believe, but there are questions on how to best proceed there.

compnerd commented 1 year ago

@lxbndr "%vswhere%" -nologo -latest -products "*" -all -prerelease -property installationPath should give us either the build tools or VSCode including pre-release versions. I agree it is a bit more complicated, and I'm not a big fan of shelling out (especially since Foundation has a known issue with sometimes hanging).

stevapple commented 1 year ago

BTW, I mostly use Build Tools, and vswhere is always present. Build Tools is considered another kind of product though, making it a bit tricky to query it the right way.

vswhere is not part of VS installation — it's part of the installer. As long as your build tools are installed through the VS installer it should be there.