[action][spm] add simulator flag for swift compiler #21707

gharary · 2023-12-09T16:38:16Z

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.
I've added or updated relevant unit tests.

Motivation and Context

This PR adds extra flag to the spm action, to be able to pass the simulator which swift build is going to compile on that.

Description

Currently, using spm action, will run the swift compiler on the first available simulator. Which is an issue if running on CI server, as some SPM packages only supports iOS platform and running the compiler on a MacOS platform would fail the build. This PR will add an extra flag to the spm action, so the user can pass the simulator on which the swift compiler should run on.
The way it works is that, currently swift compiler doesn't support a simulator flag, therefore we are passing it as a Xswiftc flag. First specifying which SDK the compiler is going to use, by passing the sdk flag. Then using xcrun to locate tools within the Xcode environment which helps find the path to the specified SDK. Then adding the target flag which is used to set the target architecture for the Swift compiler and finally passing the target platform and architecture.
Now user can sets the simulator and/or architercure of the SDK that is going to be used for compiler.

Testing Steps

Adding four unit tests to test the commands

lacostej

See comment

fastlane/lib/fastlane/actions/spm.rb

rogerluan

Congrats on your first PR in the fastlane project @gharary!

I left a question and a couple suggestions. Aside from those, would you mind adding unit tests to this PR? fastlane/spec/actions_specs/spm_spec.rb contains extensive tests already, in case you need sample code to look at when implementing these tests 😊 Let me know if you need any help!

Looking forward to getting this PR merged; the lack of simulator flag is really annoying! Thanks for working on this! 💪

fastlane/lib/fastlane/actions/spm.rb

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

fastlane/spec/actions_specs/spm_spec.rb

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

fastlane/spec/actions_specs/spm_spec.rb

rogerluan

Just a few more changes @gharary 😊 looking good!

fastlane/lib/fastlane/actions/spm.rb

rogerluan · 2023-12-16T10:02:50Z

fastlane/spec/actions_specs/spm_spec.rb

Since it looks like the macOS simulator command wasn't working (? can't really tell for sure ATM), is there any way we can add a unit test that makes sure not only the output command is the expected string, but that the command actually works?

I wasn't able to find a way to actually test the code in unit tests. I was thinking of using open3 to catch the result of executing command, but then testing the swift build command needs an actual SPM. So wasn't making sense to me. Any suggestion?

Uhm, if we wanted, we could create a sample SPM just for testing purposes. But, on a second thought, I think we actually might not need this, as long as we test it manually and make sure it works with all configurations, this should be fine. The reason is that we'd be validating whether "swift build" works, and that's not really the intent of what fastlane's unit tests should be :)

So this is good as is 🙏 thank you!

rogerluan

Just two minor nitpicks left related to code style, otherwise this PR looks good to go @gbrhaz ! Thanks all the hard work here! 💪 🎉

fastlane/lib/fastlane/actions/spm.rb

rogerluan · 2023-12-18T16:25:21Z

fastlane/spec/actions_specs/spm_spec.rb

Uhm, if we wanted, we could create a sample SPM just for testing purposes. But, on a second thought, I think we actually might not need this, as long as we test it manually and make sure it works with all configurations, this should be fine. The reason is that we'd be validating whether "swift build" works, and that's not really the intent of what fastlane's unit tests should be :)

So this is good as is 🙏 thank you!

fastlane/lib/fastlane/actions/spm.rb

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

gharary · 2023-12-18T18:06:04Z

I did test the shadowing and it works fine and also tests are passing so I think all good.

rogerluan

🚀 awesome work! Thank you for the patience and for this contribution @gharary ! 🙏

* Add simulator flag to spm action for swift compiler * add user entry syntax check * removing ipadsimulator from syntax check * Adding verify_block for flag check * adding simulators list to user_error * Update fastlane/lib/fastlane/actions/spm.rb Co-authored-by: Roger Oba <rogerluan.oba@gmail.com> * Update fastlane/lib/fastlane/actions/spm.rb Co-authored-by: Roger Oba <rogerluan.oba@gmail.com> * Adding unit tests for simulator * Fix simulator typo * Update fastlane/spec/actions_specs/spm_spec.rb Co-authored-by: Roger Oba <rogerluan.oba@gmail.com> * Fix typo. * Adding support for different simulator architecture * Adding unit tests for simulator architecture * Modify code by extracting the logic into methods * Update fastlane/lib/fastlane/actions/spm.rb Co-authored-by: Roger Oba <rogerluan.oba@gmail.com> * Update fastlane/lib/fastlane/actions/spm.rb Co-authored-by: Roger Oba <rogerluan.oba@gmail.com> --------- Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

gharary added 2 commits December 9, 2023 17:11

Add simulator flag to spm action for swift compiler

27f4904

add user entry syntax check

50e3f57

lacostej changed the title ~~Add simulator flag to spm action for swift compiler~~ [action][spm] add simulator flag for swift compiler Dec 11, 2023

removing ipadsimulator from syntax check

52c9b05

lacostej reviewed Dec 11, 2023

View reviewed changes

fastlane/lib/fastlane/actions/spm.rb Outdated Show resolved Hide resolved

Adding verify_block for flag check

9b88cd9

lacostej approved these changes Dec 11, 2023

View reviewed changes

lacostej reviewed Dec 11, 2023

View reviewed changes

fastlane/lib/fastlane/actions/spm.rb Outdated Show resolved Hide resolved

adding simulators list to user_error

a066d0f

lacostej requested a review from rogerluan December 12, 2023 19:22

rogerluan requested changes Dec 12, 2023

View reviewed changes

fastlane/lib/fastlane/actions/spm.rb Outdated Show resolved Hide resolved

fastlane/lib/fastlane/actions/spm.rb Outdated Show resolved Hide resolved

fastlane/lib/fastlane/actions/spm.rb Outdated Show resolved Hide resolved

gharary and others added 3 commits December 13, 2023 18:35

Update fastlane/lib/fastlane/actions/spm.rb

f61944a

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

Update fastlane/lib/fastlane/actions/spm.rb

d83f680

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

Adding unit tests for simulator

b07b167

rogerluan reviewed Dec 13, 2023

View reviewed changes

fastlane/spec/actions_specs/spm_spec.rb Outdated Show resolved Hide resolved

Fix simulator typo

a26bcb1

rogerluan reviewed Dec 13, 2023

View reviewed changes

fastlane/spec/actions_specs/spm_spec.rb Outdated Show resolved Hide resolved

Update fastlane/spec/actions_specs/spm_spec.rb

a31086b

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

rogerluan reviewed Dec 13, 2023

View reviewed changes

fastlane/spec/actions_specs/spm_spec.rb Outdated Show resolved Hide resolved

Fix typo.

39447f1

rogerluan added the tool: action label Dec 14, 2023

gharary added 2 commits December 16, 2023 00:00

Adding support for different simulator architecture

ce01c52

Adding unit tests for simulator architecture

dfdb19b

rogerluan requested changes Dec 16, 2023

View reviewed changes

gharary added 2 commits December 17, 2023 15:10

Modify code by extracting the logic into methods

38334a8

Merge branch 'master' into spm-add-simulator

58de71f

rogerluan reviewed Dec 18, 2023

View reviewed changes

gharary and others added 2 commits December 18, 2023 19:05

Update fastlane/lib/fastlane/actions/spm.rb

810b4bf

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

Update fastlane/lib/fastlane/actions/spm.rb

013fc34

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

rogerluan approved these changes Dec 18, 2023

View reviewed changes

rogerluan merged commit 287d910 into fastlane:master Dec 19, 2023

rogerluan mentioned this pull request Dec 20, 2023

[ci] fix linter issues that slipped to master due to CI issue #21747

Merged

3 tasks

joshdholtz mentioned this pull request Jan 2, 2024

Version bump to 2.218.0 #21760

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[action][spm] add simulator flag for swift compiler #21707

[action][spm] add simulator flag for swift compiler #21707

gharary commented Dec 9, 2023 •

edited

Loading

lacostej left a comment

rogerluan left a comment

rogerluan left a comment

rogerluan Dec 16, 2023

gharary Dec 17, 2023

rogerluan Dec 18, 2023

rogerluan left a comment

rogerluan Dec 18, 2023

gharary commented Dec 18, 2023

rogerluan left a comment

[action][spm] add simulator flag for swift compiler #21707

[action][spm] add simulator flag for swift compiler #21707

Conversation

gharary commented Dec 9, 2023 • edited Loading

Checklist

Motivation and Context

Description

Testing Steps

lacostej left a comment

Choose a reason for hiding this comment

rogerluan left a comment

Choose a reason for hiding this comment

rogerluan left a comment

Choose a reason for hiding this comment

rogerluan Dec 16, 2023

Choose a reason for hiding this comment

gharary Dec 17, 2023

Choose a reason for hiding this comment

rogerluan Dec 18, 2023

Choose a reason for hiding this comment

rogerluan left a comment

Choose a reason for hiding this comment

rogerluan Dec 18, 2023

Choose a reason for hiding this comment

gharary commented Dec 18, 2023

rogerluan left a comment

Choose a reason for hiding this comment

gharary commented Dec 9, 2023 •

edited

Loading