Closed Bug 1013201 Opened 6 years ago Closed 6 years ago

Update Gu job to build Gaia with DESKTOP_SHIMS=0

Categories

(Release Engineering :: General, defect, P1)

x86_64
Linux
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: zcampbell, Assigned: jgriffin)

Details

Attachments

(3 files, 2 obsolete files)

Currently on Gu on TBPL, the Gaia profile is build with DESKTOP_SHIMS=1. This is a 'debug' kind of profile that we used for touch events at some point.

We no longer need this and we can normalise Gaia profiles across test suites and b2g builds. Building Gu with DESKTOP_SHIMS=0 brings it into line with the other build processes.

P1 because the change has already been made on Travis.
The Tinderboxpushlog component is only for the web-app viewer of job results, rather than the build configs (Firefox OS :: Gaia::Build) or scheduling of the jobs themselves (Release Engineering::General Automation). Moving to releng for now, but it may belong in the Firefox OS product, depending on where the define lives.
Component: Tinderboxpushlog → General Automation
Product: Webtools → Release Engineering
QA Contact: catlee
Version: Trunk → unspecified
Have removed the define entirely, since it was added rather than toggled by https://hg.mozilla.org/build/mozharness/rev/bc537b691617
Attachment #8425398 - Flags: review?(aki)
Assignee: nobody → emorley
Status: NEW → ASSIGNED
Attachment #8425398 - Flags: review?(aki) → review+
Something here went live today
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Maybe this is why Gu is now permafail on v1.3 (including retriggers on previously-green runs)?

https://tbpl.mozilla.org/php/getParsedLog.php?id=40277914&tree=Mozilla-B2g28-v1.3
Flags: needinfo?(emorley)
No idea what the pref change did, was just helping Zac out; punting to him
Flags: needinfo?(emorley) → needinfo?(zcampbell)
Seems plausible though, given:
https://hg.mozilla.org/build/mozharness/rev/85bc563cec00

Zac, would something need uplifting?
Zac seems to be gone for the day, so I went ahead and backed this out from default and production.

https://hg.mozilla.org/build/mozharness/rev/9e196bd096b4
https://hg.mozilla.org/build/mozharness/rev/a5f3dd7b0dc7
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Gu is green on b2g28 again post-backout.
Zac, leaving this to you to figure out what needs uplifting etc.
Assignee: emorley → nobody
Edmorley, I did not realise that Gu shares the same config across all Gecko/Gaia versions.

The difference here is some improvements to desktopb2g profiles somewhere between 1.3/b2g28 and 1.4/b2g30 and I've a snowflake's chance in hell of working out which one it is.

Is there a way to get this config to be per-Gecko branch so we could enable this patch for the tip but not elsewhere?

It's not critical that we need this change on all branches, but to have it on m-c/gaia 2.0 would keep all configs inline in the future.
Flags: needinfo?(zcampbell) → needinfo?(emorley)
(In reply to Zac C (:zac) from comment #12)
> Edmorley, I did not realise that Gu shares the same config across all
> Gecko/Gaia versions.
> 
> The difference here is some improvements to desktopb2g profiles somewhere
> between 1.3/b2g28 and 1.4/b2g30 and I've a snowflake's chance in hell of
> working out which one it is.
> 
> Is there a way to get this config to be per-Gecko branch so we could enable
> this patch for the tip but not elsewhere?
> 
> It's not critical that we need this change on all branches, but to have it
> on m-c/gaia 2.0 would keep all configs inline in the future.

Aki, are we able to make this change:
https://hg.mozilla.org/build/mozharness/rev/acb3e29f40cf

...only for certain repositories?
Flags: needinfo?(emorley) → needinfo?(aki)
Not in any sort of elegant way.  I think it would be seriously ugly.
For instance,

* setting DESKTOP_SHIMS_HACK=1 in the mozharness config
* setting DESKTOP_SHIMS=1 if DESKTOP_SHIMS_HACK is set in the old repos
* ignoring DESKTOP_SHIMS_HACK in tip

seems a lot cleaner than any sort of what-repo-are-we-building detection code in mozharness that I can think of.  And the above is kind of ew.
Flags: needinfo?(aki)
What do other jobs do here - store their config in the branch?

We do this for Travis (and future treeherder), this make command is stored in the Gaia/git branch.

Can this mozharness script be put into the Git repo instead?
Attachment #8425398 - Flags: checked-in-
Aki, ni? for comment #15
Flags: needinfo?(aki)
Comment on attachment 8425398 [details] [diff] [review]
Update Gu job to build Gaia with DESKTOP_SHIMS disabled

(Trying to stop this appearing in bugzilla-todos)
Attachment #8425398 - Flags: review+
(In reply to Zac C (:zac) from comment #15)
> What do other jobs do here - store their config in the branch?

Yes, e.g. http://hg.mozilla.org/mozilla-central/file/a3848fbadb16/testing/config/mozharness/linux_config.py

> We do this for Travis (and future treeherder), this make command is stored
> in the Gaia/git branch.
> 
> Can this mozharness script be put into the Git repo instead?

We've discussed having mozharness be in-tree (gecko) but we're not ready for that yet.
Flags: needinfo?(aki)
We could probably make in-gaia config files ala linux_config.py that control how mozharness jobs make gaia.
Jgriffin, just hit another issue with this difference. We can't write a test the same for Travis or TBPL because the profile causes Gaia to behave differently.

Can I help with the proposal from comment #19 somehow?
Flags: needinfo?(jgriffin)
Zac, this config file will override the default gaia build options in TBPL, after I update the mozharness code (at http://hg.mozilla.org/build/mozharness/file/a9b8af62cf14/mozharness/mozilla/gaia.py#l138)
Attachment #8440795 - Flags: review?(zcampbell)
Flags: needinfo?(jgriffin)
..and the mozharness piece of this.
Attachment #8440804 - Flags: review?(zcampbell)
Assignee: nobody → jgriffin
Comment on attachment 8440804 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

I'm not familiar wiht the software package so I only feel comfortable giving f+!
Attachment #8440804 - Flags: review?(zcampbell) → feedback+
Comment on attachment 8440795 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20577

looks fine but should it not be in a specific folder or named appropriately to Gu?
So that there is no question about what test suite this config is for (as you cannot comment in json files, it needs to be intuitive).
Attachment #8440795 - Flags: review?(zcampbell) → review-
Actually this logic would apply to all Gaia tests in buildbot.  Do we want this only to apply to Gu?
I can't speak for the other test suites (and I wouldn't dare anyway!) however my intention with this change is to bring Gu into line with the way marionette js tests are run which is with DESKTOP_SHIMS=0.
Ok, I'll refactor this patch so that it can be applied per-suite.
Comment on attachment 8440795 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20577

PR updated, so re-flagging for review.
Attachment #8440795 - Flags: review- → review?(zcampbell)
Updated so that this can be applied on a per-suite basis, r=zac
Attachment #8441588 - Flags: review?(zcampbell)
Attachment #8440804 - Attachment is obsolete: true
Comment on attachment 8440795 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20577

r-, the folder path is gaia-unit-tests, should be gaia-ui-tests! :)
Attachment #8440795 - Flags: review?(zcampbell) → review-
Correctly targets gaia-ui-tests now
Attachment #8442210 - Flags: review?(zcampbell)
Attachment #8441588 - Attachment is obsolete: true
Attachment #8441588 - Flags: review?(zcampbell)
Comment on attachment 8440795 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20577

Moved build_config.json to gaia-ui-tests dir!
Attachment #8440795 - Flags: review- → review?
Comment on attachment 8440795 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20577

r+!
Attachment #8440795 - Flags: review? → review+
Comment on attachment 8442210 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

I don't think I'm the best person to review for mozharness but I am happy to f+ this.
Attachment #8442210 - Flags: review?(zcampbell) → feedback+
Comment on attachment 8442210 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

I'll pass the review to aki.
Attachment #8442210 - Flags: review?(aki)
Comment on attachment 8442210 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

>+        # if tbpl_build_config.json exists, load it
>+        if build_config_path:
>+            if os.path.exists(build_config_path):
>+                with open(build_config_path, 'r') as f:

Could be |with self.opened(build_config_path) as (f, err):|
Attachment #8442210 - Flags: review?(aki) → review+
Comment on attachment 8442210 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

https://hg.mozilla.org/build/mozharness/rev/8cec6396354b
Attachment #8442210 - Flags: checked-in+
Looks like this has been merged to production.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Verified this, thanks Jonathan!
Status: RESOLVED → VERIFIED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.