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

Generate SDKSettings.json file for Swift SDKs for Swift 5.9-6.0 #185

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

Conversation

xtremekforever
Copy link
Contributor

I know that the warnings about the SDKSettings.json file were fixed in Swift 6.1 and later and that issue #110 was closed. However, we are still generating Swift SDKs for 5.9, 5.10, and 6.0 which all give the warnings.

So, my suggestion is to generate this SDKSettings.json file for versions older than 6.1, similar to how the the Linux Static Swift SDKs do it. Here is the file that comes in the swift-6.0.3-RELEASE_static-linux-0.0.1 Swift SDK for example:

{
  "DisplayName": "Swift SDK for Statically Linked Linux (x86_64)",
  "Version": "0.0.1",
  "VersionMap": {},
  "CanonicalName": "x86_64-swift-linux-musl"
}

Taking inspiration from this, I am generating a similar file for Swift SDKs created by the generator:

{
  "CanonicalName" : "x86_64-swift-linux-gnu",
  "DisplayName" : "Swift SDK for Ubuntu Jammy (x86_64)",
  "Version" : "0.0.1",
  "VersionMap" : {

  }
}

The ordering is not the same due to how JSONEncoder() sorts things, and the formatting is a little different too due to .prettyPrinted, but it works just the same. And, the "warning: Could not read SDKSettings.json for SDK at: " warning is successfully suppressed.

This really is a minor change that is more "quality-of-life" then adding any functionality. But, not having those warnings printed when using the generated Swift SDKs are used is a nice to have.

@MaxDesiatov @euanh @kateinoigakukun

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.

Thanks! Mind adding a test for this?

 - Cover a few different arch names and ensure the correct file output is included.
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.

Overall makes sense, just a function naming nit to pick.

@MaxDesiatov MaxDesiatov requested a review from euanh February 16, 2025 01:44
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants