1146894 - Clean up B2G desktop core configs in mozharness

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Michael Shal [:mshal]]

The B2G desktop configs in mozharness have a bit of cargo-culting. We should pull out any unnecessary/redundant config settings. Depends on 1146892, since that will probably help some of the non-default PATH settings, at least on Windows.

Comment 1

[Jordan Lund (:jlund)]

(In reply to Michael Shal [:mshal] from comment #0)
> The B2G desktop configs in mozharness have a bit of cargo-culting. We should
> pull out any unnecessary/redundant config settings. Depends on 1146892,
> since that will probably help some of the non-default PATH settings, at
> least on Windows.

+1 to this

Comment 2

[Michael Shal [:mshal]]

Looking at this further, I think the primary cargo-culting are the unused STAGE_USERNAME and STAGE_SSH_KEY variables. They can be removed from all the B2G desktop configs and fx desktop configs, as far as I can tell.

The rest of the config appears to be necessary, except for some of the variables set to default values. Do we want to remove those since they're the default, or leave them in to make them easier to change in the future? Here is the set of variables in the mac config that I think can be removed without affecting compilation (untested so far - just from looking at how they're used) -

-    # decides whether we want to use moz_sign_cmd in env
-    'enable_signing': False,
-    'purge_skip': ['info', 'rel-*:45d', 'tb-rel-*:45d'],
-    'purge_basedirs':  [],
-    'enable_count_ctors': False,
-    'enable_talos_sendchange': False,
-    'enable_unittest_sendchange': False,
-    'enable_max_vsize': False,

So do we want to leave eg: enable_unittest_sendchange in the config to make it obvious that it's available but disabled?

Comment 3

[Jordan Lund (:jlund)]

(In reply to Michael Shal [:mshal] from comment #2)
> Looking at this further, I think the primary cargo-culting are the unused
> STAGE_USERNAME and STAGE_SSH_KEY variables. They can be removed from all the
> B2G desktop configs and fx desktop configs, as far as I can tell.

cool, let's remove those for sure.

> -    # decides whether we want to use moz_sign_cmd in env
> -    'enable_signing': False,
> -    'purge_skip': ['info', 'rel-*:45d', 'tb-rel-*:45d'],
> -    'purge_basedirs':  [],
> -    'enable_count_ctors': False,
> -    'enable_talos_sendchange': False,
> -    'enable_unittest_sendchange': False,
> -    'enable_max_vsize': False,
> 
> So do we want to leave eg: enable_unittest_sendchange in the config to make
> it obvious that it's available but disabled?

the way I have it right now is:

if the config item is the same across all platforms of fx desktop builds, I put it in the default config:
   - http://mxr.mozilla.org/build/source/mozharness/scripts/fx_desktop_build.py#41

if an item differs on a per platform basis, I put the item in each platform config just to be explicit, even if not having it at all in certain platforms yields the same default effect (e.g. enable_max_vsize):
   - http://mxr.mozilla.org/build/source/mozharness/configs/b2g/desktop_macosx64.py#47
   - http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildbase.py#1596

iow - the items that have values but yield no affect on the job (e.g. ones that are False) but still have at least one True in another config, we should leave them in. there are pros and cons with this approach but I think it is nice to be explicit and declare items within each config at their respective config levels (script, platform, variant, branch, and build pool).

Maybe it be easier to have a quick chat if that's not making sense.

Comment 4

[Michael Shal [:mshal]]

I think this would bring the list of things to be removed down to:

'enable_signing': False,
'enable_count_ctors': False,
'enable_talos_sendchange': False, (though enable_unittest_sendchange differs among the b2g desktop platforms, so it might make sense to leave this in too)

Though note that B2GDesktopBuild inherits from FxDesktopBuild, so I don't know if there's a good way to overwrite the default config there. Any suggestions?

Comment 5

[Michael Shal [:mshal]]

Attachment: cleanup-configs

Comment 6

[Jordan Lund (:jlund)]

> Though note that B2GDesktopBuild inherits from FxDesktopBuild, so I don't
> know if there's a good way to overwrite the default config there. Any
> suggestions?

re-write the __init__ method?

Comment 7

[Michael Shal [:mshal]]

Comment on attachment 8590461 [details] [diff] [review]
cleanup-configs

https://hg.mozilla.org/build/mozharness/rev/9d4b7c66dcbd

Comment 8

[Release Engineering SlaveAPI Service]

mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/production