Fixup Config to find DEFAULT values for missing sections.

Review Request #1851 — Created March 3, 2015 and submitted

jsirois
pants
jsirois/issues/1181/config
1181, 1190
1852
6849746...
pants-reviews
benjyw, peiyu, zundel
This supports the spirit of the `Config.get` and `Options` docs.
Previously the DEFAULT section was only consulted when the section
being queried existed but had no option defined.  Now the DEFAULT
section is also consulted when the section being queried is not
defined at all.  This should support more less brittle configs that
can have global defaults defined for all current and future scopes.

 src/python/pants/base/config.py                        | 29 +++++++++++++++++------------
 tests/python/pants_test/base/BUILD                     | 10 ++++++++++
 tests/python/pants_test/{tasks => base}/test_config.py | 14 +++++++++++---
 3 files changed, 38 insertions(+), 15 deletions(-)
CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/52814067
JS
JS
ZU
  1. 
      
  2. src/python/pants/base/config.py (Diff revision 1)
     
     

    his gives us 2 levels - task level and default. But the hierarchy of parsers is sometimes deeper than that. For example, if the option is registered in [compile.java] it should also be accessible in scope [compile]

    See python/pants/option/parser_hierarchy.py

     p self.options._parser_hierarchy._parser_by_scope.keys()
    [u'', u'doc.scaladoc', u'compile.python-eval', u'compile-changed', u'compile.scala', u'run-dirty.jvm-dirty', u'roots', u'test.run_prep_command', u'jar', u'bench', u'clean-all', u'test-changed', u'compile.scalastyle', u'resources', u'setup-py', u'repl', u'sitegen', u'gen', u'gen.ragel', u'resolve', u'filemap', u'repl.scala', u'gen.protoc', u'deferred-sources', u'repl.py', u'server', u'binary.python-binary-create', u'list', u'idea', u'bundle.dup', u'targets', u'run.jvm', u'repl-dirty.scala-dirty', u'gen.aapt', u'publish', u'test.pytest', u'test.specs', u'invalidate.ng-killall', u'provides', u'bootstrap.bootstrap-jvm-tools', u'run', u'doc.javadoc', u'dependencies', u'path', u'resources.prepare', u'changed', u'filter', u'killserver', u'clean-all.ng-killall', u'invalidate', u'markdown', u'binary.cpp-library', u'sign', u'apk', u'depmap', u'unpack-jars', u'clean-all.invalidate', u'gen.wire', u'detect-duplicates', u'run.py', u'gen.scrooge', u'sort', u'repl-dirty', u'builddict', u'goals', u'clean-all-async', u'compile.java', u'eclipse', u'compile', u'check_published_deps', u'thrift-linter', u'gen.thrift', u'run.cpp-run', u'gen.antlr', u'imports', u'run-dirty', u'ng-killall', u'binary.dex', u'filedeps', u'binary', u'imports.ivy-imports', u'clean-all-async.ng-killall', u'compile.checkstyle', u'compile.apt', u'ensime', u'pathdeps', u'test', u'gen.jaxb', u'clean-all-async.invalidate', u'resolve.ivy', u'binary.dup', u'paths', u'bundle', u'bundle.zipalign', u'run-tracker', u'minimize', u'doc', u'bootstrap', u'test.junit', u'binary.cpp-binary', u'dependees']
    
    1. Sure - can I beg off not fixing that in this review though?  This addresses only the current oddity that DEFAULT is only consulted for sections (scopes) defined in the config files.
      IOW it fixes Config - which is defined seperate from the options system and which locally (arguably) has not met its doc'd contract since inception in 2012.
      
      This issue you bring up does not bear immediately on 1181 which this aims to fix and is a longstanding issue (since options was introduced).
    2. Maybe add a TODO if you think its appropriate in one of these files.

    3. Note to self - the fix you're after will require exposing a helper like `Config.has_explicit(section, option)` and then use of this in the Parser hierarchy from the options system to implement riffling up through default layers and only consulting DEFAULT manually and last.
    4. I'll highlight here that Config does not work like the rest of the options system wrt scopes since a non global scope option can be set in the global (DEFAULT) scope.  Benjy noted this in the Options doc, but it is odd and makes some configuration un-intuitive.  For example I can globally turn off nailguns (a leaf - non-global flag) in a Config DEFAULT section, but I can't do so via env var or flag.
    5. A TODO is not appropriate in these files since the concern is an options system concern, but I've filed this issue against myself: https://github.com/pantsbuild/pants/issues/1192
  3. 
      
ZU
  1. Ship It!
  2. 
      
PE
  1. Ship It!
  2. 
      
JS
  1. Thanks guys - I'm going to wait on Benjy for this one since it touches on options semantics territory as a side-effect.
  2. 
      
JS
JS
  1. Benjy - are you ok with this?  I'd like to submit if so, but as noted Config is weird when used from the options subsystem due to its internal scoping mechanism.
  2. 
      
BE
  1. =john, this needed fixing completely regardless of the new options system, or scopes, or tasks or anything.

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

    I've seen 'typ' used in such scenarios.

  3. 
      
JS
  1. Thanks folks - submitted @ https://github.com/pantsbuild/pants/commit/8a61b1a2b6e87022147fd90f200a0ac9cb3e9c92
  2. 
      
JS
Review request changed

Status: Closed (submitted)

MA
  1. This commit has broken dx_compile out of the android system...but maybe only on my machine since it this same sequence is in the integration tests that are passing on Travis? The nailgun is timing out with a trace of:

    ```
    Invalidated 1 target..DEBUG] No ng server found with fingerprint 20454a8b7bbd843aa192eb06e749af2eaeded417, spawning...
    DEBUG] Executing: /Library/Java/JavaVirtualMachines/jdk1.7.0_65.jdk/Contents/Home/bin/java ["-Xmx1g", "-XX:MaxPermSize=256m"] -Dpants.buildroot=/Users/mateor/development/pants -Dpants.nailgun.owner=/Users/mateor/development/pants/.pants.d/ng/DxCompile_binary_dex -Dpants.nailgun.fingerprint=20454a8b7bbd843aa192eb06e749af2eaeded417 -cp .pants.d/ivy/jars/com.martiansoftware/nailgun-server/jars/nailgun-server-0.9.1.jar:../android-sdk-macosx/build-tools/19.1.0/lib/dx.jar com.martiansoftware.nailgun.NGServer :0 args={'close_fds': True, 'stdin': <open file u'/dev/null', mode 'r' at 0x109cfb5d0>, 'stderr': <open file u'/Users/mateor/development/pants/.pants.d/ng/DxCompile_binary_dex/stderr', mode 'w' at 0x109cfb6f0>, 'stdout': <open file u'/Users/mateor/development/pants/.pants.d/ng/DxCompile_binary_dex/stdout', mode 'w' at 0x109cfb660>} at cwd=/Users/mateor/development/pants
    DEBUG] Spawned ng server with fingerprint 20454a8b7bbd843aa192eb06e749af2eaeded417 @ 97129

                       ==== stderr ====
    
                       ==== stdout ====
    
               Waiting for background workers to finish.
               FAILURE
    

    Exception caught:
    File "/Users/mateor/development/pants/src/python/pants/bin/pants_exe.py", line 86, in <module>
    main()
    File "/Users/mateor/development/pants/src/python/pants/bin/pants_exe.py", line 81, in main
    _run(exiter)
    File "/Users/mateor/development/pants/src/python/pants/bin/pants_exe.py", line 74, in _run
    result = goal_runner.run()
    File "/Users/mateor/development/pants/src/python/pants/bin/goal_runner.py", line 174, in run
    result = self._do_run()
    File "/Users/mateor/development/pants/src/python/pants/bin/goal_runner.py", line 230, in _do_run
    return engine.execute(context, self.goals)
    File "/Users/mateor/development/pants/src/python/pants/engine/engine.py", line 26, in execute
    self.attempt(context, goals)
    File "/Users/mateor/development/pants/src/python/pants/engine/round_engine.py", line 212, in attempt
    goal_executor.attempt(explain)
    File "/Users/mateor/development/pants/src/python/pants/engine/round_engine.py", line 45, in attempt
    task.execute()
    File "/Users/mateor/development/pants/src/python/pants/backend/android/tasks/dx_compile.py", line 109, in execute
    self._compile_dex(args, target.build_tools_version)
    File "/Users/mateor/development/pants/src/python/pants/backend/android/tasks/dx_compile.py", line 79, in _compile_dex
    args=args, workunit_name='dx')
    File "/Users/mateor/development/pants/src/python/pants/backend/jvm/tasks/nailgun_task.py", line 89, in runjava
    workunit_labels=workunit_labels)
    File "/Users/mateor/development/pants/src/python/pants/java/util.py", line 46, in execute_java
    cwd=cwd)
    File "/Users/mateor/development/pants/src/python/pants/java/util.py", line 77, in execute_runner
    ret = runner.run(stdout=workunit.output('stdout'), stderr=workunit.output('stderr'), cwd=cwd)
    File "/Users/mateor/development/pants/src/python/pants/java/nailgun_executor.py", line 183, in run
    nailgun = self._get_nailgun_client(jvm_options, classpath, stdout, stderr)
    File "/Users/mateor/development/pants/src/python/pants/java/nailgun_executor.py", line 225, in _get_nailgun_client
    return self._spawn_nailgun_server(new_fingerprint, jvm_options, classpath, stdout, stderr)
    File "/Users/mateor/development/pants/src/python/pants/java/nailgun_executor.py", line 295, in _spawn_nailgun_server
    .format(jvm_options=jvm_options, classpath=classpath))
    File "/Users/mateor/development/pants/src/python/pants/java/nailgun_executor.py", line 259, in _await_nailgun_server
    .format(sec=nailgun_timeout_seconds, desc=debug_desc))

    Exception message: Failed to read ng output after 10 seconds.
    jvm_options=['["-Xmx1g", "-XX:MaxPermSize=256m"]'] classpath=['/Users/mateor/development/pants/.pants.d/ivy/jars/com.martiansoftware/nailgun-server/jars/nailgun-server-0.9.1.jar', u'/Users/mateor/development/android-sdk-macosx/build-tools/19.1.0/lib/dx.jar']
    ```

    I am going to try and dig deeper but thought I would see if you had any potential side-effects already in mind.

    If you are unfamiliar, dx_compile is a runjava() command that compiles java classes into a dex archive (similar to a jar).
    I have verified that reverting this commit allows it to run as before.

    1. Oops, Travis isn't running the integration tests. So this is likely a global issue. I will let you know
      what I find.

    2. Stuck nailgun servers happen. How did you determine that this was the change that broke you? Without other evidence, its possible, but not likely that this change is to blame.

      1) Try turning off nailgun by --no-use-nailgun or use_naligun: False in your pants.ini for this task.

      2) You may be running out of system memory. Kill off any unused nailgun servers.

    3. Hmmm...that last suggestion has some glimmer of possibility. I have been adding android_library support off in another branch and have lots of stalled builds as a result. Let me do some experiments, thanks Eric.

    4. I determined that this was the commit by bisecting the history. Once I found the problem started here, I reverted this commit and then saw it work as before.

    5. Some fishing lessons:

      jsirois@gill ~/dev/3rdparty/pants (master) $ git log --oneline -1
      4bef68c Shard out OSX CI.
      jsirois@gill ~/dev/3rdparty/pants (master) $ git clean -fdx && ./pants clean-all
      jsirois@gill ~/dev/3rdparty/pants (master) $ ./pants binary examples/src/android/::
      ...
      16:26:50 00:03   [binary]
      16:26:50 00:03     [python-binary-create]
      16:26:50 00:03     [binary]
      16:26:50 00:03     [dup]
      16:26:50 00:03     [dex]
      16:26:50 00:03       [dx-compile]
                           Invalidated 1 target..
                             ==== stderr ====
      
                             ==== stdout ====
      
                     Waiting for background workers to finish.
                     FAILURE
      
      Exception caught:
        File "/home/jsirois/dev/3rdparty/pants/pants.pex/.bootstrap/_pex/pex.py", line 271, in execute
          self.execute_entry(entry_point, args)
        File "/home/jsirois/dev/3rdparty/pants/pants.pex/.bootstrap/_pex/pex.py", line 320, in execute_entry
          runner(entry_point)
        File "/home/jsirois/dev/3rdparty/pants/pants.pex/.bootstrap/_pex/pex.py", line 343, in execute_pkg_resources
          runner()
        File "pants/bin/pants_exe.py", line 81, in main
        File "pants/bin/pants_exe.py", line 74, in _run
        File "pants/bin/goal_runner.py", line 174, in run
        File "pants/bin/goal_runner.py", line 230, in _do_run
        File "pants/engine/engine.py", line 26, in execute
        File "pants/engine/round_engine.py", line 212, in attempt
        File "pants/engine/round_engine.py", line 45, in attempt
        File "pants/backend/android/tasks/dx_compile.py", line 109, in execute
        File "pants/backend/android/tasks/dx_compile.py", line 79, in _compile_dex
        File "pants/backend/jvm/tasks/nailgun_task.py", line 89, in runjava
        File "pants/java/util.py", line 46, in execute_java
        File "pants/java/util.py", line 77, in execute_runner
        File "pants/java/nailgun_executor.py", line 183, in run
        File "pants/java/nailgun_executor.py", line 225, in _get_nailgun_client
        File "pants/java/nailgun_executor.py", line 295, in _spawn_nailgun_server
        File "pants/java/nailgun_executor.py", line 259, in _await_nailgun_server
      
      Exception message: Failed to read ng output after 10 seconds.
       jvm_options=['["-Xmx1g", "-XX:MaxPermSize=256m"]'] classpath=['/home/jsirois/dev/3rdparty/pants/.pants.d/ivy/jars/com.martiansoftware/nailgun-server/jars/nailgun-server-0.9.1.jar', u'/opt/android-sdk-linux/build-tools/19.1.0/lib/dx.jar']
      
      ^jsirois@gill ~/dev/3rdparty/pants (master) $ ls -l /opt/android-sdk-linux/build-tools/19.1.0/lib/dx.jar
      -rw-r--r-- 1 jsirois docker 901345 Nov 30 23:49 /opt/android-sdk-linux/build-tools/19.1.0/lib/dx.jar
      jsirois@gill ~/dev/3rdparty/pants (master) $ cat .pants.d/ng/
      Checkstyle_compile_checkstyle/ DxCompile_binary_dex/          IvyResolve_resolve_ivy/        JavaCompile/                   
      jsirois@gill ~/dev/3rdparty/pants (master) $ cat .pants.d/ng/DxCompile_binary_dex/std
      stderr  stdout  
      jsirois@gill ~/dev/3rdparty/pants (master) $ cat .pants.d/ng/DxCompile_binary_dex/stderr 
      Error: Could not find or load main class ["-Xmx1g", "-XX:MaxPermSize=256m"]```
      

      So thats an odd main class!
      Re-running with debug info:

      jsirois@gill ~/dev/3rdparty/pants (master) $ ./pants -ldebug clean-all binary examples/src/android/::
      ...
      16:29:20 00:03   [binary]
      16:29:20 00:03     [python-binary-create]
      16:29:20 00:03     [binary]
      16:29:20 00:03     [dup]
      16:29:20 00:03     [dex]
      16:29:20 00:03       [dx-compile]
                           Invalidated 1 target..DEBUG] No ng server found with fingerprint 6f6ac95bc21d7ee2447b883c884814f6eb74f46f, spawning...
      DEBUG] Executing: /opt/jdk/bin/java ["-Xmx1g", "-XX:MaxPermSize=256m"] -Dpants.buildroot=/home/jsirois/dev/3rdparty/pants -Dpants.nailgun.owner=/home/jsirois/dev/3rdparty/pants/.pants.d/ng/DxCompile_binary_dex -Dpants.nailgun.fingerprint=6f6ac95bc21d7ee2447b883c884814f6eb74f46f -cp .pants.d/ivy/jars/com.martiansoftware/nailgun-server/jars/nailgun-server-0.9.1.jar:../../../../../opt/android-sdk-linux/build-tools/19.1.0/lib/dx.jar com.martiansoftware.nailgun.NGServer :0 args={'close_fds': True, 'stdin': <open file u'/dev/null', mode 'r' at 0x7f44602e34b0>, 'stderr': <open file u'/home/jsirois/dev/3rdparty/pants/.pants.d/ng/DxCompile_binary_dex/stderr', mode 'w' at 0x7f44602e35d0>, 'stdout': <open file u'/home/jsirois/dev/3rdparty/pants/.pants.d/ng/DxCompile_binary_dex/stdout', mode 'w' at 0x7f44602e3540>} at cwd=/home/jsirois/dev/3rdparty/pants
      

      So clearly the 1st arg to java of ["-Xmx1g", "-XX:MaxPermSize=256m"] is AFU. And DxCompile defines a jvm_options that is a string (default option type) unlike other tasks jvm_options which are all lists. There is a DEFAULT jvm_options defined for a list and DxCompile parses it as a string.

    6. Looks like good detective work to me. I am at a thing until a little later tonight but I will give this a try. Thanks for taking a look!

    7. Gonna try and see if I can't get that stderr/stdout to screen while I am at it. I had several different pain points from getting no feedback from DxCompile this week.

    8. I've got the DxCompile fix posting shortly since I broke the build here.  I'll leave the NG convenience to you - it sounds like a good idea to me.
    9. That is an academic interpretation of breaking the build but I'll take it! I want to add the SDK to Travis but I haven't quite uncovered how to integrate it to the container model. But this should be covered by the ci before the end of March. https://github.com/pantsbuild/pants/issues/937

    10. https://rbcommons.com/s/twitter/r/1878/
    11. As for getting anything on travis, you can run arbitrary commands in containers as not-root (curls, untars, etc.).  So if you can setup an anroid env on your machine as not-root, you can wrap the steps in a script and have the ci.sh run that for the step that runs android tests (it would probably be speed-onerous to do this on each shard).
    12. Okay, great. I have done this in the past, something like this: https://github.com/mateor/HotSwapper/blob/master/hotswapper#L96 but I would probably use curl nowadays. I better get back to this thing I am at, thanks for showing me the process along with the fix.

  2. 
      
Loading...