[IntelliJ] Detect project changes in order to determine noop compile. i.e. immediate launch of test runner.

Review Request #4363 — Created Nov. 10, 2016 and submitted

wisechengyi
intellij-pants-plugin
change_listening
217
pants-reviews
benjyw, nhoward_tw, peiyu, stuhood, zundel

Earlier IntelliJ run configuration calls Pants every time before launching the test runner. This change leverages the intellij framework to detect whether a change on disk belongs to the project, so calling pants can be skipped when there is no changes between two test initiations.

This only applies to IntelliJ Junit/Scala Runner tests and not to Pants run configuration that directly calls ./pants test ...

Behavior:

  • Adding a file into/deleting a file from/editing a file in the project will trigger pants call.
  • Change of project settings will trigger pants call.
  • Using incremental imports will always trigger pants call because we can no longer determine whether a change belongs to the project or not.
  • After a clean-all it will always trigger a pants call.
  • After dist/export-classpath/manifest.jar getting overwritten because of other out of band pants CLI, pants will be triggered.

Minor issue:

  • Change in pants.ini is not tracked, but should be a very rare case. (Added in FIXME)

Other changes:
* Test against major pants releases, current dev releases, and master.

https://travis-ci.org/pantsbuild/intellij-pants-plugin/builds/177196230
https://travis-ci.org/pantsbuild/intellij-pants-plugin/builds/177197025
https://travis-ci.org/pantsbuild/intellij-pants-plugin/builds/177347224

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
WI
WI
ST
  1. 
      
  2. src/com/twitter/intellij/pants/file/FileChangeTracker.java (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     

    This will break if someone has compiled outside of intellij, such as on the CLI, right?

    1. That is not the case. Out of band file IO will be caught as long as they are under any of the project content roots.

      If a source file changes and a compile is issued on CLI, next time when user switches back to IntelliJ GUI, the system will detect changes on disk since user switched out of IntelliJ, hence the project will be marked as dirty.

      If a compile was issued on CLI without any source change, then manifest.jar which IJ consumes won't change either.

      However another caveat I just realize is ./pants clean-all isn't detected, but I think it can be resolved by having clean-all to remove dist/export-classpath/manifest.jar which would be likely be invalid anyway, and then have IntelliJ check for its existence.

    2. I added two additional checks.

      1. Have CompileSnapshot include the SHA of manifest.jar, so any change on manifest.jar will trigger a rebuild
      2. Inspect the validity of manifest.jar, so after a clean-all, METAINF/MANIFEST.MF entries will become invalid, thus triggering a new build.
  3. Updating the project state here means that this method isn't idempotent. IMO, should update the project state independently after a compile has actually succeeded. I can easily imagine a state where a cancelled/failed compile is considered to be clean.

    But also, it's not at all obvious from the javadoc that this will mutate the state.

    1. A failed/cancelled build will caught at the end of PantsMakeBeforeRun https://rbcommons.com/s/twitter/r/4363/diff/3#2 so the project will be marked dirty.

      Will update the javadoc.

    2. Doc updated to reflect the behavior.

  4. IIRC, remove works regardless of whether something is present... so no need to check first.

    1. Redundant check removed.

  5. IMO, the project change detection should probably be optional for at least one release.

    1. With the two additional checks added, I think it should be safe to land. Plus there is testing channel to use.

  6. 
      
WI
WI
WI
Review request changed

Status: Closed (submitted)

Change Summary:

ef08ef4540097bcb08d0d16dde23120388d09099

Loading...