feat: adds `xcodes` step #643

schrobingus · 2024-01-17T20:53:13Z

Standards checklist:

The PR title is descriptive.
I have read CONTRIBUTING.md
The code compiles (cargo build)
The code passes rustfmt (cargo fmt)
The code passes clippy (cargo clippy)
The code passes tests (cargo test)
Optional: I have tested the code myself

This pull request is respondent to #630, by adding support for the tool xcodes, which manages Xcode versions. The step upgrades the tool directly in this order:

The releases to upgrade will be filtered based on if the releases installed are regular, beta, or golden master.
With those conditions, if a new release is detected, it'll prompt if the user would like to install, given that prompts are enabled.
It'll follow through and install the new release.
If there are only two releases installed, being the new release and old release, it'll prompt if the user would like to move the old release to the trash. This is ignored if prompts are disabled.

For new steps

Optional: Topgrade skips this step where needed
Optional: The --dry-run option works with this step
Optional: The --yes option works with this step ~~if it is supported by the underlying command~~, the command might require an Apple ID 2FA verification to install the new release regardless

schrobingus · 2024-01-17T20:58:01Z

Huh, I'll check Rustfmt again.

SteveLauC · 2024-01-18T01:55:37Z

I am sorry @DottoDev that I accidentially hit that request review button:D, and sorry about this ping...

src/steps/os/macos.rs

SteveLauC · 2024-01-18T02:40:03Z

src/steps/os/macos.rs

+    let releases = ctx.run_type()
+        .execute(&xcodes)
+        .args(["update"])
+        .output_checked_utf8()?.stdout;


Would you like to give me a output of xcodes update so that I can understand the code better?

1.0 (7B85) 1.5 (7K571) 2.2.1 (8G1165) 2.3 (8M1780) 2.4 (8K1079) ... 15.1 Beta 2 (15C5042i) 15.1 Beta 3 (15C5059c) 15.1 (15C65) 15.2 Beta (15C5500c) (Installed) 15.2 (15C500b)

It just updates the list of releases available. As shown above, it also outputs the list at the same time. I can also run this on it's own and run xcodes list separately if it's more visibly sensible, for now I'll put a comment explaining this.

SteveLauC · 2024-01-24T12:05:06Z

src/steps/os/macos.rs

+    if !releases_filtered
+        .last()


It seems that I am missing something? Why are we only doing this to the .last() one?

The last one is the most recent release on the list. If it contains the "(Installed)" substring, then Xcode is fully up to date. On the contrary, it'll then ask if one would like to upgrade to the newest release.

Assume xcodes update gives the following output:

1.0 GM (0) (Installed) 1.1 GM (1) 1.4 Beta (0) (Installed) 1.5 Beta (1) 2.2.1 (0) (Installed) 2.2.2 (1)

allow_gm, allow_beta and allow_regular will all be true, releases_filtered will contain all the listed versions, in this case, if I understand correctly, we should install 1.1 GM (1), 1.5 Beta (1) and 2.2.2 (1) all rather than only the last one?

So update each release? I can do that, will commit that soon :^)

So update each release?

Emm, I am sorry, I thought this was the original implementation plan :<

I can do that, will commit that soon :^)

Thanks for doing that!

schrobingus · 2024-02-03T22:49:05Z

Each release type will now be upgraded if a release of that type is installed. (Sorry for the messy commits, by the way)

SteveLauC · 2024-02-06T08:05:11Z

src/steps/os/macos.rs

+    if releases_filtered
+        .last()
+        .map(|s| !s.contains("(Installed)"))
+        .unwrap_or(true)


If releases_filtered is empty, then .last() will return None, and we treat None as true, then we will run:

$ xcodes install <an empty string>

I believe this won't happen in reality as there are so many Xcode releases, but I believe we should check if releases_filtered is empty or not before actually involving this function, if it is empty, we should directly return.

If I remember correctly, this is already checked in it's calls, but I do agree that it is a good idea to directly return in the scenario that it is empty.

SteveLauC · 2024-02-06T08:07:25Z

src/steps/os/macos.rs

+        .filter(|release| release.contains("(Installed)"))
+        .collect();
+
+    if should_ask && releases_new_installed.len() == 2 {


If we update every kind of Xcode, should we also do this respectively?

SteveLauC · 2024-02-06T08:08:14Z

src/steps/os/macos.rs

+        let _ = process_xcodes_releases(releases_gm, should_ask, ctx);
+    }
+    if installed_beta {
+        let _ = process_xcodes_releases(releases_beta, should_ask, ctx);
+    }
+    if installed_regular {
+        let _ = process_xcodes_releases(releases_regular, should_ask, ctx);


Why are we ignoring the result of process_xcodes_releases, this could hide the possible errors

That makes sense, I'll add an error checker. Should process_xcodes_releases return a boolean indicating success?

Should process_xcodes_releases return a boolean indicating success?

It already returns a Result, which will be Err(error value) on failure, we can simply propagate the error if happens:

Suggested change

let _ = process_xcodes_releases(releases_gm, should_ask, ctx);

}

if installed_beta {

let _ = process_xcodes_releases(releases_beta, should_ask, ctx);

}

if installed_regular {

let _ = process_xcodes_releases(releases_regular, should_ask, ctx);

process_xcodes_releases(releases_gm, should_ask, ctx)?;

}

if installed_beta {

process_xcodes_releases(releases_beta, should_ask, ctx)?;

}

if installed_regular {

process_xcodes_releases(releases_regular, should_ask, ctx)?;

codecov · 2024-02-29T09:04:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 4.02%. Comparing base (650a143) to head (8b12942).

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #643      +/-   ##
========================================
- Coverage   5.57%   4.02%   -1.56%     
========================================
  Files         37      32       -5     
  Lines      11675    5893    -5782     
========================================
- Hits         651     237     -414     
+ Misses     11024    5656    -5368

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SteveLauC · 2024-02-29T09:23:43Z

No need to worry about the codecov CI failures:)

schrobingus · 2024-02-29T22:44:55Z

Applied those changes, but I just realized that I might want to also sort release candidates with the golden masters.

Edit: Done :^)

SteveLauC

Thanks!

schrobingus added 2 commits January 17, 2024 12:22

feat: add Xcodes step

dad1485

fix: Xcodes respondent to no-prompt arguments

62baf7b

SteveLauC requested a review from s34m January 18, 2024 01:54

SteveLauC reviewed Jan 18, 2024

View reviewed changes

schrobingus and others added 2 commits January 22, 2024 11:46

Merge branch 'topgrade-rs:main' into main

5dc3dd5

fix: applied rustfmt to xcodes step

1ad0a8d

SteveLauC reviewed Jan 24, 2024

View reviewed changes

schrobingus and others added 3 commits February 3, 2024 14:17

feat: xcodes step now upgrades each release type

f0baddc

Merge branch 'topgrade-rs:main' into main

0d70540

fix: tests for xcode step

6ac2194

schrobingus force-pushed the main branch from 6ac2194 to 0d70540 Compare February 3, 2024 22:36

fix: tests and formatting for xcode step

ffbc8c4

SteveLauC requested changes Feb 6, 2024

View reviewed changes

Merge branch 'topgrade-rs:main' into main

0127248

fix: make xcodes step iterate through release types, plus error checking

6d21a72

schrobingus marked this pull request as draft February 29, 2024 22:45

fix: apply release candidates to golden master sorting

8b12942

schrobingus marked this pull request as ready for review February 29, 2024 22:55

SteveLauC approved these changes Feb 29, 2024

View reviewed changes

SteveLauC merged commit 6dab1e4 into topgrade-rs:main Feb 29, 2024
14 of 19 checks passed

SteveLauC mentioned this pull request Feb 29, 2024

Support for xcodes #630

Closed

InnocentZero pushed a commit to InnocentZero/topgrade that referenced this pull request May 25, 2024

feat: adds xcodes step (topgrade-rs#643)

51725dc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds `xcodes` step #643

feat: adds `xcodes` step #643

schrobingus commented Jan 17, 2024 •

edited

Loading

schrobingus commented Jan 17, 2024

SteveLauC commented Jan 18, 2024

SteveLauC Jan 18, 2024

schrobingus Jan 22, 2024 •

edited

Loading

SteveLauC Jan 24, 2024

schrobingus Jan 26, 2024

SteveLauC Jan 27, 2024

schrobingus Feb 3, 2024

SteveLauC Feb 6, 2024 •

edited

Loading

schrobingus commented Feb 3, 2024 •

edited

Loading

SteveLauC Feb 6, 2024

schrobingus Feb 29, 2024 •

edited

Loading

SteveLauC Feb 6, 2024

SteveLauC Feb 6, 2024

schrobingus Feb 29, 2024

SteveLauC Feb 29, 2024

codecov bot commented Feb 29, 2024 •

edited

Loading

SteveLauC commented Feb 29, 2024

schrobingus commented Feb 29, 2024 •

edited

Loading

SteveLauC left a comment

feat: adds xcodes step #643

feat: adds xcodes step #643

Conversation

schrobingus commented Jan 17, 2024 • edited Loading

Standards checklist:

For new steps

schrobingus commented Jan 17, 2024

SteveLauC commented Jan 18, 2024

Choose a reason for hiding this comment

schrobingus Jan 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveLauC Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

schrobingus commented Feb 3, 2024 • edited Loading

Choose a reason for hiding this comment

schrobingus Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 29, 2024 • edited Loading

Codecov Report

SteveLauC commented Feb 29, 2024

schrobingus commented Feb 29, 2024 • edited Loading

SteveLauC left a comment

Choose a reason for hiding this comment

feat: adds `xcodes` step #643

feat: adds `xcodes` step #643

schrobingus commented Jan 17, 2024 •

edited

Loading

schrobingus Jan 22, 2024 •

edited

Loading

SteveLauC Feb 6, 2024 •

edited

Loading

schrobingus commented Feb 3, 2024 •

edited

Loading

schrobingus Feb 29, 2024 •

edited

Loading

codecov bot commented Feb 29, 2024 •

edited

Loading

schrobingus commented Feb 29, 2024 •

edited

Loading