Closed
Bug 1428110
Opened 6 years ago
Closed 6 years ago
Build fails with SDK Tools 25.2.5
Categories
(Firefox Build System :: Android Studio and Gradle Integration, defect)
Tracking
(firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: matias.nicolas.zc, Assigned: matias.nicolas.zc)
Details
Attachments
(1 file, 1 obsolete file)
1.29 KB,
patch
|
nalexander
:
review+
|
Details | Diff | Splinter Review |
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
Comment 1•6 years ago
|
||
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•6 years 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
Comment 3•6 years ago
|
||
(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•6 years ago
|
||
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 5•6 years ago
|
||
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+
![]() |
||
Updated•6 years ago
|
Flags: needinfo?(gbrown)
Assignee | ||
Comment 6•6 years 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?
Comment 7•6 years ago
|
||
(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)
Comment 9•6 years ago
|
||
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•6 years ago
|
||
In that case, this works for me
Attachment #8939922 -
Attachment is obsolete: true
Attachment #8940903 -
Flags: review?(nalexander)
Comment 11•6 years ago
|
||
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+
Updated•6 years ago
|
Assignee: nobody → matias.nicolas.zc
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed
Comment 12•6 years 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•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b96fa0adc2b3
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Updated•4 years ago
|
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 59 → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•