Update Gu job to build Gaia with DESKTOP_SHIMS=0

VERIFIED FIXED

Status

Release Engineering
General Automation
P1
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: zac, Assigned: jgriffin)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
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
Seems like this is a mozharness change (and so releng):
https://mxr.mozilla.org/build-central/search?string=DESKTOP_SHIMS&tree=build-central

https://hg.mozilla.org/build/mozharness/file/3ec16741615c/mozharness/mozilla/gaia.py#l145
Created attachment 8425398 [details] [diff] [review]
Update Gu job to build Gaia with DESKTOP_SHIMS disabled

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

Updated

3 years ago
Attachment #8425398 - Flags: review?(aki) → review+
Ty :-)

remote:   https://hg.mozilla.org/build/mozharness/rev/acb3e29f40cf
Something here went live today
Status: ASSIGNED → RESOLVED
Last Resolved: 3 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
(Reporter)

Comment 12

3 years ago
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)

Comment 14

3 years ago
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)
(Reporter)

Comment 15

3 years ago
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-
(Reporter)

Comment 16

3 years ago
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+

Comment 18

3 years ago
(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)
(Assignee)

Comment 19

3 years ago
We could probably make in-gaia config files ala linux_config.py that control how mozharness jobs make gaia.
(Reporter)

Comment 20

3 years ago
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)
(Assignee)

Comment 21

3 years ago
Created attachment 8440795 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20577

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)
(Assignee)

Comment 22

3 years ago
Created attachment 8440804 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

..and the mozharness piece of this.
Attachment #8440804 - Flags: review?(zcampbell)
(Assignee)

Updated

3 years ago
Assignee: nobody → jgriffin
(Reporter)

Comment 23

3 years ago
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+
(Reporter)

Comment 24

3 years ago
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-
(Assignee)

Comment 25

3 years ago
Actually this logic would apply to all Gaia tests in buildbot.  Do we want this only to apply to Gu?
(Reporter)

Comment 26

3 years ago
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.
(Assignee)

Comment 27

3 years ago
Ok, I'll refactor this patch so that it can be applied per-suite.
(Assignee)

Comment 28

3 years ago
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)
(Assignee)

Comment 29

3 years ago
Created attachment 8441588 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

Updated so that this can be applied on a per-suite basis, r=zac
Attachment #8441588 - Flags: review?(zcampbell)
(Assignee)

Updated

3 years ago
Attachment #8440804 - Attachment is obsolete: true
(Reporter)

Comment 30

3 years ago
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-
(Assignee)

Comment 31

3 years ago
Created attachment 8442210 [details] [diff] [review]
Use in-tree config to override gaia build opts, if it exists,

Correctly targets gaia-ui-tests now
Attachment #8442210 - Flags: review?(zcampbell)
(Assignee)

Updated

3 years ago
Attachment #8441588 - Attachment is obsolete: true
Attachment #8441588 - Flags: review?(zcampbell)
(Assignee)

Comment 32

3 years ago
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?
(Reporter)

Comment 33

3 years ago
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+
(Reporter)

Comment 34

3 years ago
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+
(Assignee)

Comment 35

3 years ago
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 36

3 years ago
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+
(Assignee)

Comment 37

3 years ago
Comment on attachment 8440795 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20577

https://github.com/mozilla-b2g/gaia/commit/a07855aedfe8fba42dac3c8506549870c89432c7
Attachment #8440795 - Flags: checked-in+
(Assignee)

Comment 38

3 years ago
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+
(Assignee)

Comment 39

3 years ago
Looks like this has been merged to production.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
(Reporter)

Comment 40

3 years ago
Verified this, thanks Jonathan!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.