An option to set the location of config files.

Review Request #3500 — Created Feb. 25, 2016 and submitted

benjyw
pants
pants-reviews
kwlzn, mateor, patricklaw, stuhood
This option is special-cased, to work around the obvious
chicken-and-egg problem.

Why is this useful? A few reasons.

- Previously the default pants.ini location was automagically used
  in several different possible control paths, which was potentially
  surprising to people trying to override it.

- It allows us to generate reference documentation without applying
  our pants.ini-based defaults, by setting the flag to an empty list.

- It allows us to remove the configpaths= constructor argument to
  OptionsBootstrapper, which was only used in tests anyway.

- In the future it will allow us to deprecate --config-override, and
  possibly the pantsrc-related options, and consolidate everything into
  the single --pants-config-files option.

ignore values from config and env vars when generating reference docs.

CI passes: https://travis-ci.org/pantsbuild/pants/builds/112340670

  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
ZU
  1. 
      
  2. If this is the case, I'm not very excited about the prospect of removing support for pantsrc_files. We have some of these set in pants.ini and developers aren't going to want to set them on the command line every time.

    pantsrc_files: [
        "squarepants/pants-jvm-platform-gen.ini",
        # For local developer settings. Not checked into git
        "pants-local.ini",
      ]
    

    If we put the item in our ./pants wrapper, this logic will only process the first instance of --pants-config-files, meaning to get everything to work, they'll have to re-specify these options to add any new items to --pants-config-files.

    1. Good point. We'll probably have to keep pantsrc as a separate thing. But at least we can get rid of --pants-config-override.

      Or-

      We could allow the config file to update the value of pants_config_override after bootstrapping but before loading the rest of the options. Thoughts? I'd rather do that in a separate change though.

    2. I'm on board with getting rid of --pants-config-override. We don't use this in pants.ini anywhere anyway.

    3. Added a comment about what would be required to deprecate the pantsrc options.

  3. 
      
KW
  1. 
      
  2. src/python/pants/option/config.py (Diff revision 1)
     
     
     

    any reason to not just return _EmptyConfig() here?

    didn't see any other uses of Config.empty() and seems simple enough to just inline for readability.

    1. No particular reason. Changed.

  3. src/python/pants/option/global_options.py (Diff revision 1)
     
     
     
     
     

    IIUC, wouldn't this mean that in order to logically append to the default config, that a user would have to know and provide the path to the default pants.ini as well?

    we have a few cases where we rely on --config-override to overlay sets of bundled changes on top of our default config. e.g.:

    --config-override=pants.ini.jdk8
    

    I think for these sorts of cases, the existing form of --config-override provides a far superior user experience than having to do this to achieve the same thing:

    --pants-config-files="['pants.ini', 'pants.ini.jdk8']"
    

    could we find a way to reconcile the idea of clobbering the config file sequence vs simply appending to it? seems like there are use cases for both.

    1. On slack there was discussion of allowing list-valued options to both append and override. We can hold off on deprecating --config-override until then. Added comment to that effect.

  4. 
      
ST
  1. 
      
  2. Would be good to put default in the name maybe.

    1. Good call, done.

  3. src/python/pants/option/config.py (Diff revision 1)
     
     

    Worth marking this explicitly AbstractClass? Took me a second to realize it's not concrete.

  4. src/python/pants/option/global_options.py (Diff revision 1)
     
     
     

    I'd be fine with starting a (long) deprecation cycle now... no time like the present.

    1. We'll need to wait until we have a way to both append and override list-valued flags.

  5. 
      
ZU
  1. 
      
  2. src/python/pants/option/global_options.py (Diff revision 1)
     
     
     

    remove aside about pantsrc files

    1. Added a separate TODO with what it would take to be able to deprecate the pantsrc flags.

  3. 
      
BE
BE
BE
ZU
  1. Ship It!
  2. 
      
KW
  1. Ship It!
  2. 
      
BE
  1. Thanks for the careful review cycle folks! Submitted as f6c4df9b9ca1f3f0d6ae428fb535413dff22c47b.

  2. 
      
BE
Review request changed

Status: Closed (submitted)

Change Summary:

f6c4df9b9ca1f3f0d6ae428fb535413dff22c47b

Loading...