Closed Bug 1174757 Opened 8 years ago Closed 7 years ago

[geckoview] Default GeckoView build flag to off locally

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

defect
Not set
normal

Tracking

(firefox41 affected)

RESOLVED DUPLICATE of bug 1212347
Tracking Status
firefox41 --- affected

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(2 files)

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.
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
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?
(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)
(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?
(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.
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)
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!
Pushed the pre.
Keywords: leave-open
Assignee: rnewman → nalexander
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)
(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
Closed: 7 years ago
Resolution: --- → DUPLICATE
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Product: Firefox for Android → Firefox Build System
You need to log in before you can comment on or make changes to this bug.