[action] add `xcodes` action, deprecating `xcversion` and `xcode-install` #20672

rogerluan · 2022-09-18T23:50:20Z

Checklist

I've run bundle exec rspec from the root directory to see all new and existing tests pass
I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
I've read the Contribution Guidelines
I've updated the documentation if necessary.

Motivation and Context

With the planned sunset of xcode-install (aka xcversion) gem (xcpretty/xcode-install#467), a new action was needed to support xcodes.

Description

This PR depends on XcodesOrg/xcodes#220 and XcodesOrg/xcodes#182 merge & release so the installed option with specified version is available, as well as --install and --select arguments for the install option.

These PRs were later merged & released under v1.1.0 https://github.com/RobotsAndPencils/xcodes/releases/tag/1.1.0, so v1.1.0 of xcodes is required to use and test this feature.

Testing Steps

To test this branch, modify your Gemfile as:

gem 'fastlane', git: 'https://github.com/fastlane/fastlane.git', branch: 'rogerluan-xcodes'

And run bundle install to apply the changes.

Then switch your implementation of xcversion and xcode_install according to the migration guide which can be found here: https://github.com/xcpretty/xcode-install/blob/master/MIGRATION.md

Any and every testing is highly appreciated! I did very limited testing only 🙇

rogerluan · 2022-09-19T10:41:04Z

The spec failure that is happening in this PR also happens in master, and it may be something related to recent changes on Apple's end 😬

rogerluan · 2022-09-19T20:13:39Z

It's weird:

Locally, on my machine, a lot of specs fail both in this PR as well as in master, same for rubocop violations
In CI, master seems to not be failing
In CI, in another PR of mine, all specs passed
In CI, this PR keeps failing in non-related changes 🤔

rogerluan · 2022-09-20T11:45:53Z

Ugh, so, locally, here's the breakdown:

Ruby 3.1.x: a bunch of linter and specs issues
Ruby 3.x.x: linter and specs are all passing
Ruby 2.7.x: linter and specs are all passing
Ruby 2.6.x: I'm having trouble installing this version using rbenv (M2 machine) 😥 so I couldn't test..

CI is failing specs and it is running Ruby 2.6.0 apparently... but not sure why this is happening in my PR only, and on presumably unrelated files (fastlane_core/spec/cert_checker_spec.rb) something related to:

#installed_wwdr_certificates
      should return installed certificate's alias (FAILED - 4)
      should return an empty array if unknown WWDR certificates are found (FAILED - 5)

😬 Any help would be appreciated

revolter

Hi @rogerluan 👋🏻

Nice to "see" you again after a long time 😃 And sorry for the late reply.

😬 Any help would be appreciated

I can't promise anything, but I hope I'll take a look some time after reviewing this.

I read the migration guide and this PR's description, I started testing it, but it failed with:

[21:00:46]: Couldn't find xcodes binary at path ''
[21:00:46]: You passed invalid parameters to 'xcodes'.
[21:00:46]: Check out the error below and available options by running `fastlane action xcodes`
+------------------+------------------+
|            Lane Context             |
+------------------+------------------+
| DEFAULT_PLATFORM | ios              |
| PLATFORM_NAME    | ios              |
| LANE_NAME        | ios ensure_xcode |
+------------------+------------------+
[21:00:46]: Invalid default value for binary_path, doesn't match verify_block

Although it is fairly obvious that the issue is that it can't find xcodes, and you have to (most probably) run brew install robotsandpencils/made/xcodes, I think that this should be mentioned in the migration guide and/or the new action's description, since it's a required dependency that's not automatically managed by fastlane. What do you think?

fastlane/lib/fastlane/actions/xcodes.rb

fastlane/spec/helper/xcodes_helper_spec.rb

rogerluan · 2022-10-14T10:55:54Z

Oops, forgot to address the "no binary" issue. Gimme a few mins

rogerluan · 2022-10-14T11:15:44Z

Done @revolter 😊 Nice catch!

Also: xcpretty/xcode-install@43328e9

rogerluan · 2022-10-29T19:57:00Z

@revolter would you mind helping me understand what's failing in the CI specs? I added comments about it here and I'm still clueless on how to solve this 😿

revolter

Just some small changes, to be less confusing for future contributors 🤔

fastlane/lib/fastlane/actions/xcodes.rb

fastlane/spec/actions_specs/xcodes_spec.rb

revolter · 2022-11-01T20:58:53Z

@rogerluan, would you mind if I'd remove the merge commit, and rebase the branch instead?

rogerluan · 2022-11-01T22:40:23Z

@rogerluan, would you mind if I'd remove the merge commit, and rebase the branch instead?

By all means @revolter 🙇
Would that help debug the CI problem?

revolter · 2022-11-02T06:27:51Z

Would that help debug the CI problem?

I hope it fixes it 😂 Because it doesn't make sense

…xcodes' action to replace the 'xcode_install' one.

This makes it consistent with the rest of the codebase.

This was caused by the fact that that test calls a method that uses `$?` (https://stackoverflow.com/a/6834572/865175), which, somehow, (I think) was executed right after a call to XcodesHelper's `find_xcodes_binary_path` method. This latter one returned a non-zero code, because `xcodes` can't be installed on Ubuntu, which got picked up by the certificates return code check, so it treated its actual command as failed.

revolter · 2022-11-02T22:14:42Z

Ok, this took me a lot longer than I thought it would 😐 Very very weird issue, but great debugging session nonetheless ❤️

I also noticed that some paths were being concatenated using +, so I used the same method as in xcode_select.rb (note: it works with an already present slash because that method makes sure not to duplicate the separator: https://stackoverflow.com/a/4114966/865175).

There are still some failures, but these ones might not be that hard to fix 🤔

(Also, because I rebased and force-pushed to the branch, guessing that you didn't commit anything after 136b664, please delete the local branch, fetch the new changes, and check it out again, to avoid other superfluous merges.)

I really need to go to sleep now, as it's already tomorrow 😫

revolter · 2022-11-03T07:18:41Z

Please let me work on this more? 😂 I have an idea that I want to test this evening.

(reverted from commit 11bd750)

revolter · 2022-11-03T17:20:28Z

Ok, I was not happy with the fix, because I didn't understand it fully (still don't), and felt more like a hack. From what I (extensively) read, even through the source code, and by also testing it locally by replacing the curl command with ls, it looks like this (the last commit) is the safe and correct way to fix this issue for good.

revolter · 2022-11-03T17:32:25Z

Somehow, the 2 failures (Validate Documentation and Validate Fastlane.swift generation) got fixed too 😮

rogerluan

@revolter dude, this was just amazing!! Thank you thank you thank you ❤️ Great context that you provided in the commit messages also! I'd never have guessed something like this, I can't even imagine how was your train of thought and how you got to the root cause 🤯

Separately, do you think we should at least create an Issue for future improvement of getting rid of all the $? in other specs (and maybe in the source code too)?

Again, thank you so much 🙇 I'm so glad we'll be able to deliver this PR to xcode-install users 💪

rogerluan · 2022-11-04T12:29:08Z

fastlane_core/lib/fastlane_core/cert_checker.rb

      if FastlaneCore::Globals.verbose?
        UI.command_output(stdout)
        UI.command_output(stderr)
      end

-      unless $?.success?
+      unless status.success?


👏👏👏

rogerluan · 2022-11-04T12:29:11Z

fastlane/lib/fastlane/helper/xcodes_helper.rb

@@ -12,7 +12,7 @@ def self.read_xcode_version_file
      end

      def self.find_xcodes_binary_path
-        `which xcodes`.strip
+        Actions.sh("which xcodes", log: false).strip


What the... 🤯

In the case of the tests, `$?` was not mocked, so it looks like it was indeed just a coincidence that it worked. Meaning, some other shell command that somehow was actually executed (so it wasn't stubbed) succeeded, which caused the `$?.success?` to succeed too.

revolter · 2022-11-04T18:52:04Z

I can't even imagine how was your train of thought and how you got to the root cause 🤯

Brute-force trial and error, using divide et impera, by reverting half of the changes, then half of the half, etc., until I pinpointed the responsible line of code, and then a lightbulb went off in my head about the $? usage from cert_checker.rb.

Separately, do you think we should at least create an Issue for future improvement of getting rid of all the $? in other specs (and maybe in the source code too)?

Hmm, maybe that would be the way to go, but it's also a case of "if it ain't broken, don't fix it" 🤔 Maybe the best thing is that at least the 2 of us know of this, and hopefully remember it when (if) it happens in the future.

Again, thank you so much 🙇 I'm so glad we'll be able to deliver this PR to xcode-install users 💪

Really happy that I could (finally) help you with this one. Thank you too for the kind words, I'm really flattered ☺️ I'm happy that you took the time to do the other 99% of work for this migration ❤️

joshdholtz

This is soooo amazing!! ❤️ Sorry it took so long but I'm back from family leave and thank for doing this 😊

fastlane-bot · 2022-11-16T19:30:54Z

Congratulations! 🎉 This was released as part of fastlane 2.211.0 🚀

rogerluan added the tool: action label Sep 18, 2022

rogerluan mentioned this pull request Sep 19, 2022

[Discussion] Is it time to sunset this project? 🌆 xcpretty/xcode-install#467

Open

rogerluan marked this pull request as ready for review September 19, 2022 00:05

rogerluan changed the title ~~[action] add 'xcodes' action, deprecating 'xcversion' and 'xcode-install'~~ [action] add xcodes action, deprecating xcversion and xcode-install Sep 19, 2022

rogerluan requested review from revolter and joshdholtz September 20, 2022 11:49

rogerluan mentioned this pull request Sep 21, 2022

Add "version" argument to "installed" function, printing Xcode's installation path XcodesOrg/xcodes#220

Merged

fastlane-bot added the status: needs-attention label Oct 3, 2022

revolter requested changes Oct 13, 2022

View reviewed changes

rogerluan requested a review from revolter October 14, 2022 10:53

rogerluan mentioned this pull request Oct 29, 2022

[!] Cannot find an installed Xcode satisfying '13.0' using MacOS 11.7 xcpretty/xcode-install#469

Open

revolter requested changes Nov 1, 2022

View reviewed changes

revolter force-pushed the rogerluan-xcodes branch from c335335 to b5fbe8f Compare November 1, 2022 20:33

rogerluan added 8 commits November 2, 2022 08:32

Move helpers to a separate and centralized file.

7c1de15

Deprecate 'xcode-install' gem (the project has been sunset), adding '…

676e3ee

…xcodes' action to replace the 'xcode_install' one.

Add 'select_only' option to 'xcodes', deprecating 'xcversion' action.

8c5c733

Update documentation to state it also supports .xcode-version file.

eb583d5

Add TODO for a future improvement.

54786b0

Lint.

b8d6090

Fix link.

766c53b

Remove TODO comment.

6d6ff1f

revolter force-pushed the rogerluan-xcodes branch 7 times, most recently from 2b04c63 to fad9d58 Compare November 2, 2022 21:58

revolter added 3 commits November 3, 2022 00:08

Remove superfluous trailing slash

7d0caea

This makes it consistent with the rest of the codebase.

Fix unsafe paths concatenation

317296b

revolter force-pushed the rogerluan-xcodes branch from fad9d58 to 11bd750 Compare November 2, 2022 22:08

Replace Actions.sh call with a native one

1a27157

(reverted from commit 11bd750)

revolter force-pushed the rogerluan-xcodes branch from ab46738 to 0c2dddf Compare November 3, 2022 17:18

revolter approved these changes Nov 4, 2022

View reviewed changes

rogerluan commented Nov 4, 2022

View reviewed changes

revolter force-pushed the rogerluan-xcodes branch from 0c2dddf to 58a0ed9 Compare November 4, 2022 17:54

joshdholtz approved these changes Nov 12, 2022

View reviewed changes

joshdholtz merged commit 4b30089 into master Nov 12, 2022

rogerluan deleted the rogerluan-xcodes branch November 12, 2022 13:42

joshdholtz mentioned this pull request Nov 12, 2022

Version bump to 2.211.0 #20852

Merged

mokagio mentioned this pull request Nov 16, 2022

ensure_xcode_version uses xcversion but has not been deprecated #20866

Open

4 tasks

fastlane-bot removed the status: needs-attention label Nov 16, 2022

fastlane-bot added the status: released label Nov 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[action] add `xcodes` action, deprecating `xcversion` and `xcode-install` #20672

[action] add `xcodes` action, deprecating `xcversion` and `xcode-install` #20672

rogerluan commented Sep 18, 2022 •

edited

rogerluan commented Sep 19, 2022

rogerluan commented Sep 19, 2022

rogerluan commented Sep 20, 2022

revolter left a comment

rogerluan commented Oct 14, 2022

rogerluan commented Oct 14, 2022 •

edited

rogerluan commented Oct 29, 2022

revolter left a comment

revolter commented Nov 1, 2022

rogerluan commented Nov 1, 2022

revolter commented Nov 2, 2022

revolter commented Nov 2, 2022 •

edited

revolter commented Nov 3, 2022

revolter commented Nov 3, 2022

revolter commented Nov 3, 2022

rogerluan left a comment

rogerluan Nov 4, 2022

rogerluan Nov 4, 2022

revolter commented Nov 4, 2022

joshdholtz left a comment

fastlane-bot commented Nov 16, 2022

[action] add xcodes action, deprecating xcversion and xcode-install #20672

[action] add xcodes action, deprecating xcversion and xcode-install #20672

Conversation

rogerluan commented Sep 18, 2022 • edited

Checklist

Motivation and Context

Description

Testing Steps

rogerluan commented Sep 19, 2022

rogerluan commented Sep 19, 2022

rogerluan commented Sep 20, 2022

revolter left a comment

Choose a reason for hiding this comment

rogerluan commented Oct 14, 2022

rogerluan commented Oct 14, 2022 • edited

rogerluan commented Oct 29, 2022

revolter left a comment

Choose a reason for hiding this comment

revolter commented Nov 1, 2022

rogerluan commented Nov 1, 2022

revolter commented Nov 2, 2022

revolter commented Nov 2, 2022 • edited

revolter commented Nov 3, 2022

revolter commented Nov 3, 2022

revolter commented Nov 3, 2022

rogerluan left a comment

Choose a reason for hiding this comment

rogerluan Nov 4, 2022

Choose a reason for hiding this comment

rogerluan Nov 4, 2022

Choose a reason for hiding this comment

revolter commented Nov 4, 2022

joshdholtz left a comment

Choose a reason for hiding this comment

fastlane-bot commented Nov 16, 2022

[action] add `xcodes` action, deprecating `xcversion` and `xcode-install` #20672

[action] add `xcodes` action, deprecating `xcversion` and `xcode-install` #20672

rogerluan commented Sep 18, 2022 •

edited

rogerluan commented Oct 14, 2022 •

edited

revolter commented Nov 2, 2022 •

edited