Swiftly proxies - Githubissues

cmcgee1024 commented 1 month ago

Add a design for the new swiftly proxy system for more flexible routing of command invocations to a toolchain.

Re-target the symbolic links that swiftly creates to point to swiftly itself, which can decide the selected toolchain more flexibly on a per-invocation basis.

Add support for reading the toolchain version from a .swift-version file if a selector isn't given. The swiftly use command will attempt to update a swift version file, or in certain circumstances it will create the version file for you. The special swiftly install with no selector argument that will attempt to install a toolchain version specified in a version file.

Create a swiftly run subcommand that can run arbitrary commands, such as build scripts or makefile generators. This subcommand will set the PATH environment variable to the currently selected toolchain, and other special environment variables like CC and CXX to specifically point to the clang and clang++ executables. Add an ability for this subcommand to accept arbitrary toolchain selectors that take the higher precedence over the presence of the swift version file or the global default.

swiftly run swift build +latest
swiftly run cmake -G ninja +main-snapshot +install

Update swiftly use and swiftly list to take into account that there is a global default, which is the fallback if there is no .swift-version file, but also a potentially different toolchain that is in use in the current context. The swiftly use subcommand with not arguments will show in brackets the reason why the current in-use toolchain is selected, whether it is a special .swift-version file in which case it will show the file path, or it is the global default.

BrianHenryIE commented 1 month ago

Hijackingswift altogether isn't ideal. I've used which swift to determine the toolchain directory and proxying adds a level of indirection that isn't obvious.

https://github.com/anders0nmat/swift-embedded-pm/pull/3/files

Consider:

swiftly run 5.10.1 build

Or proxy CLI arguments that don't exist in swiftly to swift:

swiftly build

(I just write Swift for fun, so take my suggestions with a grain of salt)

cmcgee1024 commented 1 month ago

@BrianHenryIE your feedback is appreciated, thank you.

I've used which swift to determine the toolchain directory and proxying adds a level of indirection that isn't obvious.

https://github.com/anders0nmat/swift-embedded-pm/pull/3/files

It seems from the above patch that the end goal is to be able to invoke the llvm-ar executable from the current toolchain. I'm curious, why dig through the symlink to the real path using the realpath command? The llvm-ar command should be a sibling of the swift command, whether if it is a symlink created by swiftly, or a toolchain binary installed by other means (e.g. manual installation, system package). Inspecting the symlink would seem to couple this CMake script to swiftly being the toolchain manager.

Consider:

swiftly run 5.10.1 build

Providing a swiftly run subcommand is something that we can consider as an enhancement to the current swiftly design. We could allow someone to run any command that they want, not just the toolchain ones. The command runs where the PATH is set to the toolchain binaries with the highest precedence. Also, we can set certain environment variables like CC and CXX to the selected clang version.

# Let's use the swift toolchain to compile this C project:
swiftly run 5.10 ./configure
swiftly run 5.10 make

We can put these ideas and more into #22

cmcgee1024 commented 1 month ago

I'm not 100% convinced by this yet. I wonder how the core team would feel about the swift exe being hijacked by swiftly.

I'm definitely interested to hear if there are opinions from the core team about this enhancement. I prefer to think of this as extending the semantics, not "hijacking" the toolchain itself. There are precedents for a proxy mechanism of a toolchain manager on top of the toolchain itself, such as rustup.

cmcgee1024 commented 1 month ago

I'd be interested to hear the core team's view on intercepting any calls to swift. Node does a similar thing with nvm IIRC so there's precedent, not that that's the best example of an ecosystem if there are any security concerns 😅

Do you know a way to engage the core team on this? I'm also curious if there are any concerns or suggestions from there. The proxy idea has been mentioned in the forums a couple of times now, the last one points to this PR. I have received some feedback, but I don't know if any have been representative of the core.

I would say that the + syntax is not overly discoverable or intuitive, I'm not aware of any other tool that has this. I'd be more inclined to have an actual flag that can either be intercepted or called via swiftly run etc.

Thank you, this is a fair observation.

The proxy concept along with the + syntax is inspired in part by rustup: => https://rust-lang.github.io/rustup/concepts/proxies.html

In their documentation there is only a small mention of the special selector syntax: => https://rust-lang.github.io/rustup/overrides.html#toolchain-override-shorthand

I think that this syntax can be discoverable in swiftly by providing (some) examples in the getting started guide, demonstrating how to use swiftly. In the swiftly steady state I imagine that the syntax won't be used very often as the toolchain will likely be selected with either the in-use global default for the user, or hopefully, the project's .swift-version file that aligns the project participants. In these cases swiftly works just like using the toolchain directly, with the exception of times when the selected toolchain isn't installed where it prompts the user to install it. This is what's interesting about the proxy concept.

I can see the + syntax being particularly useful when someone wants to run a check against the latest snapshot, or a previous stable build.

Overall however I agree that having a mechanism for being able to run commands in Swift with different versions without having to constantly switch your local version is valuable and should be implemented

I'm wondering, since this has come up a few times, if it might be worth providing both proxy capability and #22 at the same time in this PR to suit different preferences and use cases.

0xTim commented 1 month ago

Do you know a way to engage the core team on this?

Maybe @shahmishal can bring it up but this should probably be raised at the Platform Steering Group until whatever group will end up with ownership of this takes over them

airspeedswift commented 1 month ago

this should probably be raised at the Platform Steering Group

I think this is right. There are plans to add a third steering group in future (dubbed "Ecosystem") and this might fall under that rather than Platform, but I could see it going either way in this case so starting with the platform steering group is the right way to go for now.

The first step is to write up a pitch and post it to the forums. That pitch should cover the pros/cons of the approach, and recommend one (with the others being alternatives considered). Then engagement on the merits can happen in that thread. That said, it's fine to hash out early ideas on a doc in this repo beforehand.

Once the pitch has had some time to converge on a way forward, the platform steering group would take it up as a proposal. You don't need the steering group to pre-clear that pitch, though you might want to give them a heads up about it, so they can engage in it too since they'll ultimately be the ones deciding which way to go with this.

Note that since Swiftly was ingested into swiftlang and so is part of the Swift project now, I don't see anything contentious about close integration between the swift launch tool and swiftly per se. But of course, the other tradeoffs cited in this thread about exactly how that integration actually works should be discussed.

cmcgee1024 commented 1 month ago

Thanks everyone for the discussion here. This has been really positive. I will change the shape of this PR in a couple of ways and see if this can be accomplished in the near term:

Retain proxy concept for further refinement in the future.
Implement #22 in here to support a swiftly specific syntax for on-the-fly toolchain selection and also the extra capabilities that feature can provide.
Defer the special toolchain selector syntax (ie. the injection of the '+' into tool invocations) for later after a pitch has been made on the forums and the platform steering group.

Please let me know if there are any concerns over this latest pivot.

cmcgee1024 commented 3 weeks ago

@swift-ci test macOS

rauhul commented 3 weeks ago

Out of curiosity, do you have an idea of how this would interact with xcrun?

cmcgee1024 commented 3 weeks ago

Out of curiosity, do you have an idea of how this would interact with xcrun?

Swiftly's mechanisms for toolchain selection are entirely separate from xcrun. But, it will install toolchain packages in the user's volume, and those should be reachable with xcrun.

cmcgee1024 commented 3 weeks ago

@swift-ci test macOS