Invoke pants regardless sources have changed because resources may have changed; pants options fix.

Review Request #3652 — Created April 5, 2016 and submitted

wisechengyi
intellij-pants-plugin
125
3650
pants-reviews
benjyw, nhoward_tw, peiyu, stuhood, zundel

Currently the changes in resources will not be detected by hasDirtyTargets(holder) in PantsTargetBuilder because it can only detect changes in PantsBuildTargetType whereas resources are handled by ResourcesTargetType. Therefore this RB disgard any check on DirtyFilesHolder<PantsSourceRootDescriptor, PantsBuildTarget> holder, so the behavior will be idential with PantsCompile in https://rbcommons.com/s/twitter/r/3607/

It is possible to design another Builder to handle the changes ResourcesTargetType, but it will also introduce the issue where both builders will be called without knowing whether the other has performed the job, so the work may be duplicated.

It is also possible to mark resources as sources, but UX will be bad because it will unnecessarily increase the load on already-slow source indexing, and many "syntax errors" will be found.

https://travis-ci.org/wisechengyi/intellij-pants-plugin/builds/124336910

WI
ST
  1. so the behavior will be idential with PantsCompile in https://rbcommons.com/s/twitter/r/3607/

    If that's the case, is this worth doing? Should this mode of execution just be deprecated?

    1. AFAIK that is still the only way to handle any action under "Build" Menu such as "make project" and "rebuild project", and these actions will explicity call IntelliJ's Make process.

  2. 
      
WI
PE
  1. 
      
  2. questions:
    1. how much perf impact is this? since by skipping the check we would build unnecessarily? i am not clear when build is called
    2. lots of methods are cleaned up, say https://github.com/pantsbuild/pants/issues/3043 is addressed, do we need them back?

    1. how much perf impact is this?

      compare to status quo of intellij, zero impact if something is changed; otherwise it will invoke pants again but hits all the cache.
      compare to status quo of iterating over CLI, zero impact, because cli has to issue pants command again in order to run/test anything.

      since by skipping the check we would build unnecessarily?

      Yes in cases where nothing is touched.

      i am not clear when build is called

      build is called whenever user invokes 'Make' or 'Rebuild project'.

      lots of methods are cleaned up, say https://github.com/pantsbuild/pants/issues/3043 is addressed, do we need them back?

      Yes, we will always want to call pants for work necessity.

    2. Thanks, Yi!

  3. 
      
WI
WI
PE
  1. lgtm

  2. if we do plan to add the necessity check back, ok to clean up the code, not sure whether to keep the test but commented with a todo would be better, up to you

  3. 
      
WI
WI
WI
Review request changed

Status: Closed (submitted)

Change Summary:

38d0a01d20c8cd86687ed7bfd5713206c8ae1572

Loading...