Refactor options bootstrapping.

Review Request #1453 — Created Dec. 9, 2014 and submitted

jsirois, patricklaw, zundel

Instead of free-floating functions, the bootstrapping is now
encapsulated in a bootstrapper class. This class exposes both the
bootstrap and full options.

This is necessary to support the startup sequence in properly:

- To create full options we need the known scopes.
- To get the known scopes we need to load plugins and extensions.
- The plugin and extension registration code may use config.

Therefore we need a way to load the bootstrap config, then do some work,
then load the full options.

Currently we achieve this with a gross manual config-reading hack in
This change will allow us to get rid of that hack.

This change also fixes an issue where in tests the buildroot wasn't plumbed in
everywhere it should be.

Unit tests pass. Integration tests pass.

  2. If you extract a bootstrapper() function you get DRY + a bonus of no continuation needed.

  3. I prefer "state transitions return new types" - aka stateless objects. It can be more ceremony though and the OptionsBootstrapper has narrow use so this is probably the way to go ... aka talking out loud.

    1. Yeah, since this was only a local issue, I decided to keep it simple and comment it.

  4. How about nix the continuation, lift the constructor to this line and break inside it's arg list.

  2. src/python/pants/backend/core/tasks/ (Diff revisions 1 - 2)

    2 blank lines

Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as a3b2f2ab21d1d16e58213973e59049d755374db1.
  1. This may be coincidental, but our builds have been red since this landed:
    python3.2 has crept back in to the interpreters being selected from raising errors for unicode literals. The --interpreter arguments used by the script are global options which are tied up in the bits this RB touched so I'm leaning towards not coincidence.

    1. I think the "<buildroot>" here is text intended for use in the online help. I ?think? we don't want to use it as part of a real path.

           **** Failed to install wheel-0.23.0. stderr:
                         File "<stdin>", line 2
                           sys.path.insert(0, u'/home/travis/build/pantsbuild/pants/<buildroot>/.pants.d/python/interpreters/CPython-3.2.5/setuptools-5.4.1-py3.2.egg'); import setuptools
                       SyntaxError: invalid syntax
    2. ...but I don't see "<buildroot>" in many places in the log, so this is probably not the real problem. Sorry for the noise.

    3. No - I think it is. A slight bit of handwave but see here: and note that the bootstrapping code sets cached Config. If the cached config used by the pants test runner were not to have the interpreter constraints present in pants.ini - we'd pick up python3.2.

      Thanks for looking! I'll try a quick adjustment to to pass explicit --interpreter flags to the test run like it used to to work around - If that gets things green - this code can be revisited if it even needs to be - the caching is a temp hack as I recall.

    4. Still fuzzy on this but I have a fix in unrelated code I'm working on.

    5. Still under CI - but

      Sorry for the trigger finger suspecting this RB. I'm still not clear why this popped up for this review.

  2. This was the problem. OptionsBootstrapper mutates global Config._default values in its constructor and unlike the BootstrapOptionsTest this mutation was not reset. I have a fix coming shortly.