Fish Trophy

benjyw got a fish trophy!

Remove all global config state.

Review Request #2222 — Created May 14, 2015 and submitted

benjyw
pants
1544
076f02b...
pants-reviews
jsirois, zundel

- No more config cache.
- No more global default bootstrap options, so also no dynamic
resetting of those...
- Removed many superfluous BUILD dependencies on the config library.
- test_gen_tasks_options_reference_data no longer broken.

There are now only 4 deps on config in the entire codebase:
1. The options system depends on it, of course.
2. The migrate config script.
3. Android keystore code, which uses it to reads its own config
(which has nothing to do with the main pants config).
4. Its unittest.

So this commit completes the process of switching everything over
to new-style options. It should now be possible (with maybe some
tweaking of defaults) to run pants with an empty pants.ini.
I have not tried this... :)

Actually, the above statement is not quite true: backends are still read
directly from config.

CI, amazingly, passed on the first attempt: https://travis-ci.org/pantsbuild/pants/builds/62586666

BE
ZU
  1. 
      
  2. src/python/pants/bin/goal_runner.py (Diff revision 1)
     
     

    Why are these still strictly config items instead of something like a bootstrap option?

    1. They should be, for sure. Next RB...

  3. 
      
ZU
  1. Definately deserving RB to be designated an auspicious number like #2222

  2. 
      
JS
  1. Yay!
  2. How about just call it yourself here?  I was going to ask for a doc comment to explain the state requirement, but I can't see why its actually needed.  If I guess right, you only did not s/get_bootstrap_options:Options/get_bootstrap_config:(Options, Sonfig)/ to cut down on api ripple.
    1. Ooops, I just pushed based on Eric's shipit. I'll address this in a follow-up commit if needed. I think I didn't call it here in order to force the user (myself, really) to reason about where exactly in the sequence each thing is called. I did consider having get_bootstrap_options() return the config as well, since post_bootstrap_config() is called in exactly one place, and I would be able to get rid of it entirely. But yeah, I figured it would be weird for the other handful of call sites of get_bootstrap_options() to have this rather superfluous config argument to ignore. Let me ruminate on this some more.

    2. Actually, now that I think of it, the right thing to do is make backends-packages and backends-plugins into options, and get rid of GoalRunner's need for a config reference entirely.

    3. I like it - fits Eric's observation.
  3. 
      
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as 9bd9f7fd6942526bcb69200376ff35426e61c6a3. Thanks Eric and John!

Loading...