Closed Bug 974777 Opened 9 years ago Closed 9 years ago

add --dump-config and --dump-config-hierarchy to mozharness


(Release Engineering :: Applications: MozharnessCore, defect)

Not set


(Not tracked)



(Reporter: jlund, Assigned: jlund)



(Whiteboard: [mozharness])


(2 files, 2 obsolete files)

This bug is to serve 3 goals:

1) It would be nice to be able to run any script where we can pass things like --dump-config and all the script will do is run BaseScript's dump_config() and then exit. This would be similar to --list-actions but for configs.

2) For more verbose detail, we should also be able to see which config files were used (if any!) and clearly outline where each item in self.config came from. This would only show the keys/values that made it to self.config and ignore ones that were overridden. Passing --dump-config-hierarchy does this: it outlines the hierarchy of how things were determined.
For example, if you supplied a default_config, more than one config file, and a few command line options, it would be nice to see which keys/values self.config was going to use and where they came from.

3) Finally, as it stands, a script can not control the way a list of config files make up self.config (in terms of hierarchy). Nor can a script control the way a config file updates self.config. For certain instances, this would be nice.
for example: in desktop builds, you can specify things like '--branch mozilla-central', '--build-pool production', or '--custom-build-variant asan'

these will add the appropriate config files based upon the arg passed. But if you pass '--branch' after '--build-pool', I still want the cfg coming from 'build-pool' to have higher precedence over the branch cfg. Further, '--branch' adds a file called '' to the config_files list. I don't want all of that cfg added to self.config: I only want the dict inside that matches 'mozilla-central'.

the addition of (3) adds some complexity if you choose to use it. The Firefox desktop build is using the flexibility but every other script should behave the exact same. (1) and (2) clears up some complexity by being explicit of what is going on. It also allows mozharness devs/users to see how self.config is being made up, hopefully making it easier to develop on.

for further details, see blog post: (note --list-config-files is now --dump-config and --dump-config-hierarchy, and much of the code examples have changed). 

Patches, nosetests, and ash results incoming.
I did a full push to ash since this patch touches every mozharness script.

Assuming ash is clean / close to m-c, we should have a good idea if this changes anything (which it shouldn't):

Following the results I'll request a r?
So the B2G ICS Emulator builds failed. My patch includes a dict comprehension. Turns out not all our slaves are running python 2.7+ (when dict comprehension syntax was introduced)

for reference, reopened:

new patch and test builds incoming.
Rail, you have reviewed much of mozharness. This is an internal patch that affects every script. However this should be a no-op change for any existing script.

I have other small patches from 'Bug 858797 - fx desktop builds in mozharness' which I think I'll defer to you for r? as well.

Once all of my script dependencies are landed, I'll be looking for a r? of my actual desktop build script. Unless you explicitly say you're happy to r? the main script as well, I'll try and find someone else who can review it so I am not dumping everything on you! :)
Attachment #8378814 - Attachment is obsolete: true
Attachment #8379254 - Flags: review?(rail)
Attachment #8378813 - Flags: review?(rail)
gazooks! I had two dict comprehensions in my earlier patch. the tbpl build failed again. But I patched it and this time it looks like they passed:

Also, my old patch for printing the env on run_command() with formatting / alignment, snuck its in here.

I like it better than pretty printing the env like we do now as I think it's more readable:

However it is not part of this bug so I'll remove it and add it to a different bug. Third time is a charm. sorry for the noise.
Attachment #8379254 - Attachment is obsolete: true
Attachment #8379254 - Flags: review?(rail)
Attachment #8379546 - Flags: review?(rail)
Comment on attachment 8379546 [details] [diff] [review]

Review of attachment 8379546 [details] [diff] [review]:

Attachment #8379546 - Flags: review?(rail) → review+
Attachment #8378813 - Flags: review?(rail) → review+
Comment on attachment 8378813 [details] [diff] [review]

Review of attachment 8378813 [details] [diff] [review]:

pushed to default:
Attachment #8378813 - Flags: checked-in+
Comment on attachment 8379546 [details] [diff] [review]

Review of attachment 8379546 [details] [diff] [review]:

pushed to default:
Attachment #8379546 - Flags: checked-in+
in production.
Closed: 9 years ago
Resolution: --- → FIXED
Component: General Automation → Mozharness
You need to log in before you can comment on or make changes to this bug.