[gym] Fix analyze_build_time setting interfering with xcpretty log generation #12527
Conversation
| @@ -46,26 +46,25 @@ def options | |||
| options | |||
| end | |||
|
|
|||
| def actions | |||
| def buildactions | |||
There was a problem hiding this comment.
renamed to match more closely with xcodebuild's own documentation:
buildaction ...
Specify a build action (or actions) to perform on the target. Available build actions are:
| suffix = [] | ||
| suffix << "CODE_SIGN_IDENTITY=#{Gym.config[:codesigning_identity].shellescape}" if Gym.config[:codesigning_identity] | ||
| suffix | ||
| def setting |
There was a problem hiding this comment.
renamed to match more closely with xcodebuild's own documentation:
setting=value
Set the build setting setting to value.
| @@ -100,6 +100,20 @@ def build_app | |||
| mark_archive_as_built_by_gym(BuildCommandGenerator.archive_path) | |||
| UI.success("Successfully stored the archive. You can find it in the Xcode Organizer.") unless Gym.config[:archive_path].nil? | |||
| UI.verbose("Stored the archive in: " + BuildCommandGenerator.archive_path) | |||
|
|
|||
| post_build_app | |||
There was a problem hiding this comment.
is there a better way to chain this after the success of line 93?
There was a problem hiding this comment.
This seems like a good enough place to me 😊
|
Ha, I just saw #12523 which tackles the same bug. Please close this PR if needed. |
| "| tee #{log_path.shellescape}", | ||
| "| grep .[0-9]ms | grep -v ^0.[0-9]ms | sort -nr > culprits.txt", | ||
| "| tee #{@log_path.shellescape}", | ||
| "| xcpretty" |
There was a problem hiding this comment.
I think this is a duplicate since xcpretty gets added here - https://github.com/fastlane/fastlane/pull/12527/files?utf8=✓&diff=unified#diff-440c4122de8c51769dc40db14188f030R70... right? 🤔
There was a problem hiding this comment.
I think the git diff is misleading here. The | xcpretty command is part of the pipe method, which does indeed manifest in the BuildCommandGenerator.generate output that this test is testing.
I added another test case, which pushed the old xcpretty down a few lines.
There was a problem hiding this comment.
My bad, that was in a spec 😝 You good 👍
allewun
changed the title
gym/lib/gym/runner.rb
Outdated
| # Post-processing of build_app | ||
| def post_build_app | ||
| command = BuildCommandGenerator.post_build | ||
| print_command(command, "Generated Post-Build Command") if FastlaneCore::Globals.verbose? |
There was a problem hiding this comment.
@allewun I did just catch one bug there 😊 Things go 💥 if the command array here is empty since... Putting an unless command.empty? around this fixed it for me... Do you have this same issue?
There was a problem hiding this comment.
Nice catch! I added a guard here.
|
@joshdholtz should be good to go, let me know if there's anything missing! |
|
Hey @allewun 👋 Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉 Please let us know if this change requires an immediate release by adding a comment here 👍 |
fastlane-bot
added
status: released
and removed
status: included-in-next-release
labels
|
Congratulations! 🎉 This was released as part of fastlane 2.96.0 🚀 |
Fixes #10187
Checklist
bundle exec rspecfrom the root directory to see all new and existing tests passbundle exec rubocop -ato ensure the code style is validMotivation and Context
gym'sanalyze_build_timeoption interferes with its ownxcprettyoutput, since the analysis steps are incorrectly inserted between thexcodebuildandxcprettypipelines and it improperly consumes output that should be piped toxcpretty. Processes that depend onxcprettywill thus fail (e.g. the json-compilation-database isn't generated whenanalyze_build_timeis on).Description
I moved the
analyze_build_timesteps to apost_buildstep that waits until the mainxcodebuild ... | xcprettycommand has finished. It seems more fitting that any build analysis should happen after building has completed, not during the build process.