Closed Bug 1446270 Opened 3 years ago Closed 3 years ago

Android/arm target should turn on crash reporter as default

Categories

(Firefox Build System :: General, enhancement)

3 Branch
ARM
Android
enhancement
Not set
normal

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(1 file)

You know, android/arm's target value is arm-unknown-linux-androideabi, But https://searchfox.org/mozilla-central/rev/6e96a3f1e44e286ddae5fdafab737709741d237a/old-configure.in#2834 matches as android/arm target, so MOZ_CRASHREPORTER is alway unset for Android/arm.
automation adds --enable-crashreporter, so MOZ_CRASHREPORTER is set.
Attachment #8959451 - Flags: review?(core-build-config-reviews) → review?(nfroyd)
Comment on attachment 8959451 [details]
Bug 1446270 - Android/arm target should turn on crash reporter as default.

https://reviewboard.mozilla.org/r/228262/#review234166

Does this mean we haven't been getting crashreports from Android for...a long time?  Or just that it hasn't been the default for the build?

::: old-configure.in:2834
(Diff revision 1)
> +*-android*|*-linuxandroid*)
> +  MOZ_CRASHREPORTER=1
> +  ;;

Might be worth a comment here indicating that this needs to come before the linux case?  Ideally this code will go away soon, so perhaps it doesn't matter much.
Attachment #8959451 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #3)
> Comment on attachment 8959451 [details]
> Bug 1446270 - Android/arm target should turn on crash reporter as default.
> 
> https://reviewboard.mozilla.org/r/228262/#review234166
> 
> Does this mean we haven't been getting crashreports from Android for...a
> long time?  Or just that it hasn't been the default for the build?

We definitely have been getting Android crash reports, but it's probably not been the default for the build.

> ::: old-configure.in:2834
> (Diff revision 1)
> > +*-android*|*-linuxandroid*)
> > +  MOZ_CRASHREPORTER=1
> > +  ;;
> 
> Might be worth a comment here indicating that this needs to come before the
> linux case?  Ideally this code will go away soon, so perhaps it doesn't
> matter much.

Let us hope :)
Someone should move this all to moz.configure at some point.
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf807404eb88
Android/arm target should turn on crash reporter as default. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/bf807404eb88
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.