Add Swift plugin #1708
Add Swift plugin #1708
Conversation
* only call it if the plugin was actually updated * set the ASDF_PLUGIN_PREV_REF and ASDF_PLUGIN_POST_REF env vars
* activate: added --shims * runtime_symlinks: do not fail if version parsing fails
hopefully this is more portable Fixes jdx#1485
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* fix(deps): update rust crate regex to 1.10.3 * [MegaLinter] Apply linters fixes --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <renovate[bot]@users.noreply.github.com>
* cargo: allow cargo-binstall from mise itself Fixes jdx#1506 I have not tested this myself but it should fix the problem. * Commit from GitHub Actions (test) --------- Co-authored-by: mise[bot] <[email protected]>
| .arg(&bin_directory) | ||
| .execute()?; | ||
| } else { | ||
| file::unzip(download_path, &ctx.tv.download_path())?; |
There was a problem hiding this comment.
@jdx I noticed the tars contain other artifacts beyond the binary. I assume we have to respect the directory structure here. Should I untar everything into bin_directory and use make_executable?
There was a problem hiding this comment.
in general in mise we always just leave all the files that tools put into their tarballs and don't prune anything unless we have a good reason to prune files we explicitly don't want.
In regards to make_executable—yeah you'll likely need that since zip files don't have permissions. I believe we do this for bun which also uses zip.
| } | ||
|
|
||
| fn os() -> String { | ||
| // ASK: Is it ok if unlike other plugins, we return a String here instead of a static str? |
There was a problem hiding this comment.
Other modules return a &str with a static lifetime. However, since I need to get the Linux distribution at runtime, I needed to change the return type to String. Do you think it's fine?
There was a problem hiding this comment.
yeah exactly, you'll need to export full strings. rust makes you do a bit more work around that since it technically is less efficient (the idea in rust is that anything that has a performance cost you will see in your code), but clearly for what we're doing in this file that's a completely irrelevant cost.
In fact there is probably not a single place in our entire codebase where string instantiation is a cost worth considering. I use &'static str purely just to stay in the habit in case code I write ever does benefit from that consideration but it's probably not something in this project is ever (or at least very rarely) important.
| file::create_dir_all(&bin_directory)?; | ||
|
|
||
| if os() == "osx" { | ||
| CmdLineRunner::new("/usr/sbin/installer") |
There was a problem hiding this comment.
When using installer, the two values that -target can take are CurrentUserHomeDirectory or LocalSystem. In the case of the former, the Swift toolchain ends up being installed at ~/Library/Developer/Toolchains/swift-5.9.2-RELEASE.xctoolchain/. In the case of the later, the directory is /Library/Developer/Toolchains.
@jdx what do you think if I use the former to prevent having to use sudo, and then copy the *.xctoolchain into the Mise directory?
There was a problem hiding this comment.
using sudo is kind of a dealbreaker for anything in mise since doing that screws up permissions—so yeah I think what you said about CurrentUserHomeDirectory is better, then we move the xctoolchain into place sounds like the right approach
There was a problem hiding this comment.
Thanks! I'll do that then.
| fn install(&self, ctx: &InstallContext, download_path: &Path) -> Result<()> { | ||
| let filename = download_path.file_name().unwrap().to_string_lossy(); | ||
| ctx.pr.set_message(format!("installing {filename}")); | ||
| file::remove_all(ctx.tv.install_path())?; |
There was a problem hiding this comment.
I'm not 100% sure if this is necessary. I think mise actually does this automatically. Not a big deal though, I'm happy to leave it in just to be safe.
|
@pepicrft really sorry it took me so long to get around to this. It looks excellent though. I had some feedback that really is just nitpicks pretty much. The one thing we should get is an e2e test though, https://github.com/jdx/mise/blob/main/e2e/test_erlang is a good example of one you can use as a reference. You can either just copy If you have any trouble with the e2e test I'd be happy to add that to your PR too. |
jdx
force-pushed
the
main
branch
2 times, most recently
from
fad123b
to
d13511e
Compare
|
No worries @jdx. I appreciate your review of the PR. I have one question. Xcode installations come with a Swift toolchain embedded in the application bundle, which is the one used by default when that version of Xcode is selected via If you agree, how do you think I should approach it from the |
|
Having recently gone through installing swift on linux, there's a couple of issues on different distributions it seems (primarily due to linking to native libraries). Have you been able to test this on multiple distributions? For example, on arch the most popular package to install swift uses patchelf because the source swift distribution uses incorrect paths (at least for arch). Using https://github.com/swift-server/swiftly as an alternative, has similar issues where just running I'm not very familiar with swift itself or the best way to resolve this across different distributions, but it would be great if it could accommodate them. I know there are options when building swift to statically link its dependency libraries too, which might be another avenue to go down? |
|
Thanks @johnpyp! I haven't tested it yet in Linux distributions, but I'll make sure I do before considering the PR ready. What other contributions come to your mind? |
|
For Linux support, the Ubuntu 24.04 builds of Swift 5.10.1 work perfectly under Arch Linux without modification. The Ubuntu 24.04 builds don't seem to exist for all versions yet, but that might be a good option to use if they're available. |
I'm implementing a core plugin to manage Swift versions. As a follow-up work, I'll implement a backend to install community packages representing CLIs.
The current Swift asdf-plugin turns out to be using swiftenv under the hood installing it if absent in the environment using Homebrew (too much indirection).