[geckoview] Default GeckoView build flag to off locally

RESOLVED DUPLICATE of bug 1212347

Status

()

RESOLVED DUPLICATE of bug 1212347
3 years ago
8 months ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox41 affected)

Details

Attachments

(2 attachments)

We should flip the GeckoView build flag so that by default GV is only built on the builders.

We have very few local conflicts (since GV is basically Fennec); GV building is slow during packaging; and we see terrible errors like https://code.google.com/p/android/issues/detail?id=176488 that we have little chance to address.
(Assignee)

Comment 1

3 years ago
The relevant build flag is MOZ_DISABLE_GECKOVIEW.  I suggest we flip that to MOZ_PACKAGE_GECKOVIEW or similar and /enable/ it in automation, rather than disable it.

https://dxr.mozilla.org/mozilla-central/search?q=MOZ_DISABLE_GECKOVIEW&case=true&redirect=true
Created attachment 8622525 [details] [diff] [review]
Only build GeckoView for API11+ Nightly. v1

Untested. N.B., we only ever build GV if NIGHTLY_BUILD is set anyway, so no point touching other mozconfigs, no?
Attachment #8622525 - Flags: review?(nalexander)
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
When I mentioned doing this a while ago, someone made the case that this was needed on local builds because it was easy to break without knowing before landing. Isn't this true anymore?
(Assignee)

Comment 4

3 years ago
(In reply to Mike Hommey [:glandium] from comment #3)
> When I mentioned doing this a while ago, someone made the case that this was
> needed on local builds because it was easy to break without knowing before
> landing. Isn't this true anymore?

I don't think it's true anymore.  That might have been blassey?
Flags: needinfo?(blassey.bugs)
It was probably blassey or mfinkle.
(In reply to Nick Alexander :nalexander from comment #4)
> (In reply to Mike Hommey [:glandium] from comment #3)
> > When I mentioned doing this a while ago, someone made the case that this was
> > needed on local builds because it was easy to break without knowing before
> > landing. Isn't this true anymore?
> 
> I don't think it's true anymore.  That might have been blassey?

why would this not be true anymore?
Flags: needinfo?(blassey.bugs)
(Assignee)

Comment 7

3 years ago
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #6)
> (In reply to Nick Alexander :nalexander from comment #4)
> > (In reply to Mike Hommey [:glandium] from comment #3)
> > > When I mentioned doing this a while ago, someone made the case that this was
> > > needed on local builds because it was easy to break without knowing before
> > > landing. Isn't this true anymore?
> > 
> > I don't think it's true anymore.  That might have been blassey?
> 
> why would this not be true anymore?

The GeckoView packaging has not been fragile, to the best of my knowledge, for a long time.  The amount of churn in the build system is low.

Local developers are spending a lot of time, including dealing with tools failures that we can't really work-around, packaging a thing that gets minimal usage.
Sadly, we have GeckoView bustage right now. I had to manually patch my Android SDK top be able to build/package GeckoView. The builders don't seem to have the problem. Does that mean we are still catching GeckoView bustage on TBPL?
(Assignee)

Comment 9

3 years ago
(In reply to Mark Finkle (:mfinkle) from comment #8)
> Sadly, we have GeckoView bustage right now. I had to manually patch my
> Android SDK top be able to build/package GeckoView. The builders don't seem
> to have the problem. Does that mean we are still catching GeckoView bustage
> on TBPL?

We have a weak combination: we catch build time bustage -- really only /package time/ bustage -- on TBPL; and we catch no run-time bustage.  I have been pushing Geckoview AARs to TBPL for a while; it wouldn't be too hard to run a CI or Travis Gradle job that interates against those AARs and actually runs a test suite.
(Assignee)

Comment 10

3 years ago
Comment on attachment 8622525 [details] [diff] [review]
Only build GeckoView for API11+ Nightly. v1

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

Passing on this for now.  Sounds like some work is still needed.
Attachment #8622525 - Flags: review?(nalexander)
(Assignee)

Comment 11

3 years ago
Created attachment 8626384 [details]
MozReview Request: Bug 1174757 - Pre: Don't archive geckoview resources when not necessary. r=glandium

Bug 1174757 - Pre: Don't archive geckoview resources when not necessary. r=glandium

This is a small optimization that will also make it easier to flip the
MOZ_DISABLE_GECKOVIEW conditional.  We don't currently package
geckolibs without geckoview, and it's reasonable to do both if we want
one.
Attachment #8626384 - Flags: review?(mh+mozilla)
Attachment #8626384 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8626384 [details]
MozReview Request: Bug 1174757 - Pre: Don't archive geckoview resources when not necessary. r=glandium

https://reviewboard.mozilla.org/r/12083/#review10713

Ship It!
(Assignee)

Comment 14

3 years ago
Pushed the pre.
Keywords: leave-open
Assignee: rnewman → nalexander

Comment 16

3 years ago
I'm not a huge fan of leave-open bugs, especially if they haven't been touched in months. Can we close this and file new bugs for the necessary follow-up work?
Disabling the GV build is replaced by bug 1212347, so if it's just that, it can be duped.

Nick, what else is there to do here?
Flags: needinfo?(nalexander)
(Assignee)

Comment 18

3 years ago
(In reply to Michael Comella (:mcomella) from comment #17)
> Disabling the GV build is replaced by bug 1212347, so if it's just that, it
> can be duped.
> 
> Nick, what else is there to do here?

Nothing.  Somebody (possibly future me) can try to make GV better (perhaps post Gradle builds), and re-implement GVE at that time.
Flags: needinfo?(nalexander)
Sounds like those can be a follow-ups if we decide to do them.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1212347
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.