Closed StevenSorial closed 1 year ago
@MattKiazyk This PR is only for listing and not for downloading/installing. Its mostly finished. Do you think it's OK to review this PR on its own or should we include installing in the PR?
@MattKiazyk Thanks for the review. I resolved the comments. Is there any reason async/await can't be used in the project? Concurrency should be supported starting from macOS 10.15 I used it in the tests but would like to use it in the executable itself instead of PromiseKit
Is there any reason async/await can't be used in the project? Concurrency should be supported starting from macOS 10.15 I used it in the tests but would like to use it in the executable itself instead of PromiseKit
No reason why. This project was started long before Swift Concurrency and Combine. PromiseKit was used in most projects that the contributors worked on so it was the simplest for us.
I think as long as files are well organized, it can be added. My goal was to switch all of it at one point, but the time just hasn't come.
I think as long as files are well organized, it can be added. My goal was to switch all of it at one point, but the time just hasn't come.
I made the new command async
as a start. I think it's better to migrate the project gradually using bridging method between PromiseKit and async/await
@StevenMagdy Can you mark as ready for review when you're ready for another look?
There are a few open questions left that need to be answered. I will demonstrate them with a use case for each one
Installed runtime = A runtime that exists in
/Library/Developer/CoreSimulator/Profiles/Runtimes
or/Library/Developer/CoreSimulator/Volumes/
Bundled runtime = A runtime that exists in
Xcode-X.X.X.app/
Should xcodes runtimes
list iOS 15.5? While Xcode 13.3 canβt create a simulator with 15.5 runtime, a 15.5 runtime is still prefectly installable, discoverable by xcrun simctl runtime list
, and can be used if the user later installs Xcode >=14.0.
If we choose to not list it, what should happen if it was already installed? In that case the user would not see iOS 15.5 (installed)
as part of the list and might think that the runtime was deleted.
This use case applies to runtimes that is newer than the installed Xcode and canβt be used by it. The PR currently list them.
In that case there are two exact copies of the iOS 15.5 runtime, one is bundled and the other is installed.
How should we present that info in the list? This PR chooses two duplicate the 15.5 line, one is iOS 15.5 (Bundled)
and the other is iOS 15.5 (installed)
. Another possiblilty is iOS 15.5 (Bundled, Installed)
But I think the user might think its the exact runtime.
In that case xcrun simctl runtime list
will only report the bundled runtimes of Xcode 14.0 because its the selected Xcode, so, xcodes runtimes
will have iOS 16.0 (Bundled)
and iOS 15.5
with no (Bundled)
. We can avoid this by scanning every Xcode-X.X.X.app/
for its bundled runtimes but Iβm not sure its worth it just for the (Bundled)
indicator.
@MattKiazyk @rogerluan what do you think?
Hi @StevenMagdy π
I think we should list them. Just like users are capable of downloading and installing an Xcode version that their current macOS version doesn't allow running it, the user can first download & install and only upgrade their macOS version later on π So current behavior is correct IMO.
I think they should display in separate lines, I think this makes it clear here that they are located in different places, although it may or may not be clear that they are the exact same copies just installed from different sources. No strong opinion here, though, if @MattKiazyk or yourself thinks it should be displayed in one line, that's fine too.
This is a trickier one. In your example, iOS 15.5
is listed as installed even though it's bundled in Xcode 13.5, and not just installed as a runtime? Ideally I'd prefer to display to which version of Xcode each iOS version is bundled to if the user has more than 1 Xcode version installed. Not sure how much longer the user would have to wait to see this info (e.g. how much delay it will increase in the command execution), but I think it's the optimal UX and should be worth the time implementing. If it would add too much time to the command execution, I'd probably add an option to disable this so it runs faster (for advanced users).
With that being said, however, I don't think it's too much of a deal breaker to keep the current behavior. What I'm suggesting would be my expectation as a user, but if it'd be too much of a challenge to introduce this functionality, it can be ticketed as a nice-to-have
Looking forward to hearing @MattKiazyk's thoughts as well π€ Great write up @StevenMagdy! πͺ
Hi @rogerluan, Thanks for your answer.
Agreed on use case 1 and 2.
In your example, iOS 15.5 is listed as installed even though it's bundled in Xcode 13.5, and not just installed as a runtime?
No, sorry if it wasn't clear. In this example, no runtime is installed (i.e. nothing exists in /Library/Developer/CoreSimulator/Profiles/Runtimes
or /Library/Developer/CoreSimulator/Volumes/
). The only iOS runtimes are 16.0 and 15.5 bundled in Xcode 14.0 and 13.5, respectively.
Currently, if Xcode 14.0 is selected, the list would have iOS 16.0 (Bundled)
and iOS 15.5
, and vice versa if Xcode 13.5 is selected.
if it'd be too much of a challenge to introduce this functionality, it can be ticketed as a nice-to-have
My concern would be that we would be deviating from Apple's official installed/bundled runtimes list reported by xcrun simctl runtime list
. Also, it might be more confusing, if Xcode 14.0 is selected then the runtime bundled in Xcode 13.5 is unusable because Xcode 14.0 can't create a simulator using that runtime, i.e. the user can't run simctl create mysim iPhone8 iphoneos15.5
until they set Xcode 13.5 as selected.
Edit: I forgot to mention that this PR chooses to mitigate this UX issue by appending (Bundled with selected Xcode)
instead of (Bundled)
to clarify that there might be other bundled runtimes in other Xcodes.
I wouldn't be too much concerned with the UX issue you're pointing out if xcodes properly listed the xcode version it's bundled in π€· to me, that'd make sense. For instance:
iOS 15.5 (Bundled with Xcode 13.5)
iOS 16 (Bundled with Xcode 14, currently selected)
It'd then implicitly indicate "hey, if you wanna use 15.5, you must first select Xcode 14" (sorta). If someone attempts to use it without selecting Xcode 13.5 first, then it'd throw an error but I think users could figure out what the issue is on their own and that they have to select that Xcode version first...
Another alternative (less preferred IMO because it tries to outsmart users) would be to automatically select the Xcode version needed to run that runtime. I'm not a fan of this, but it could be an option maybe?
We should still allow to install it. Like @rogerluan indicated, we allow to install Xcode if they don't have the min OS. Xcodes.app does a better job of this to check and ask before installing.
Yeah separate lines. If you delete the bundled one, the previous one is there (and taking up precious bits on the harddrive)
I like (Bundled with selected Xcode)
option. We can add another log item (at the bottom) to show more information.
@MattKiazyk I added a note at the end as you suggested. ππΌ
iOS 15.5 (Bundled with Xcode 13.5) iOS 16 (Bundled with Xcode 14, currently selected)
@rogerluan That would be fine too and it solves the confusion issue. I'm ok with either option.
I still have a slight preference for the implementation of this:
iOS 15.5 (Bundled with Xcode 13.5) iOS 16 (Bundled with Xcode 14, currently selected)
It's more clear to the user than just the footnote that was added. But that may be just me π€·ββοΈ @MattKiazyk WDYT?
@MattKiazyk CI was outdated. Could you please run it again?
This PR adds a new command
xcodes runtimes
, which is intended to be a replacement forxcversion simulators
from xcode-install. I chose the word "runtimes" to avoid confusion with the actual simulators (ie. iPhone Xs), which is also how apple refer to it inxcrun simctl runtime
.xcversion simulators
used a list specific to each installed Xcode, this is now deprecated and now Apple returns all possible runtimes, which makes filtering our responsibilty.