1013201 - Update Gu job to build Gaia with DESKTOP_SHIMS=0

Status: VERIFIED FIXED

(Release Engineering :: General, defect, P1)

Description

[Zac C (:zac)]

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.

Comment 1

[Ed Morley [:emorley]]

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.

Comment 2

[Ed Morley [:emorley]]

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

Comment 3

[Ed Morley [:emorley]]

Attachment: 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

Comment 4

[Ed Morley [:emorley]]

Ty :-)

remote:   https://hg.mozilla.org/build/mozharness/rev/acb3e29f40cf

Comment 5

[Justin Wood (:Callek)]

Something here went live today

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Ed Morley [:emorley]]

No idea what the pref change did, was just helping Zac out; punting to him

Comment 8

[Ed Morley [:emorley]]

Seems plausible though, given:
https://hg.mozilla.org/build/mozharness/rev/85bc563cec00

Zac, would something need uplifting?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Gu is green on b2g28 again post-backout.

Comment 11

[Ed Morley [:emorley]]

Zac, leaving this to you to figure out what needs uplifting etc.

Comment 12

[Zac C (:zac)]

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.

Comment 13

[Ed Morley [: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?

Comment 14

[Aki Sasaki (not active)]

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.

Comment 15

[Zac C (:zac)]

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?

Comment 16

[Zac C (:zac)]

Aki, ni? for comment #15

Comment 17

[Ed Morley [:emorley]]

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)

Comment 18

[Aki Sasaki (not active)]

(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.

Comment 19

[Jonathan Griffin (:jgriffin)]

We could probably make in-gaia config files ala linux_config.py that control how mozharness jobs make gaia.

Comment 20

[Zac C (:zac)]

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?

Comment 21

[Jonathan Griffin (:jgriffin)]

Attachment: 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)

Comment 22

[Jonathan Griffin (:jgriffin)]

Attachment: Use in-tree config to override gaia build opts, if it exists,

..and the mozharness piece of this.

Comment 23

[Zac C (:zac)]

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+!

Comment 24

[Zac C (:zac)]

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).

Comment 25

[Jonathan Griffin (:jgriffin)]

Actually this logic would apply to all Gaia tests in buildbot.  Do we want this only to apply to Gu?

Comment 26

[Zac C (:zac)]

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.

Comment 27

[Jonathan Griffin (:jgriffin)]

Ok, I'll refactor this patch so that it can be applied per-suite.

Comment 28

[Jonathan Griffin (:jgriffin)]

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.

Comment 29

[Jonathan Griffin (:jgriffin)]

Attachment: 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

Comment 30

[Zac C (:zac)]

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! :)

Comment 31

[Jonathan Griffin (:jgriffin)]

Attachment: Use in-tree config to override gaia build opts, if it exists,

Correctly targets gaia-ui-tests now

Comment 32

[Jonathan Griffin (:jgriffin)]

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!

Comment 33

[Zac C (:zac)]

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

r+!

Comment 34

[Zac C (:zac)]

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.

Comment 35

[Jonathan Griffin (:jgriffin)]

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.

Comment 36

[Aki Sasaki (not active)]

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):|

Comment 37

[Jonathan Griffin (:jgriffin)]

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

Comment 38

[Jonathan Griffin (:jgriffin)]

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

Comment 39

[Jonathan Griffin (:jgriffin)]

Looks like this has been merged to production.

Comment 40

[Zac C (:zac)]

Verified this, thanks Jonathan!