Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Incremental compilation "sometimes" does not delete previous outputs #332

Closed
yigit opened this issue Feb 25, 2021 · 2 comments
Closed

Incremental compilation "sometimes" does not delete previous outputs #332

yigit opened this issue Feb 25, 2021 · 2 comments
Assignees
@ting-yuan
Labels
bug Something isn't working P1 major features or blocking bugs
Milestone
1.0.0-beta

Comments

@yigit
Copy link
Collaborator

yigit commented Feb 25, 2021

I'm trying to add incremental compilation to room and one of our existing tests is failing.
The test asserts that deleting a source element will delete the file that was generated for it. For some reason, Outputs to remove is empty in the kspDirrtySetByOutputs directory. Unfortunately, I've not been able to reproduce this outside AndroidX.

I'm suspecting that this might be some path issues. If you check the kspDirtySetByOutputs log, it has some of the files as absolute paths while others as local paths.

// from kspDirtySetByDeps.log
Removed
  src/main/java/room/testapp/Dao1.java:/tmp/junit2413537717527726914/src/main/java/room/testapp/Database1.java:/tmp/junit2413537717527726914/src/main/java/room/testapp/Entity1.java
Dirty

The compilation fails with:

> Task :compileDebugJavaWithJavac FAILED
/tmp/junit2413537717527726914/build/generated/ksp/debug/java/room/testapp/Database1_Impl.java:28: error: cannot find symbol
public final class Database1_Impl extends Database1 {
                                          ^

Database1 is deleted in the test but if you check kspSourceToOutputs below, we've set it as a dependency on Database1_Impl.

Here are the contents of the 4 incremental logs:

kspDirtySet.log

=== Build 1614283533349 ===
All Files
  src/main/java/placeholer.kt
  src/main/java/room/testapp/Entity1.java
  src/main/java/room/testapp/Database1.java
  src/main/java/room/testapp/Dao1.java
  src/main/java/room/testapp/Entity2.java
  src/main/java/room/testapp/Dao2.java
  src/main/java/room/testapp/Database2.java
Dirty:
  src/main/java/placeholer.kt
  src/main/java/room/testapp/Entity1.java
  src/main/java/room/testapp/Database1.java
  src/main/java/room/testapp/Dao1.java
  src/main/java/room/testapp/Entity2.java
  src/main/java/room/testapp/Dao2.java
  src/main/java/room/testapp/Database2.java

Dirty / All: 100.00%

=== Build 1614283538201 ===
All Files
  src/main/java/placeholer.kt
  src/main/java/room/testapp/Entity2.java
  src/main/java/room/testapp/Dao2.java
  src/main/java/room/testapp/Database2.java
Dirty:

Dirty / All: 0.00%

kspDirtySetByDeps

=== Build 1614283538201 ===
Modified
Removed
  src/main/java/room/testapp/Dao1.java:/tmp/junit2413537717527726914/src/main/java/room/testapp/Database1.java:/tmp/junit2413537717527726914/src/main/java/room/testapp/Entity1.java
Dirty

kspDirtySetByOutputs

=== Build 1614283538201 ===
Dirty sources
  src/main/java/room/testapp/Dao1.java:/tmp/junit2413537717527726914/src/main/java/room/testapp/Database1.java:/tmp/junit2413537717527726914/src/main/java/room/testapp/Entity1.java
Outputs to remove

kspSourceToOutputs

=== Build 1614283533349 ===
Accumulated source to outputs map
  src/main/java/room/testapp/Database1.java:
    build/generated/ksp/debug/java/room/testapp/Database1_Impl.java
    build/generated/ksp/debug/java/room/testapp/Dao1_Impl.java
  src/main/java/room/testapp/Database2.java:
    build/generated/ksp/debug/java/room/testapp/Dao2_Impl.java
    build/generated/ksp/debug/java/room/testapp/Database2_Impl.java

Reprocessed sources and their outputs
  src/main/java/room/testapp/Database1.java:
    build/generated/ksp/debug/java/room/testapp/Dao1_Impl.java
    build/generated/ksp/debug/java/room/testapp/Database1_Impl.java
  src/main/java/room/testapp/Database2.java:
    build/generated/ksp/debug/java/room/testapp/Dao2_Impl.java
    build/generated/ksp/debug/java/room/testapp/Database2_Impl.java

All reprocessed outputs
  build/generated/ksp/debug/java/room/testapp/Dao1_Impl.java
  build/generated/ksp/debug/java/room/testapp/Dao2_Impl.java
  build/generated/ksp/debug/java/room/testapp/Database1_Impl.java
  build/generated/ksp/debug/java/room/testapp/Database2_Impl.java

=== Build 1614283538201 ===
Accumulated source to outputs map
  src/main/java/room/testapp/Database1.java:
    build/generated/ksp/debug/java/room/testapp/Database1_Impl.java
    build/generated/ksp/debug/java/room/testapp/Dao1_Impl.java
  src/main/java/room/testapp/Database2.java:
    build/generated/ksp/debug/java/room/testapp/Dao2_Impl.java
    build/generated/ksp/debug/java/room/testapp/Database2_Impl.java

Reprocessed sources and their outputs

All reprocessed outputs
@yigit
Copy link
Collaborator Author

yigit commented Feb 25, 2021

Can you try with this app?
I wrote a simple shell script to repro it so you should be able to reproduce it by going into the project and then running ./repro.sh
ksp-incremental-bug-repro.tar.gz

@yigit yigit changed the title Incremental compilation "sometimes" do not delete previous outputs Incremental compilation "sometimes" does not delete previous outputs Feb 25, 2021
@ting-yuan ting-yuan self-assigned this Feb 25, 2021
@ting-yuan ting-yuan added bug Something isn't working P1 major features or blocking bugs labels Feb 25, 2021
@ting-yuan ting-yuan added this to the 2021Q1 milestone Feb 25, 2021
@ting-yuan
Copy link
Collaborator

Fixed in #333.

copybara-service bot pushed a commit to androidx/androidx that referenced this issue Feb 27, 2021
@yigit
Incremental processing for KSP
fc9df11 
This CL adds incremental compilation for the KSP backend.

We do not respect the `room.incremental` flag since it was added to
initially enable incremental java annotation processing and KSP is a new
backend hence we should always be incremental. (developer can still turn
it off for all of KSP).

The way we collect these files is a bit hacky because it requires us to
wrap a KSFile into a javax.model.Element because the only API available
in JavaPoet receives javax.model.Element. An alternative implementation
could abstract JavaPoet but it would be a more complicated change.

There is a KI in KSP: google/ksp#332
* KSP does not delete previous outputs if originating files are deleted.
That test is disabled for now until there is a new KSP release with the
fix. This does not easily reproduce in a sample app but does reproduce
in our testing setup.

Bug: 176453350
Test: RoomIncrementalAnnotationProcessingTest
RelNote: Added incremental compilation support for KSP.

Change-Id: I031c1f94890ebe2a382c26f7f0745edb790a5a7b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ting-yuan ting-yuan

Labels
bug Something isn't working P1 major features or blocking bugs
Projects
None yet
Milestone
1.0.0-beta
Development

No branches or pull requests
2 participants
@yigit @ting-yuan