Closed
Bug 974777
Opened 11 years ago
Closed 11 years ago
add --dump-config and --dump-config-hierarchy to mozharness
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jlund, Assigned: jlund)
References
Details
(Whiteboard: [mozharness])
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
rail
:
review+
jlund
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
rail
:
review+
jlund
:
checked-in+
|
Details | Diff | Splinter Review |
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 'branch_specifics.py' 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: http://jordan-lund.ghost.io/diving-into-list-config-files/ (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.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
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):
https://tbpl.mozilla.org/?showall=all&tree=Ash&rev=a2fd0f69bd1b
Following the results I'll request a r?
Assignee | ||
Comment 4•11 years ago
|
||
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: https://bugzilla.mozilla.org/show_bug.cgi?id=602908#c61
new patch and test builds incoming.
Assignee | ||
Comment 5•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Attachment #8378813 -
Flags: review?(rail)
Assignee | ||
Comment 6•11 years ago
|
||
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:
https://tbpl.mozilla.org/?showall=all&tree=Ash&jobname=b2g_ash_emulator_dep
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:
https://pastebin.mozilla.org/4351244
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 7•11 years ago
|
||
Comment on attachment 8379546 [details] [diff] [review]
974777_add_dump_config_options-mh-200214-2.diff
Review of attachment 8379546 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM
Attachment #8379546 -
Flags: review?(rail) → review+
Updated•11 years ago
|
Attachment #8378813 -
Flags: review?(rail) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 8378813 [details] [diff] [review]
974777_add_dump_config_options-nosetests-mh-190214.diff
Review of attachment 8378813 [details] [diff] [review]:
-----------------------------------------------------------------
pushed to default: http://hg.mozilla.org/build/mozharness/rev/d4e5e9d4118a
Attachment #8378813 -
Flags: checked-in+
Assignee | ||
Comment 9•11 years ago
|
||
Comment on attachment 8379546 [details] [diff] [review]
974777_add_dump_config_options-mh-200214-2.diff
Review of attachment 8379546 [details] [diff] [review]:
-----------------------------------------------------------------
pushed to default: http://hg.mozilla.org/build/mozharness/rev/d4e5e9d4118a
Attachment #8379546 -
Flags: checked-in+
Comment 10•11 years ago
|
||
in production.
Assignee | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Component: General Automation → Mozharness
You need to log in
before you can comment on or make changes to this bug.
Description
•