Simplify ScroogeGen.

Review Request #1494 — Created Dec. 17, 2014 and submitted

benjyw
pants
4c4957d...
pants-reviews
dturner-tw, ity, jsirois, tejal

It used to support multiple compilers, but no longer needs to.
This change implements a TODO to rip out that support.

This will make a future change (to switch tool registration to use
the options system) easier.

Ran the scrooge unittest.

Test coverage of scrooge is slim, so I'd appreciate folks at Twitter patching this in and taking it for a spin. Thanks!

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
DT
  1. 
      
  2. repl.scala doesn't look right here.

    1. Ooops! Copy-paste mishap.

  3. 
      
BE
JI
  1. 
      
  2. s/ini_section/_CONFIG_SECTION/

  3. s/'scrooge-gen'/_CONFIG_SECTION/

  4. s/'scrooge-gen'/_CONFIG_SECTION/

  5. seems like using named params was the recommended way for string formatting from past rb's.

    1. That is useful when the format string is complicated, with several parameters, but it can also be unnecessarily verbose and onerous. So I prefer to leave this to individual judgement.

  6. s/'scrooge-gen'/_CONFIG_SECTION/

  7. ditto for named param

  8. 
      
JS
  1. 
      
  2. output_style is enough now, 'scrooge-gen/' can be omitted

  3. A tuple would be better (more minimal) here.

  4. 
      
BE
JS
  1. Ship It!

  2. 
      
IT
  1. Ship It!

  2. 
      
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as 47ee133566c5a6eb2e34c7bc64a3801ee72a5a0f.
JS
  1. We reviewed this poorly - looks like it caused a real CI break, for example: https://travis-ci.org/pantsbuild/pants/jobs/44389076
    ``console
    ==================== FAILURES ====================
    EclipseIntegrationTest.test_eclipse_on_thriftdeptest

                     self = <pants_test.tasks.test_eclipse_integration.EclipseIntegrationTest testMethod=test_eclipse_on_thriftdeptest>
    
                         def test_eclipse_on_thriftdeptest(self):
                     >     self._eclipse_test(['testprojects/src/java/com/pants/testproject/thriftdeptest::'])
    
                     tests/python/pants_test/tasks/test_eclipse_integration.py:63: 
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                     tests/python/pants_test/tasks/test_eclipse_integration.py:23: in _eclipse_test
                         self.assert_success(pants_run)
                     tests/python/pants_test/pants_run_integration_test.py:136: in assert_success
                         self.assert_result(pants_run, self.PANTS_SUCCESS_CODE, expected=True, msg=msg)
                     tests/python/pants_test/pants_run_integration_test.py:159: in assert_result
                         assertion(value, pants_run.returncode, error_msg)
                     E   AssertionError: 0 != 1 : /home/travis/build/pantsbuild/pants/pants --no-lock --kill-nailguns --no-pantsrc --print-exception-stacktrace eclipse --project-dir=.pants.d/tmp/test-eclipse/tmpCvXjKR testprojects/src/java/com/pants/testproject/thriftdeptest::
                     E   returncode: 1
                     E   stdout:
                     E      
                     E      22:30:35 00:00 [main]
                     E                     (To run a reporting server: ./pants server)
                     E      22:30:35 00:00   [bootstrap]
                     E      22:30:35 00:00   [setup]
                     E      22:30:35 00:00     [parse]
                     E                     Executing tasks in goals: bootstrap -> imports -> gen -> check-exclusives -> resolve -> eclipse
                     E      22:30:36 00:01   [bootstrap]
                     E      22:30:36 00:01     [bootstrap-jvm-tools]
                     E      22:30:36 00:01   [imports]
                     E      22:30:36 00:01     [ivy-imports]
                     E      22:30:36 00:01   [gen]
                     E      22:30:36 00:01     [thrift]
                     E      22:30:36 00:01     [scrooge]
                     E                         Invalidated 1 target containing 1 payload file.
                     E                     FAILURE
                     E   stderr:
                     E      Traceback (most recent call last):
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/.bootstrap/_pex/pex.py", line 272, in execute
                     E          self.execute_entry(entry_point, args)
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/.bootstrap/_pex/pex.py", line 320, in execute_entry
                     E          runner(entry_point)
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/.bootstrap/_pex/pex.py", line 343, in execute_pkg_resources
                     E          runner()
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/bin/pants_exe.py", line 66, in main
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/bin/pants_exe.py", line 61, in _run
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/bin/goal_runner.py", line 161, in run
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/bin/goal_runner.py", line 277, in _do_run
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/engine/engine.py", line 48, in execute
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/engine/round_engine.py", line 185, in attempt
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/engine/round_engine.py", line 42, in attempt
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/backend/codegen/tasks/scrooge_gen.py", line 117, in execute
                     E        File "/home/travis/build/pantsbuild/pants/pants.pex/pants/backend/codegen/tasks/scrooge_gen.py", line 139, in gen
                     E      AttributeError: 'PartialCmd' object has no attribute 'compiler'
                      1 failed, 31 passed, 2 skipped in 530.26 seconds
    

    ```

    1. Grr. Reverted for now.

    2. Ooops, this was a stray line that should have been deleted. Fixed and pushed again.

  2. 
      
JS
  1. 
      
  2. This check is still needed, it skips, for example:

    java_thrift_library(..., compiler='thrift', ...)
    

    Where 'thrift' means the Apache thrift compiler.
    The predicate only selects java_thrift_librarys whose compiler is 'scrooge' when the check is working.

    This is cause dup-gen and this CI error: https://travis-ci.org/pantsbuild/pants/jobs/44396603

    23:56:16 01:07         [compile]
    23:56:16 01:07           [jmake]
                             Jmake version 1.3.8-3
                             warning: bootstrap class path not set in conjunction with -source 1.6
                             /home/travis/build/pantsbuild/pants/.pants.d/gen/scrooge/java-sync/com/pants/examples/distance/thriftjava/Distance.java:32: error: duplicate class: com.pants.examples.distance.thriftjava.Distance
                             public class Distance implements TBase<Distance, Distance._Fields>, java.io.Serializable, Cloneable {
                                    ^
                             /home/travis/build/pantsbuild/pants/.pants.d/gen/scrooge/java-sync/com/pants/examples/precipitation/thriftjava/Precipitation.java:32: error: duplicate class: com.pants.examples.precipitation.thriftjava.Precipitation
                             public class Precipitation implements TBase<Precipitation, Precipitation._Fields>, java.io.Serializable, Cloneable {
                                    ^
                             Writing project database...  Done.
    

    I'll send up a fix shortly.

    1. https://rbcommons.com/s/twitter/r/1497/

  3. 
      
Loading...