Migrate BinaryUtil options to bootstrap options.

[kwlzn]

Problem

In order to fully externalize pantsd lifecycle, we need to untangle pantsd's reliance on Subsystem dependencies and their options. This is an initial step in that direction.

Solution

Migrate BinaryUtil options to bootstrap options.

Result

Able to leverage BinaryUtil before subsystems are setup by directly calling BinaryUtil.__init__ with global options.

[benjyw]

Presumably the Factory should no longer extend Subsystem, and should not have an options_scope member.

[kwlzn]

BinaryUtil still functions as a Subsystem in all the existing call sites - that part isn't changing, so it should still extend Subsystem afaict.

and removing the options_scope on the class produces the following traceback:

Exception caught: (<type 'exceptions.TypeError'>)
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 73, in <module>
    main()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 69, in main
    PantsLoader.run()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 65, in run
    cls.load_and_execute(entrypoint)
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 58, in load_and_execute
    entrypoint_main()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_exe.py", line 26, in main
    PantsRunner(exiter).run()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_runner.py", line 57, in run
    options_bootstrapper=options_bootstrapper)
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_runner.py", line 46, in _run
    return LocalPantsRunner(exiter, args, env, options_bootstrapper=options_bootstrapper).run()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/local_pants_runner.py", line 37, in run
    self._run()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/local_pants_runner.py", line 43, in _run
    options, build_config = OptionsInitializer(options_bootstrapper, exiter=self._exiter).setup()
  File "/Users/kwilson/dev/pants/src/python/pants/init/options_initializer.py", line 161, in setup
    options = self._install_options(self._options_bootstrapper, build_configuration)
  File "/Users/kwilson/dev/pants/src/python/pants/init/options_initializer.py", line 115, in _install_options
    known_scope_infos.append(subsystem.get_scope_info())
  File "/Users/kwilson/dev/pants/src/python/pants/subsystem/subsystem.py", line 80, in get_scope_info
    cls.validate_scope_name_component(cls.options_scope)
  File "/Users/kwilson/dev/pants/src/python/pants/option/optionable.py", line 39, in validate_scope_name_component
    if not cls.is_valid_scope_name_component(s):
  File "/Users/kwilson/dev/pants/src/python/pants/option/optionable.py", line 35, in is_valid_scope_name_component
    return cls._scope_name_component_re.match(s) is not None

Exception message: expected string or buffer

so I left it in + thought having an explicit namespace would help make it clearer which options in the bootstrap options were related. let me know if you see a better way.

[benjyw]

If it's a subsystem then it definitely needs an options_scope, but it seems like a better idea if it could not be one any more...

AFAICT the callsites do BinaryUtil.Factory.create() to get an instance, and that needn't change if we make Factory no longer extend Subsystem. Would that break in some way I'm not seeing?

[kwlzn]

Would that break in some way I'm not seeing?

yes, during the call to cls.global_instance().get_options() to get the bootstrap options - unless we want to explicitly thread an options context into every call.

BinaryUtil.Factory is also :API: Public and widely in use in private plugins afaik, so we couldn't change that directly without a new create method + a deprecation slog. I think it's still perfectly reasonable to continue to address BinaryUtil as a subsystem post-subsystem initialization. this change just permits us to utilize BinaryUtil prior to subsystem initialization with only bootstrap options.

[benjyw]

I just don't love that it has its own options scope, even though it's not allowed to have its own options. That seems dirty. Do we add a comment saying "do not register options here", or do we allow that, with the understanding that those options will only have their default values during initialization?

I suppose that's not horrible. We know exactly what we need BinaryUtil for during initialization, so it may have options in the future that are irrelevant to our needs, but relevant to those of tasks.

[kwlzn]

this seems like a bi-product of the Subsystem design tho - if Subsystem would relax that requirement, we could indeed avoid the (currently unused) options scope. right now, it's required - but doesn't seem strictly necessary (case in point).

will add a quick comment to clarify for now.

[kwlzn]

CI is timing out on integration test shard 1, but given the prior green CI + comment-only change I'm going to merge this under the assumption that the real problem is #4850