Build fails with SDK Tools 25.2.5

RESOLVED FIXED in Firefox 59

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: matias.nicolas.zc, Assigned: matias.nicolas.zc)

Tracking

58 Branch
Firefox 59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171130114045

Steps to reproduce:

Start a new build of firefox for android from the latest mozilla-beta branch with Android SDK Tools 25.2.5


Actual results:

Fails at configure stage with:
checking for Android tools... /opt/AndroidSDK/tools
checking for emulator... :
checking for emulator... (cached) :
configure: error: The program emulator was not found.


Expected results:

Build should work as with the 57-branch.
Emulator is present at tools/emulator
Matias: thanks for the report!  I think that we don't support 25.2.5; we require 25.0.3 for beta, I believe.  You can check by looking for the versions in https://searchfox.org/mozilla-central/source/old-configure.in#2147.

gbrown: is this related to https://bugzilla.mozilla.org/show_bug.cgi?id=1390606?
Flags: needinfo?(gbrown)
(Assignee)

Comment 2

a year ago
(In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018) from comment #1)
> Matias: thanks for the report!  I think that we don't support 25.2.5; we
> require 25.0.3 for beta, I believe.  You can check by looking for the
> versions in
> https://searchfox.org/mozilla-central/source/old-configure.in#2147.
> 
> gbrown: is this related to
> https://bugzilla.mozilla.org/show_bug.cgi?id=1390606?

My build-tools version is 25.0.3, the problem is with android sdk tools.

Reverting the changes from Bug 1409624 works for me, it seems to fail at the second check
(In reply to Matías Zúñiga from comment #2)
> (In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018)
> from comment #1)
> > Matias: thanks for the report!  I think that we don't support 25.2.5; we
> > require 25.0.3 for beta, I believe.  You can check by looking for the
> > versions in
> > https://searchfox.org/mozilla-central/source/old-configure.in#2147.
> > 
> > gbrown: is this related to
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1390606?
> 
> My build-tools version is 25.0.3, the problem is with android sdk tools.
> 
> Reverting the changes from Bug 1409624 works for me, it seems to fail at the
> second check

D'oh -- sorry!  Can you think about a patch that would accept both configurations?  Google likes to change these things in ways that seem random :(  Thanks!
(Assignee)

Comment 4

a year ago
Created attachment 8939922 [details] [diff] [review]
bug_1428110_-_Unset_EMULATOR_path_before_checking_again_for_it.patch

So, the problem is that autoconf firstly checks if the path is already set. Because it gets set in the first check (with a value ":"), it doesn't get set in the second check and fails.

Unsetting the autoconf variable before the second check works.
Attachment #8939922 - Flags: review?(nalexander)
Comment on attachment 8939922 [details] [diff] [review]
bug_1428110_-_Unset_EMULATOR_path_before_checking_again_for_it.patch

Review of attachment 8939922 [details] [diff] [review]:
-----------------------------------------------------------------

MOZ_PATH_PROG passes through to AC_PATH_PROG, and per https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Generic-Programs.html, we can check both paths in one go by separating with $PATH_SEPARATOR.  Can you try to make that work rather than using this internal autoconf variable, and report back?  Thanks!
Attachment #8939922 - Flags: review?(nalexander) → feedback+
Flags: needinfo?(gbrown)
(Assignee)

Comment 6

a year ago
(In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018) from comment #5)
> Comment on attachment 8939922 [details] [diff] [review]
> bug_1428110_-_Unset_EMULATOR_path_before_checking_again_for_it.patch
> 
> Review of attachment 8939922 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> MOZ_PATH_PROG passes through to AC_PATH_PROG, and per
> https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Generic-
> Programs.html, we can check both paths in one go by separating with
> $PATH_SEPARATOR.  Can you try to make that work rather than using this
> internal autoconf variable, and report back?  Thanks!

$PATH_SEPARATOR is not set for me, and im not sure if i can assume ":" as the separator used by the shell, just bash and zsh are supported?
(In reply to Matías Zúñiga from comment #6)
> (In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018)
> from comment #5)
> > Comment on attachment 8939922 [details] [diff] [review]
> > bug_1428110_-_Unset_EMULATOR_path_before_checking_again_for_it.patch
> > 
> > Review of attachment 8939922 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > MOZ_PATH_PROG passes through to AC_PATH_PROG, and per
> > https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Generic-
> > Programs.html, we can check both paths in one go by separating with
> > $PATH_SEPARATOR.  Can you try to make that work rather than using this
> > internal autoconf variable, and report back?  Thanks!
> 
> $PATH_SEPARATOR is not set for me, and im not sure if i can assume ":" as
> the separator used by the shell, just bash and zsh are supported?

Urgh, that sucks.  I think we can assume some things about the shell, but I'm not sure exactly what.  glandium, chmanchester -- any thoughts on how to specify two paths to MOZ_PATH_PROG in a cross-platform way?
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(cmanchester)
Move the tests to python configure?
Flags: needinfo?(mh+mozilla)
I'm not entirely sure, and I don't see any uses like this in old-configure today but I think assuming ":" would probably work.
Flags: needinfo?(cmanchester)
(Assignee)

Comment 10

a year ago
Created attachment 8940903 [details] [diff] [review]
bug_1428110_-_Check_both_paths_of_EMULATOR_in_one_pass.patch

In that case, this works for me
Attachment #8939922 - Attachment is obsolete: true
Attachment #8940903 - Flags: review?(nalexander)
Comment on attachment 8940903 [details] [diff] [review]
bug_1428110_-_Check_both_paths_of_EMULATOR_in_one_pass.patch

Review of attachment 8940903 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, Matías!
Attachment #8940903 - Flags: review?(nalexander) → review+
Assignee: nobody → matias.nicolas.zc
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed

Comment 12

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b96fa0adc2b3
Check both paths of EMULATOR in one pass. r=nalexander
Keywords: checkin-needed

Comment 13

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b96fa0adc2b3
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.