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

[TIMOB-20130] SDK no longer finds Genymotion after upgrade to 2.6.0 #7669

Merged
merged 2 commits into from Feb 2, 2016

Conversation

FokkeZB
Copy link
Contributor

@FokkeZB FokkeZB commented Feb 2, 2016

@@ -286,7 +286,10 @@ function findGenymotion(dir, config, callback) {
if (!player || !fs.existsSync(player)) {
player = path.join(dir, 'player' + exe);
}
if (player && !fs.existsSync(player)) {
if (!fs.existsSync(player) && process.platform != 'win32') {
Copy link
Contributor

Choose a reason for hiding this comment

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

You still need to check if player is truthy in every if statement. The path you are setting is OS X specific, so just compare to darwin. Use !== instead of !=.

if (process.platform === 'darwin' && (!player || !fs.existsSync(player))) {
    player = path.join(dir, 'player.app', 'Contents', 'MacOS', 'player');
}
if (player && !fs.existsSync(player)) {
    player = null;
}

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 get the darwin fix, but why the truthy check? After line https://github.com/FokkeZB/titanium_mobile/blob/patch-37/node_modules/titanium-sdk/lib/emulators/genymotion.js#L287 it will always be truthy since path.join() does not resolve and just joins the arguments 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 blame you for the == btw, thought I should have corrected it indeed ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with omitting the player truthiness checks. At least now it works on Linux.

I've slowly been migrating == to ===. There's a lot of code. :)

Can you also please fix master?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it works on Linux as it was since the fs.existsSync on the next line will simply return false, but here's the, uh... "forward port" for master:

#7670

@cb1kenobi
Copy link
Contributor

Since this is the backport, I suggest fixing master, then backporting both this commit and the fix.

@cb1kenobi
Copy link
Contributor

Thank you! APPROVED

cb1kenobi added a commit that referenced this pull request Feb 2, 2016
[TIMOB-20130] SDK no longer finds Genymotion after upgrade to 2.6.0
@cb1kenobi cb1kenobi merged commit 1619c16 into tidev:5_2_X Feb 2, 2016
@FokkeZB FokkeZB deleted the patch-37 branch March 23, 2016 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants