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.

Sign up for GitHub

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

Jump to bottom

SimLauncher: fix autodetection of app bundle #974

Merged
merged 11 commits into from
Feb 16, 2016
Merged

SimLauncher: fix autodetection of app bundle #974

Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e2e478e
SimLauncher: fix autodetection of app bundle
ark-konopacki Jan 6, 2016
bf73acb
Updated executable auto detection
ark-konopacki Jan 12, 2016
a2b4a68
Fix searched dir in error message
ark-konopacki Jan 12, 2016
5560af5
Minor fix in detecting iphonesimulator folder
ark-konopacki Jan 12, 2016
b60d713
Minor fixes after review
ark-konopacki Jan 13, 2016
9e6ba0b
Fixed find mechanism with correct order
ark-konopacki Jan 13, 2016
4dae351
Added break to stop iterations after first file with calabash
ark-konopacki Jan 13, 2016
eb06783
Merge branch 'develop' of https://github.com/calabash/calabash-ios in…
ark-konopacki Feb 5, 2016
b1f0add
Update patch to use app.calabash_server_version method
ark-konopacki Feb 5, 2016
bcfb77e
Detect app: extract search and select to methods
jmoody Feb 15, 2016
f4a710d
Detect app: use methods we extracted
jmoody Feb 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions calabash-cucumber/lib/calabash-cucumber/launch/simulator_launcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,26 @@ def app_bundle_or_raise(path=nil, device_build_dir='iPhoneSimulator')
puts('-'*37)
end
else
bo_dir = build_output_dir_for_project
sim_dirs = Dir.glob(File.join(bo_dir, '*-iphonesimulator', '*.app'))
search_dir = build_output_dir_for_project || DERIVED_DATA
sim_dirs = ''
apps = `find #{search_dir} -type d -name "*.app" -exec stat -f "%m %N" {} \\; | sort -rn | cut -d" " -f2`.split("\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to be careful of UTF-8 chars here.

We can use ruby's Find or Dir.glob + File.mtime to perform this operation.

apps.each do |app_path|
lipo = RunLoop::Lipo.new(app_path)
arches = lipo.info
if arches.include?("x86_64") || arches.include?("i386")
app = RunLoop::App.new(app_path)
executable_name = app.executable_name
path_to_bin = File.join(app_path, executable_name)
if `xcrun strings "#{path_to_bin}" | grep -E 'CALABASH VERSION'`.include? "CALABASH VERSION"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want to make more work for you, but do you think you could implement this in:

The method name in each should be calabash_server_version. You can duplicate the code in each - you don't have to worry about repeating yourself. If you are really ambitious you can write a Strings class that can call strings on an arbitrary file.

There is a small problem with just querying the executable for the server - sometimes apps dynamically link calabash. This is different from dylib injection. In this case, the app loads a calabash dylib from the app bundle at runtime.

I am not overly worried about his case. User that do use this kind of dynamic linking will probably be savvy enough to set the APP variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I should add that ^ is one of my tasks for this week.

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 don't want to make more work for you, but do you think you could implement this

I can do that.

I am not overly worried about his case. User that do use this kind of dynamic linking will probably be savvy enough to set the APP variable.

we can add support for that case later in run_loop

Copy link
Contributor

Choose a reason for hiding this comment

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

Here also we need to be careful of UTF-8 characters.

Also, we should wait for Improve how App and Ipa instances detect the Calabash server version #379 to be merged and released.

Copy link
Contributor

Choose a reason for hiding this comment

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

That PR has been merged.

I recommend a rebase against develop.

You can create an App instance and ask for the calabash version.

app.calabash_server_version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmody thanks for feedback, since that changes is in run_loop should i worried about it versions? I mean if we will have old run_loop installed without
app.calabash_server_version method new implementation will fail in that case.
And i think i should update it to find instead of each.

sim_dirs = Dir.glob(app_path)
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

In the worst case, this looks like it will iterate over all apps that found and then sim_dirs will be the last app that matches the criteria and not the first.

I think we want to use: apps.find do instead of apps.each

sim_dirs = apps.find do |app_path|

end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmoody: maybe it can be done like by adding break after
sim_dirs = Dir.glob(app_path)


if sim_dirs.empty?
msg = ['Unable to auto detect APP_BUNDLE_PATH.']
msg << 'Have you built your app for simulator?'
msg << "Searched dir: #{bo_dir}"
msg << "Searched dir: #{search_dir}"
msg << 'Please build your app from Xcode'
msg << 'You should build the -cal target.'
msg << ''
Expand Down