square / wire

gRPC and protocol buffers for Android, Kotlin, Swift and Java.
https://square.github.io/wire/
Apache License 2.0
4.24k stars 570 forks source link

Project dependencies don't invalidate properly in the configuration cache #2815

Open swankjesse opened 8 months ago

swankjesse commented 8 months ago

I’m using the Wire Gradle plugin and the Gradle configuration cache.

My Wire build includes this syntax:

  sourcePath {
    srcProject(":upstream-protos")
    include("file_a.proto")
    include("file_b.proto")
  }

Unfortunately when I make changes to upstream-protos my Wire project doesn’t rebuild enough, and produces an incorrect compilation result.

In the interim I’m using this to workaround:

    tasks.withType<WireTask> {
      notCompatibleWithConfigurationCache("Project dependencies don't invalidate properly")
    }

I can also use --no-configuration-cache as a workaround, but that’s more severe.

oldergod commented 8 months ago

@martinbonnin tried to repro and wasn't able to. If any of the .proto in upstream changes, he's getting

Calculating task graph as configuration cache cannot be reused because an input to unknown location has changed.

Would it be because another file than file_a and file_b is updated?

martinbonnin commented 8 months ago

What I did is there: https://github.com/martinbonnin/test-wire

git checkout https://github.com/martinbonnin/test-wire
cd test-wire
./gradlew :module2:generateProtos --configuration-cache
sed -i.old 's/picture_urls2/picture_urls/g' module1/src/main/protos/dino.proto
./gradlew :module2:generateProtos --configuration-cache
# Calculating task graph as configuration cache cannot be reused because an input to unknown location has changed.

And the output is technically correct but it theorically should be able to reuse the CC I think? So there might be different problems there?

martinbonnin commented 8 months ago

Did a bit more digging, my hunch is that the problem lies somewhere around here:

    return project.provider {
      configuration.dependencies.flatMap { dep ->
        val sortedFiles = configuration.files(dep).sortedWith(compareBy { it.name })
        sortedFiles.flatMap { file -> file.toLocations(project, dep) }
      }
    }

This provider here breaks the task dependency between upstream modules and the WireTask. So when the module changes, nothing tells WireTask to recompute. Maybe it worked before CC because of some other dependency, I'm not sure. Ideally, WireTask.sourceInput should be moved from sourceInput: ListProperty<InputLocation> to sourceInput: ConfigurableFileCollection.

~I'll try to give it a shot~ well, that didn't turn out really well, it's more complicated than what I anticipated since the base path of the files is required. I think https://github.com/gradle/gradle/issues/27836 would be quite useful here.

swankjesse commented 7 months ago

This might help? https://github.com/square/wire/pull/2838/files

swankjesse commented 7 months ago

This too: https://github.com/square/wire/pull/2848