Prevent keyNotFound error with unknown simulators #261

dnicolson · 2023-01-04T07:27:57Z

This pull request prevents the following error when running xcodes runtimes when an "Unknown Platform Simulator" is installed:

Error: keyNotFound(CodingKeys(stringValue: "build", intValue: nil), Swift.DecodingError.Context(codingPath: [_JSONKey(stringValue: "820783CB-C389-4006-B2D2-D063F3FD10A1", intValue: nil)], debugDescription: "No value associated with key CodingKeys(stringValue: \"build\", intValue: nil) (\"build\").", underlyingError: nil))

If one accidentally runs xcrun simctl runtime add with an incompatible pre-iOS 16 Simulator runtime:

xcrun simctl runtime add com.apple.pkg.iPhoneSimulatorSDK15_0-15.0.1.1633542405.dmg

The following error will be displayed:

D: 94DEF0B1-1585-4D71-AB37-98AB08FDD000 <unknown platform> (Unusable - Missing Signature: Error Domain=SimDiskImageErrorDomain Code=3 "Missing Signature" UserInfo={NSLocalizedDescription=Missing Signature, unusableErrorDetail=})

The problem is that it's not immediately obvious that Xcode has still installed the runtime, but with a missing signature state:

And when xcodes runs xcrun simctl runtime list -j, there is no build key resulting in the error:

"820783CB-C389-4006-B2D2-D063F3FD10A1" : {
  "deletable" : true,
  "identifier" : "820783CB-C389-4006-B2D2-D063F3FD10A1",
  "kind" : "Disk Image",
  "path" : "\/Library\/Developer\/CoreSimulator\/Images\/Inbox\/820783CB-C389-4006-B2D2-D063F3FD10A1.dmg",
  "signatureState" : "Missing",
  "sizeBytes" : 5304795932,
  "state" : "Unusable",
  "unusableErrorDetail" : "Error Domain=SimDiskImageErrorDomain Code=3 \"Missing Signature\" UserInfo={NSLocalizedDescription=Missing Signature, unusableErrorDetail=}",
  "unusableErrorMessage" : "Missing Signature",
  "unusableSubstate" : "Missing Signature"
}

This change results in the following xcodes runtimes output:

-- Unknown --
Unknown 0 (Downloaded)

Another approach could be to filter out undecodable runtimes in the RuntimeList class, this actually resulted in more code though.

StevenSorial · 2023-01-06T05:24:22Z

Nice catch, thank you :)

Another approach could be to filter out undecodable runtimes in the RuntimeList class, this actually resulted in more code though.

I prefer that approach. it doesn't have to be a lot of code, actually, it's less code.

Remove all the installed runtimes with the state unusable, before looping over them, then you can then force-unwrap build, version, and platformIdentifier (and remove the unknown enum case).

I would also add a note at the end with how many unknown runtimes were found, with a suggestion to run xcrun simctl runtime list -j for more info.

Disclaimer: I contributed the runtimes feature but I'm not part of the team, so you might want to wait for their opinion/review.

dnicolson · 2023-01-06T07:08:27Z

The amount of unknown runtimes is indicated by how many "Unknown" runtimes are listed, it should be comparable to what is in Xcode. It's not ideal that there is a 0 though, but further hacks wouldn't have been nice.

The original approach seemed to require using a custom decoder and possibly filtering out the items unable to be decoded afterward. Or do you mean keeping the optional keys so that the decoding doesn't fail?

StevenSorial · 2023-01-06T07:17:04Z

Or do you mean keeping the optional keys so that the decoding doesn't fail?

yeah, what i meant is removing the

-- Unknown --
Unknown 0 (Downloaded)

and replacing it at the end with (if more than 0):

Note: Found 1 unknown downloaded runtime(s). For more info run `xcrun simctl runtime list -j`.

you just have to add let unusableCount = installed.removeAll { $0.state == "Unusable" }.count before looping over the installed array, and then you can force-unwrap the newly optional values.

dnicolson · 2023-01-06T07:41:00Z

I've made those changes, thanks for the input!

MattKiazyk

Thanks for adding this @dnicolson 🎉

Can we remove the force unwraps? As well as add a quick test for these? Let me know if you need some help.

MattKiazyk · 2023-01-06T15:00:29Z

Sources/XcodesKit/RuntimeInstaller.swift

        installed.forEach { runtime in
            let resolvedBetaNumber = downloadablesResponse.sdkToSeedMappings.first {
-                $0.buildUpdate == runtime.build
+                $0.buildUpdate == runtime.build!


Can we refactor this section a bit?

Can we not use force unwrap. As a rule, I don't use them. Totally agree that it should never happen but it's bitten me in the past. Perhaps making an overloaded PrintableRuntime() to allow optionals?

I'm not sure how best to achieve this, feel free to make any changes.

Just my 2 cents here, personally I also try to avoid them if I can, but in this case, I would prefer a force unwrap to fail than hiding the error and providing an empty string as a default value, in that case, we would know that xcrun simctl runtime list printed something that we didn't account for, and there is probably a new state of a runtime that we should write code for. I see it as a assert/precondition and not a code smell.

@MattKiazyk can you please give an update on how you would like to proceed here?

MattKiazyk · 2023-01-06T15:00:54Z

Sources/XcodesKit/RuntimeInstaller.swift

@@ -90,6 +88,9 @@ public class RuntimeInstaller {
            }
        }
        Current.logging.log("\nNote: Bundled runtimes are indicated for the currently selected Xcode, more bundled runtimes may exist in other Xcode(s)")
+        if unusableCount > 0 {


I like this vs having the original Unknown 👍

MattKiazyk · 2023-01-06T15:01:57Z

Sources/XcodesKit/Models+Runtimes.swift

@@ -108,18 +108,18 @@ extension DownloadableRuntime {
 }

 public struct InstalledRuntime: Decodable {
-    let build: String
+    let build: String?


#picky can we add a comment on why these are optional

Bobbyphtr · 2024-07-24T06:33:57Z

Hi, is there any updates for this PR?

dnicolson · 2024-07-28T21:29:38Z

@MattKiazyk can you please give an update?

Prevent keyNotFound error with unknown simulators

7aa5a38

dnicolson requested a review from a team as a code owner January 4, 2023 07:27

Remove unusable runtimes from output

9fb309c

MattKiazyk requested changes Jan 6, 2023

View reviewed changes

dnicolson added 2 commits January 6, 2023 21:07

Add comments for unusable runtime attributes

01be5cf

Add test for unusable runtimes

dbb6855

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent keyNotFound error with unknown simulators #261

Prevent keyNotFound error with unknown simulators #261

dnicolson commented Jan 4, 2023

StevenSorial commented Jan 6, 2023

dnicolson commented Jan 6, 2023

StevenSorial commented Jan 6, 2023

dnicolson commented Jan 6, 2023

MattKiazyk left a comment

MattKiazyk Jan 6, 2023

dnicolson Jan 6, 2023

StevenSorial Jan 8, 2023

dnicolson Apr 2, 2023

MattKiazyk Jan 6, 2023

MattKiazyk Jan 6, 2023

Bobbyphtr commented Jul 24, 2024

dnicolson commented Jul 28, 2024

Prevent keyNotFound error with unknown simulators #261

Are you sure you want to change the base?

Prevent keyNotFound error with unknown simulators #261

Conversation

dnicolson commented Jan 4, 2023

StevenSorial commented Jan 6, 2023

dnicolson commented Jan 6, 2023

StevenSorial commented Jan 6, 2023

dnicolson commented Jan 6, 2023

MattKiazyk left a comment

Choose a reason for hiding this comment

MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

dnicolson Jan 6, 2023

Choose a reason for hiding this comment

StevenSorial Jan 8, 2023

Choose a reason for hiding this comment

dnicolson Apr 2, 2023

Choose a reason for hiding this comment

MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

MattKiazyk Jan 6, 2023

Choose a reason for hiding this comment

Bobbyphtr commented Jul 24, 2024

dnicolson commented Jul 28, 2024