Fixup CppToolchain to be lazy and actually cache.

Review Request #1850 — Created March 2, 2015 and submitted

John Sirois
pants
jsirois/cpp/fix_toolchain
1189
c2930bd...
pants-reviews
dturner-tw
Previously the c++ compiler was resolved in the constructor.  Doing this
work in the constructor made every indirect user pay the price with the
concrete example being the test_cpp_toolchain.py unit tests which would
fail if run without CXX exported.

Also, only the compiler tool was cached, other tool lookups via
register_tool populated but did not use the cache.  Fixup register_tool
to fully self-contain its caching behavior.

Finally, fixup test_cpp_toolchain.py to be unit-friendly and run in the
absence of CXX being exported and also tighten up
test_cpp_integration.py exception trapping.

 contrib/cpp/src/python/pants/contrib/cpp/toolchain/cpp_toolchain.py     | 41 +++++++++++++++++++++++------------------
 contrib/cpp/tests/python/pants_test/contrib/cpp/test_cpp_integration.py |  6 +++---
 contrib/cpp/tests/python/pants_test/contrib/cpp/test_cpp_toolchain.py   | 40 +++++++++++++++++++++++-----------------
 3 files changed, 49 insertions(+), 38 deletions(-)
Previously this failed but now passes:
```console
$ PANTS_DEV=1 ./pants test contrib/cpp/::
```

And this still works, with no skips:
```console
$ CXX=g++ PANTS_DEV=1 ./pants test contrib/cpp/::
```

CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/52806157
  • 0
  • 0
  • 1
  • 1
  • 2
Description From Last Updated
Dominic Hamon
  1. 
      
  2. can this overwrite tools now? maybe it's worth erroring if we already have a tool with 'name' in the cache.

    1. It can't, since the caching is now fully contained to this method, the short circuit check against _validated_tools up above on lines 42-44 guards over-write.  Each tool(name) is populated only once lazily.
  3. pulling the basename out of the path is harder to reason about than just using 'good-tool' directly.

  4. 
      
John Sirois
Dominic Hamon
  1. Ship It!
  2. 
      
John Sirois
John Sirois
Review request changed

Status: Closed (submitted)

David Turner
  1. Ship It!
  2. 
      
Loading...