An option to set the location of config files.

Review Request #3463 — Created Feb. 15, 2016 and discarded

benjyw
pants
2877
pants-reviews
kwlzn, mateor, patricklaw, stuhood

This option is special-cased, to work around the obvious
chicken-and-egg problem.

Why is this useful?

- Previously the default pants.ini location was automagically located
and used, which was potentially surprising to people trying to
override it. Now it's simply the default value of an option, which
everyone understands.

- 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.

CI away here: https://travis-ci.org/pantsbuild/pants/builds/109265229

  • 1
  • 0
  • 0
  • 0
  • 1
Description From Last Updated
def get_buildroot is currently defined in terms of a hardcoded lookup for a 'pants.ini' file, so I think there is ... ST stuhood
MA
  1. Count me among those who thought the required 'pants.ini' was surprising. This opens up perhaps more intuitive behavior for us, since we maintain a sub-monorepo with lots of inherited config. Thanks Benjy.

  2. This seems reasonable to me.

  3. Is this test showing that the flag overrides the environmental variable? PANTS_CONFIG isn't registered in the bootstrapper.

    1. It's testing a staticmethod that acts only on its arguments, so it's doing the right thing.

  4. 
      
ST
  1. Thanks a lot for doing this Benjy!

  2. src/python/pants/base/build_environment.py (Diff revision 1)
     
     
     
     

    def get_buildroot is currently defined in terms of a hardcoded lookup for a 'pants.ini' file, so I think there is a bit more to do here. As implemented in this review, --pants-config-files will only work if there is some pants.ini file in the search path used by get_buildroot.

    To get around that, you might want to add the buildroot location as another option? It could continue to default to the path of the pants.ini file as specified by --pants-config-files (with an error indicating that it needs to be specified explicitly if more than one have been specified?)

    1. Good catch. Not sure about the proposal to add a buildroot option, because where would you specify it? In the config file? Part of my reasoning behind this change is that it would be conceptually nice if Pants worked without any config file at all.

      Now I'm wondering (and this would have to be opened up to a wider audience) - is there any reason that the default buildroot isn't just the cwd? Currently we scan up the tree until we find a pants.ini, but a little testing shows that running pants in a subdir of the repo root fails in a variety of different ways depending on subtle conditions. Two I found in just 45 seconds are A) the dir you're in will be on the PYTHONPATH with unpredictable results, and B) cmd-line specs aren't interpreted correctly.

      So maybe the default buildroot (unless overridden by tests) is just cwd? The current logic has been there since the dawn of pants, but seems unused and incorrect to me.

      Thoughts before I open this up wider?

  3. 
      
BE
Review request changed

Status: Discarded

Loading...