Closed Bug 1271342 Opened 8 years ago Closed 10 months ago

MH_CUSTOM_VARIANT_CFG appears to be broken with relative paths, rather than shortnames

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: mcomella, Unassigned)

References

(Blocks 1 open bug)

Details

I'm trying to change Android over to relative paths in bug 1270992 to avoid the confusing layer of indirection. However:

1) I pass in a full path, "builds/releng_sub_android_configs/64_lint.py", and get a KeyError [1][2]
2) I pass in a partial path to be filled, and an error is thrown because the file does not exist [3][4]

I think these are both bugs:
1) should not throw a KeyError because it is a valid path
2) should fill in the %s values in order to check if the path exists.

[1]: https://public-artifacts.taskcluster.net/c2NjOFpsTT-_qPHegh-4jQ/0/public/logs/live_backing.log
[2]: + python2.7 /home/worker/workspace/build/src/testing/mozharness/scripts/fx_desktop_build.py --config builds/releng_base_android_64_builds.py --config disable_signing.py --config platform_supports_post_upload_to_latest.py --custom-build-variant-cfg=builds/releng_sub_android_configs/64_lint.py --disable-mock --get-secrets --build --multi-l10n --update --log-level=debug --scm-level=1 --work-dir=/home/worker/workspace/build --branch=try --build-pool=taskcluster
23:35:20     INFO - MultiFileLogger online at 20160506 23:35:20 in /home/worker
Traceback (most recent call last):
  File "/home/worker/workspace/build/src/testing/mozharness/scripts/fx_desktop_build.py", line 172, in <module>
    fx_desktop_build = FxDesktopBuild()
  File "/home/worker/workspace/build/src/testing/mozharness/scripts/fx_desktop_build.py", line 104, in __init__
    super(FxDesktopBuild, self).__init__(**buildscript_kwargs)
  File "/home/worker/workspace/build/src/testing/mozharness/mozharness/mozilla/building/buildbase.py", line 592, in __init__
    super(BuildScript, self).__init__(**kwargs)
  File "/home/worker/workspace/build/src/testing/mozharness/mozharness/base/python.py", line 91, in __init__
    super(VirtualenvMixin, self).__init__(*args, **kwargs)
  File "/home/worker/workspace/build/src/testing/mozharness/mozharness/base/vcs/vcsbase.py", line 127, in __init__
    super(VCSScript, self).__init__(**kwargs)
  File "/home/worker/workspace/build/src/testing/mozharness/mozharness/base/script.py", line 1563, in __init__
    self._pre_config_lock(rw_config)
  File "/home/worker/workspace/build/src/testing/mozharness/scripts/fx_desktop_build.py", line 108, in _pre_config_lock
    super(FxDesktopBuild, self)._pre_config_lock(rw_config)
  File "/home/worker/workspace/build/src/testing/mozharness/mozharness/mozilla/building/buildbase.py", line 633, in _pre_config_lock
    variant_cfg = BuildOptionParser.build_variants[build_variant] % (
KeyError: 'builds/releng_sub_android_configs/64_lint.py'

[3]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3454c31759d1&selectedJob=20495881
[4]: + python2.7 /home/worker/workspace/build/src/testing/mozharness/scripts/fx_desktop_build.py --config builds/releng_base_android_64_builds.py --config disable_signing.py --config platform_supports_post_upload_to_latest.py --custom-build-variant-cfg=builds/releng_sub_%s_configs/%s_lint.py --disable-mock --get-secrets --build --multi-l10n --update --log-level=debug --scm-level=1 --work-dir=/home/worker/workspace/build --branch=try --build-pool=taskcluster
Whoops!
'--custom-build-variant' was passed but an appropriate config file could not be determined. Tried using: 'builds/releng_sub_%s_configs/%s_lint.py' but it was either not:
	-- a valid shortname: ['cross-debug', 'b2g-debug', 'graphene', 'code-coverage', 'x86', 'android-checkstyle', 'api-15-gradle-dependencies', 'api-15-partner-sample1', 'api-9', 'source', 'stat-and-debug', 'api-11-debug', 'asan-and-debug', 'api-15', 'api-11', 'android-test', 'mulet', 'api-9-debug', 'asan', 'api-11-partner-sample1', 'api-15-debug', 'tsan', 'horizon', 'debug', 'cross-opt'] 
	-- a valid path in ['.', '/home/worker/workspace/build/src/testing/mozharness/scripts/../configs', '/home/worker/workspace/build/src/testing/mozharness/scripts/../../configs'] 
	-- a valid variant for the given platform and bits.
yes, making short names was a mistake in hindsight. I thought it be a convenience but really, explicit is better than implicit and 99% of the time, we call mozharness scripts from automation so there is no convenience actually gained.


I think the long term fix would be the following:

1) remove the custom option parser that determines appropriate config files (based on short names, etc)
    - https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py?from=buildbase.py#328

2) remove the custom config generator that determines order and precedence of config files
     - https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py?from=buildbase.py#235

3) break up branch_specifics.py and build_pool_specifics.py into individual config files that are passed to script args
    - these live in: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/builds

4) use explicit full config file names:

     style example:
         `--cfg path/to/lowest_precedence_config.py ... --cfg path/to/highest_precedence_config.py`

     concrete example:

         instead of:
              `--cfg configs/builds/releng_base_linux_64_builds.py --custom-build-variant-cfg debug --branch date --build-pool production`

         we do:
              `--cfg configs/builds/releng_base_linux_64_builds.py --cfg configs/builds/configs/builds/releng_sub_linux_configs/64_debug.py --cfg configs/builds/mozilla-central.py --build-pool configs/builds/production.py`

5) fix every place we call mozharness build jobs in buildbot and taskcluster world



there is some work involved in that. We would have to figure out the actually net benefit for doing this or even part of this work. I'll do it if there is desire and motivation from others to do so. I wonder if we can poll interested people to see their thoughts.

the band aid fix is to play with the dials so that we can use `--custom-build-variant-cfg debug` and `--cfg configs/builds/configs/builds/releng_sub_linux_configs/64_debug.py` interchangeably without disrupting the option and config parser logic/precedence flow. this would be a simpler fix but won't clear up all the confusion and likely will only add more code complexity.
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.