Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gym] Fix analyze_build_time setting interfering with xcpretty log generation #12527

Merged
merged 5 commits into from
May 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 17 additions & 13 deletions gym/lib/gym/generators/build_command_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ def generate
parts = prefix
parts << "xcodebuild"
parts += options
parts += actions
parts += suffix
parts += buildactions
parts += setting
parts += pipe

parts
Expand Down Expand Up @@ -46,26 +46,25 @@ def options
options
end

def actions
def buildactions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

config = Gym.config

actions = []
actions << :clean if config[:clean]
actions << :archive unless config[:skip_archive]
buildactions = []
buildactions << :clean if config[:clean]
buildactions << :archive unless config[:skip_archive]

actions
buildactions
end

def suffix
suffix = []
suffix << "CODE_SIGN_IDENTITY=#{Gym.config[:codesigning_identity].shellescape}" if Gym.config[:codesigning_identity]
suffix
def setting
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed to match more closely with xcodebuild's own documentation:

setting=value
           Set the build setting setting to value.

setting = []
setting << "CODE_SIGN_IDENTITY=#{Gym.config[:codesigning_identity].shellescape}" if Gym.config[:codesigning_identity]
setting
end

def pipe
pipe = []
pipe << "| tee #{xcodebuild_log_path.shellescape}"
pipe << "| grep .[0-9]ms | grep -v ^0.[0-9]ms | sort -nr > culprits.txt" if Gym.config[:analyze_build_time]
unless Gym.config[:disable_xcpretty]
formatter = Gym.config[:xcpretty_formatter]
pipe << "| xcpretty"
Expand All @@ -89,10 +88,15 @@ def pipe
end
end
pipe << "> /dev/null" if Gym.config[:suppress_xcode_output]

pipe
end

def post_build
commands = []
commands << %{grep -E '^[0-9.]+ms' #{xcodebuild_log_path.shellescape} | grep -vE '^0\.[0-9]' | sort -nr > culprits.txt} if Gym.config[:analyze_build_time]
commands
end

def xcodebuild_log_path
file_name = "#{Gym.project.app_name}-#{Gym.config[:scheme]}.log"
containing = File.expand_path(Gym.config[:buildlog_path])
Expand Down
17 changes: 17 additions & 0 deletions gym/lib/gym/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,23 @@ 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a better way to chain this after the success of line 93?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good enough place to me 😊

end

# Post-processing of build_app
def post_build_app
command = BuildCommandGenerator.post_build

return if command.empty?

print_command(command, "Generated Post-Build Command") if FastlaneCore::Globals.verbose?
FastlaneCore::CommandExecutor.execute(command: command,
print_all: true,
print_command: !Gym.config[:silent],
error: proc do |output|
ErrorHandler.handle_build_error(output)
end)
end

# Makes sure the archive is there and valid
Expand Down
35 changes: 31 additions & 4 deletions gym/spec/build_command_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,11 @@
end

describe "Analyze Build Time Example" do
it "uses the correct build command with the example project", requires_xcodebuild: true do
log_path = File.expand_path("#{FastlaneCore::Helper.buildlog_path}/gym/ExampleProductName-Example.log")
before do
@log_path = File.expand_path("#{FastlaneCore::Helper.buildlog_path}/gym/ExampleProductName-Example.log")
end

it "uses the correct build command with the example project when option is enabled", requires_xcodebuild: true do
options = { project: "./gym/examples/standard/Example.xcodeproj", analyze_build_time: true, scheme: 'Example' }
Gym.config = FastlaneCore::Configuration.create(Gym::Options.available_options, options)

Expand All @@ -213,10 +215,35 @@
"-archivePath #{Gym::BuildCommandGenerator.archive_path.shellescape}",
"OTHER_SWIFT_FLAGS=\"-Xfrontend -debug-time-function-bodies\"",
:archive,
"| tee #{log_path.shellescape}",
"| grep .[0-9]ms | grep -v ^0.[0-9]ms | sort -nr > culprits.txt",
"| tee #{@log_path.shellescape}",
"| xcpretty"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, that was in a spec 😝 You good 👍

])

result = Gym::BuildCommandGenerator.post_build
expect(result).to eq([
"grep -E '^[0-9.]+ms' #{@log_path.shellescape} | grep -vE '^0\.[0-9]' | sort -nr > culprits.txt"
])
end

it "uses the correct build command with the example project when option is disabled", requires_xcodebuild: true do
options = { project: "./gym/examples/standard/Example.xcodeproj", analyze_build_time: false, scheme: 'Example' }
Gym.config = FastlaneCore::Configuration.create(Gym::Options.available_options, options)

result = Gym::BuildCommandGenerator.generate
expect(result).to eq([
"set -o pipefail &&",
"xcodebuild",
"-scheme Example",
"-project ./gym/examples/standard/Example.xcodeproj",
"-destination 'generic/platform=iOS'",
"-archivePath #{Gym::BuildCommandGenerator.archive_path.shellescape}",
:archive,
"| tee #{@log_path.shellescape}",
"| xcpretty"
])

result = Gym::BuildCommandGenerator.post_build
expect(result).to be_empty
end
end
end
Expand Down