Add FindBugs plugin to released plugins

Review Request #3909 — Created May 19, 2016 and submitted

cheister
pants
release-findbugs-plugin
3470
3521
pants-reviews
jsirois, stuhood, zundel
Add FindBugs plugin to released plugins

Travis CI: https://travis-ci.org/pantsbuild/pants/builds/134540517

  1. Can you post a gist/snippet with the full output of your release run that fails?

    1. Here is the stacktrace from the Travis CI build https://travis-ci.org/pantsbuild/pants/jobs/131325956

      [== 13:12 Installing and testing package pantsbuild.pants.contrib.findbugs-1.1.0-pre1 ... ==]
      Exception caught: (<class 'pants.engine.round_engine.TaskOrderError'>)
        File "/tmp/pants.gIwng/bin/pants", line 9, in <module>
          load_entry_point('pantsbuild.pants==1.1.0-pre1', 'console_scripts', 'pants')()
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/pants_exe.py", line 44, in main
          PantsRunner(exiter).run()
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/pants_runner.py", line 56, in run
          options_bootstrapper=options_bootstrapper)
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/pants_runner.py", line 46, in _run
          return LocalPantsRunner(exiter, args, env, options_bootstrapper=options_bootstrapper).run()
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/local_pants_runner.py", line 53, in run
          self._maybe_profiled(self._run)
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/local_pants_runner.py", line 50, in _maybe_profiled
          runner()
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/local_pants_runner.py", line 95, in _run
          result = goal_runner.run()
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/goal_runner.py", line 226, in run
          result = self._execute_engine()
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/bin/goal_runner.py", line 215, in _execute_engine
          result = engine.execute(self._context, self._goals)
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/engine/legacy_engine.py", line 26, in execute
          self.attempt(context, goals)
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/engine/round_engine.py", line 208, in attempt
          goal_executors = list(self._prepare(context, goals))
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/engine/round_engine.py", line 198, in _prepare
          self._visit_goal(goal, context, goal_info_by_goal, target_roots_replacement)
        File "/tmp/pants.gIwng/lib/python2.7/site-packages/pants/engine/round_engine.py", line 174, in _visit_goal
          ordering=ordering))
      Exception message: TaskRegistrar 'findbugs' with action FindBugs_compile_findbugs depends on runtime_classpath from task ZincCompile_compile_zinc which is ordered after it in the 'compile' goal:
          [0] 'compile-jvm-prep-command' RunCompileJvmPrepCommand_compile_compile_jvm_prep_command
          [1] 'compile-prep-command' RunCompilePrepCommand_compile_compile_prep_command
          [2] 'findbugs' FindBugs_compile_findbugs
          [3] 'compile' NoopCompile_compile
          [4] 'zinc' ZincCompile_compile_zinc
          [5] 'jvm-dep-check' JvmDependencyCheck_compile_jvm_dep_check
          [6] 'checkstyle' Checkstyle_compile_checkstyle
          [7] 'scalastyle' Scalastyle_compile_scalastyle
      Failed to install and test package pantsbuild.pants.contrib.findbugs-1.1.0-pre1!
      
    2. @cheister: In register.py for findbugs, we call:

        task(name='findbugs', action=FindBugs).install('compile')
      

      This does install the task last as far as the current view of the world goes, but maybe compile.zinc hasn't been installed yet. Not sure why that is.

      In this case, an easy workaround would be to install findbugs in the test goal or don't install it in the compile goal at all. Either way, it would would always follow the compile step (but may not be what people want in production.)

      I'll keep looking at the release script to see if there's something more I can do.

    3. So, this is the line of the release script that I think is generating the pants command that is failing. It is different from the way we normally invoke pants and I suspect this is why we see different behavior. In particular, we might want to change the way we invoke --backend-packages. Right now it is replacing all defaults and maybe we just want to append new items to backend-packages now that we have that feature for options parsing.

      # When we do (dry-run) testing, we need to run the packaged pants.
      # It doesn't have internal backend plugins so when we execute it
      # at the repo build root, the root pants.ini will ask it load
      # internal backend packages and their dependencies which it doesn't have,
      # and it'll fail. To solve that problem, we load the internal backend package
      # dependencies into the pantsbuild.pants venv.
      function execute_packaged_pants_with_internal_backends() {
        pip install --ignore-installed \
          -r pants-plugins/3rdparty/python/requirements.txt &> /dev/null && \
        PANTS_PYTHON_REPOS_REPOS="['${ROOT}/dist']" pants \
          --no-verify-config \
          --pythonpath="['pants-plugins/src/python']" \
          --backend-packages="[ \
              'internal_backend.optional', \
              'internal_backend.repositories', \
              'internal_backend.sitegen', \
              'internal_backend.utilities', \
            ]" \
          "$@"
      }
      
    4. +1 ... in practice, built-in backends are always loaded first. And so it's not really cheating to guarantee that ordering here too.

    5. Nope, they aren't!

      There is an option being passed in the code above: --plugins="['pantsbuild.pants.contrib.findbugs==1.1.0-pre2']" that you don't see outright in the code.

      If you look in extension_loader.py it demonstrates the problem. Plugins were being loaded before backends.

      This patch fixes the problem in loading this test, but I'm not sure if anyone depends on the current ordering behavior.

      diff --git a/src/python/pants/bin/extension_loader.py b/src/python/pants/bin/extension_loader.py
      index 70fd0be..4db042f 100644
      --- a/src/python/pants/bin/extension_loader.py
      +++ b/src/python/pants/bin/extension_loader.py
      @@ -32,8 +32,8 @@ def load_plugins_and_backends(plugins, working_set, backends):
         :param list<str> backends: Source backends to load (see `load_build_configuration_from_source`).
         """
         build_configuration = BuildConfiguration()
      -  load_plugins(build_configuration, plugins or [], working_set)
         load_build_configuration_from_source(build_configuration, additional_backends=backends or [])
      +  load_plugins(build_configuration, plugins or [], working_set)
         return build_configuration
      

      We could add a separate argument, something like --plugins-post for a list of plugins to be loaded after the backends are resolved without breaking existing behavior.

    6. I proposed this fix in https://rbcommons.com/s/twitter/r/3950/

    7. +1 I tracked it down to the same part of the code. It seems like loading the backends before the plugins makes sense to me.

    8. I've just merged this fix to master at ec8530e. If you rebase your change then I think CI will pass.

    9. Build is now green.

  2. 
      
  1. Ship It!
  2. 
      
  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

  1. Thanks Chris and Stu! Committed to master at 2cf475a

  2. 
      
Loading...