1123824 - |mach bootstrap| suggests a mozconfig with bad --with-android-sdk on Linux

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Wesley Johnston (:wesj)]

On Linux, ./mach bootstrap created a mozconfig pointing to /home/wesj/.mozbuild/android-sdk-linux for me. That looks right! but it apparently needs to point to the platform folder that you want instead:

https://wiki.mozilla.org/Mobile/Fennec/Android#Preparing_a_Fennec_mozconfig

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: bz://1123824/nalexander

/r/2989 - Bug 1123824 - Include platforms/android-VERSION in suggested mozconfig. r=gps

Pull down this commit:

hg pull review -r 17f61e65598ab1eebcf0d17bac95c7c894e5e269

Comment 2

[Monty the Python Review Bot]

https://reviewboard.mozilla.org/r/2989/#review2311

Always look on the bright side of life.

I analyzed your Python changes and found 1 errors.

The following files were examined:

  python/mozboot/mozboot/debian.py

::: python/mozboot/mozboot/debian.py
(Diff revision 1)
> +        sdk_path = os.path.join(self.sdk_path, 'platforms', android.ANDROID_PLATFORM)

E501: line too long (85 > 79 characters)
Limit all lines to a maximum of 79 characters.

There are still many devices around that are limited to 80 character
lines; plus, limiting windows to 80 characters makes it possible to have
several windows side-by-side.  The default wrapping on such devices looks
ugly.  Therefore, please limit all lines to a maximum of 79 characters.
For flowing long blocks of text (docstrings or comments), limiting the
length to 72 characters is recommended.

Reports error E501.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Monty the Python Review Bot from comment #2)
> https://reviewboard.mozilla.org/r/2989/#review2311
> 
> Always look on the bright side of life.
> 
> I analyzed your Python changes and found 1 errors.
> 
> The following files were examined:
> 
>   python/mozboot/mozboot/debian.py
> 
> ::: python/mozboot/mozboot/debian.py
> (Diff revision 1)
> > +        sdk_path = os.path.join(self.sdk_path, 'platforms', android.ANDROID_PLATFORM)
> 
> E501: line too long (85 > 79 characters)
> Limit all lines to a maximum of 79 characters.
> 
> There are still many devices around that are limited to 80 character
> lines; plus, limiting windows to 80 characters makes it possible to have
> several windows side-by-side.  The default wrapping on such devices looks
> ugly.  Therefore, please limit all lines to a maximum of 79 characters.
> For flowing long blocks of text (docstrings or comments), limiting the
> length to 72 characters is recommended.
> 
> Reports error E501.

I am unimpressed.  We have 28" monitors; I don't need to wrap to 80 characters.

Comment 4

[Nick Alexander :nalexander [he/him]]

This manifests like:

12:52 MungoRae: Hi, I'm trying to build firefox Android source from these instructions https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec. When performing './mach build' I run into the error "configure: error: The path in --with-android-sdk isn't valid (source.properties hasn't been found)". I am wondering if someone here can help me figure out what I am doing wrong. Thanks.

Comment 6

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/ee25e5277ab6

Comment 7

[Nick Alexander :nalexander [he/him]]

gps: we were getting 1-5 people a day reporting this.  It's better to land and fix if needed than wait for review indefinitely.  And we had a contributor post a (slightly flawed) fix for review.

Comment 8

[Gregory Szorc [:gps]]

Yeah, that's fine. I was busy fighting fires the past few days and haven't touched my review queue since monday or tuesday. Bad timing.

Comment 9

[Mike Hommey [:glandium]]

Huh, so now there's code to add platforms/android-21 in osx.py and debian.py. And that doesn't work on other distros...

Comment 10

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Hommey [:glandium] from comment #9)
> Huh, so now there's code to add platforms/android-21 in osx.py and
> debian.py. And that doesn't work on other distros...

Yeah, we use SDK path to mean two things: the root to the Android SDK, and the platform SDK version we compile against.  The first makes more sense, especially in the bootstrapper.

The right fix would be to make --android-sdk-root sensible everywhere in the build system and include the platform as an explicit version, but that's a lot of churn for little value.

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ee25e5277ab6