Provide wrappers for tool binaries instead of symlink about swiftly HOT 10 OPEN

FYI

apple/swift-driver#1583

from swiftly.

I would rather we did not use proxy shell scripts. The swiftly swift install is non-standard in that the binaries are referenced from a folder outside the actual swift toolchain folder. With symbolic links it is very easy to find the true bin folder (and thus the toolchain folder) for the current installation by following the symbolic links. If we change this to wrapper shell scripts this process becomes a lot harder and more brittle as we have to parse these shell scripts and rely on them following a certain structure.

Being able to reach the actual true swift installation is needed by many tools that makes use of the swift toolchain eg the VSCode swift extension.

from swiftly.

If we change this to wrapper shell scripts this process becomes a lot harder and more brittle as we have to parse these shell scripts and rely on them following a certain structure.

Sorry, I don't understand this. Could you elaborate "parse these shell scripts"?
As far as I know, having a wrapper for the initial launch of actual swift command won't be a problem as long as the actual binary path is not moved outside of the toolchain's usr/bin because all toolchain resources like stdlib swiftmodule/library binary/tools are resolved relatively from the driver executable file's path.

Also, from tooling perspective, it won't change anything from the current situation because they just find a proxy executable instead of a symbolic link and those two work in the same way.

from swiftly.

Sorry maybe I wasn't clear enough. I'll give you an example

The VSCode swift extension needs to know where the LLDB lib liblldb.so is. It needs to pass it the vscode-lldb extension to ensure it uses the swift version of LLDB. If ~/.local/bin/swift is a symbolic link I can follow the link to find the actual location of the swift executable and use this information alongside knowledge of the structure of the toolchain to get the location of the LLDB lib.

If ~/.local/bin/swift is a proxy (shell script or whatever) I do not have information about the real location of the swift executable.

from swiftly.

Okay, I understand the problem. Thank you for elaborating clearly!

However, I think avoiding proxy loses a lot of opportunities to provide practical features (e.g. select version based on .swift-version file, select a specific version locally (not globally) )

IMHO, for the liblldb issue, the proper way would be to ask swift command to provide the canonical toolchain directory by something like swift --print-toolchain-path instead of assuming the given swift executable should be or point the canonical toolchain binary. Note that the driver doesn't have the option now but we can have some workarounds for now by getting the current executable (lldb) by REPL mode (I think there are still some other ways):

$ echo "print(CommandLine.arguments[0])" | swift repl 2> /dev/null
/home/katei/.local/share/swiftly/toolchains/main-snapshot-2022-06-08/usr/bin/repl_swift

from swiftly.

@patrickfreed Do you have any thoughts on this?

from swiftly.

I think the idea of using proxies rather than simple symlinks is very interesting (and TIL rustup uses them, that was an oversight by me in the design), but I'm hesitant to introduce them until we have a specific feature in mind that we want.

I think you've pointed out the two main use-cases, namely a local specification of the in-use toolchain (e.g. a .swift-version file or some such) or some alternative one-off syntax for selecting a version (similar to +nightly in cargo).

Regarding the former, it was an explicit decision to omit that feature for now for and to see if it turned out to be something that's important to users.

Regarding the latter, swiftly won't be as ubiquitous in the Swift ecosystem as rustup is in the Rust ecosystem, so I think introducing new syntax to the swift executable might not be the best idea (you can imagine users writing scripts that work with swiftly-installed toolchains but not manually downloaded ones). We might also run into issues supporting such syntax on macOS. Instead, I think it might be better to introduce a new command in swiftly itself for that kind of functionality (see #22), which would mean that shims wouldn't be required.

So in summary, I think its an interesting idea that we might want to consider pursuing in the future, but I'd rather let it be done as part of implementing some other user-visible feature rather than in anticipation of such a feature. I will admit that switching from symlinks to shims in a later release of swiftly will require a bit more work from us (it should be invisible to the user at least), but I think I'm okay with that.

Thanks for bringing this up!

from swiftly.

Okay, it makes sense to me.

from swiftly.

@adam-fowler Typically it's the proxy/shim's job to show the toolchain being used to the user. This means with swiftly shim being adopted the VSCode extension can use something like swiftly show toolchain-path to get the real path if it really needs to.

Too easy access to the toolchain installation has been and is still a big problem for Swift (especially on Linux and Windows). Ideally we need only swift and swiftc in $PATH for daily use, and should be able to use Swift with nothing in $PATH. The DEB and RPM packages already did it and it's appealing to make it the new default for packaging (including on Windows, where symlinks are not capable and a shim could be the only choice).

So from my point, we can start with a real bin/ directory with swift and swiftc symlinks pointing to the tools (or pointing to swift-driver directly). This is not so complicated but would largely ease the transition to using a shim.

from swiftly.

If apple/swift#70932 (#92) doesn't get resolved, we may need to consider doing this to ensure swiftly continues to work with the new driver. Ideally, we won't be forced to do anything here though--I agree with @adam-fowler's assessment that this really is a bug.

from swiftly.