Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable host toolchain on macOS by default, improve README.md #196

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

xtremekforever
Copy link
Contributor

I put this against #189 since it adds some documentation to help with common issues people run into. But primarily, this PR sets --host-toolchain on macOS by default, since most people are using Xcode and not the Swift OSS toolchain to run the generator and build using generated Swift SDKs.

 - It can still be disabled, but most people would want to include it when just building a Swift SDK with minimal params.
Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. I'm not fond of requiring --target with a full triple name these days, as it's mostly CPU arch that users care about, and other triple components can (and should be) inferred. But that's something to do in the future, not blocking for this PR.

@MaxDesiatov
Copy link
Contributor

@swift-ci test

@xtremekforever
Copy link
Contributor Author

Also, thought I would share. I was talking to the author of SwiftToolkit.dev yesterday and he ran into the same issue as others had with using Swift SDKs generated on macOS without the host toolchain. I explained to him that --host-toolchain was needed if he was not using the Swift OSS toolchain. This is what inspired this change, since most people are running the Xcode Swift toolchain.

However, he did write a nice post about running Swift on the Raspberry Pi, which is well supported now for aarch64 PIs:

https://swifttoolkit.dev/posts/r-pi

@xtremekforever
Copy link
Contributor Author

Makes sense to me. I'm not fond of requiring --target with a full triple name these days, as it's mostly CPU arch that users care about, and other triple components can (and should be) inferred. But that's something to do in the future, not blocking for this PR.

Yeah, especially since we only ever generate Swift SDKs to cross-compile to Linux it doesn't seem useful to have to include the whole triple all the time. So something like this could be awesome in the future:

--target x86_64
--target aarch64
--target armv7

Or, even let it take alternate names too:

--target amd64
--target arm64
--target arm    # could default to armv7 since this is the latest arm "32-bit" supported

@MaxDesiatov
Copy link
Contributor

We previously had this called --arch IIRC, not sure when exactly it morphed into a full triple option named --target. @kateinoigakukun maybe it was WASI related?

@xtremekforever
Copy link
Contributor Author

We previously had this called --arch IIRC, not sure when exactly it morphed into a full triple option named --target. @kateinoigakukun maybe it was WASI related?

Was it --target-arch?

  --target-arch <target-arch>
                          Deprecated. Use `--target` instead (values: arm, armeb, aarch64, aarch64e, aarch64_be,
                          aarch64_32, arc, avr, bpfel, bpfeb, hexagon, m68k, mips, mipsel, mips64, mips64el, msp430, ppc,
                          ppc64, ppc64le, r600, amdgcn, riscv32, riscv64, sparc, sparcv9, sparcel, systemz, tce, tcele,
                          thumb, thumbeb, i386, x86_64, xcore, nvptx, nvptx64, le32, le64, amdil, amdil64, hsail,
                          hsail64, spir, spir64, kalimba, shave, lanai, wasm32, wasm64, renderscript32, renderscript64,
                          xtensa)

@kateinoigakukun
Copy link
Member

It makes sense to me to alias known arch names to recipe-specific defaults. I made the option to accept a full triple to be able to handle "environment" part difference but I understand it's not always necessary.

@xtremekforever
Copy link
Contributor Author

It makes sense to me to alias known arch names to recipe-specific defaults. I made the option to accept a full triple to be able to handle "environment" part difference but I understand it's not always necessary.

So, I wonder if maybe using --target-arch would not be deprecated but instead an alternative for being able to pass just the arch, so you could tell it aarch64 or x86_64. The problem is that it'll have to translate them to the *-unknown-linux-gnu but maybe it's doable. What do you think @MaxDesiatov ?

Then if you need something that's not a Linux platform, just use the whole triple argument instead...

@MaxDesiatov MaxDesiatov requested a review from euanh March 9, 2025 12:56
@MaxDesiatov
Copy link
Contributor

The problem is that it'll have to translate them to the *-unknown-linux-gnu but maybe it's doable.

Surely doable, ideally accounting for possible future Musl support (hence -gnu is not the only available environment/ABI). Not required for this PR of course.

@MaxDesiatov
Copy link
Contributor

@swift-ci test

@MaxDesiatov
Copy link
Contributor

@swift-ci test

@xtremekforever
Copy link
Contributor Author

xtremekforever commented Mar 11, 2025

Right now if I give the swift-sdk-generator the --target-arch param it'll tell me to use --target instead:

2025-03-11T14:20:19-0400 warning org.swift.swift-sdk-generator : [GeneratorCLI] deprecated: Please use `--target aarch64-unknown-linux-gnu` instead of `--target-arch aarch64`

And the end result is a Swift SDK for the wrong arch and not what I wanted:

After that, use the newly installed SDK when building with this command:
swift build --experimental-swift-sdk 6.0.3-RELEASE_ubuntu_jammy_x86_64

So my question is- should we revive the use of the --target-arch param as an alternative to the use of --target, and of course it'll just default to *-linux-gnu as it does now?

    func deriveTargetTriple(hostTriple: Triple) -> Triple {
      if let target = generatorOptions.target {
        return target
      }
      if let arch = generatorOptions.targetArch {
        let target = Triple(arch: arch, vendor: nil, os: .linux, environment: .gnu)
        appLogger.warning(
          "deprecated: Please use `--target \(target.triple)` instead of `--target-arch \(arch)`")
      }
      return Triple(arch: hostTriple.arch!, vendor: nil, os: .linux, environment: .gnu)
    }

I can also update the README.md to recommend using --target-arch by default, but also showing that the full triple can be supplied with --target if needed for non linux-gnu (in other words, not the default) environments.

@MaxDesiatov MaxDesiatov changed the title Enable host toolchain inclusion on macOS by default, add documentation to README.md Enable host toolchain on macOS by default, improve README.md Mar 12, 2025
@MaxDesiatov MaxDesiatov added the documentation Improvements or additions to documentation label Mar 12, 2025
@MaxDesiatov
Copy link
Contributor

@swift-ci test

1 similar comment
@MaxDesiatov
Copy link
Contributor

@swift-ci test

@MaxDesiatov
Copy link
Contributor

@swift-ci test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants