google / protobuf-gradle-plugin

Protobuf Plugin for Gradle
Other
1.73k stars 269 forks source link

Task generateProto is not executed before task compileJava during build in version 0.9.3 #729

Closed clo-vis closed 10 months ago

clo-vis commented 10 months ago

gradle version: 7.4.2

src/main/java/Foo.java

import example.Example;
class Foo { Example.MyEnum field; }

src/main/proto/example.proto

package example;
enum MyEnum { A = 0; B = 1; }

build.gradle

plugins {
    id 'java'
    id 'eclipse'
    id 'com.google.protobuf' version '0.9.3'  // used to work with 0.8.19
}

repositories { mavenCentral() }

def protobufVersion = '3.23.4'

java {
    sourceCompatibility = JavaVersion.VERSION_17
    withSourcesJar()
}

protobuf {
    protoc {
        artifact = 'com.google.protobuf:protoc:' + protobufVersion
    }
}

dependencies { implementation group: 'com.google.protobuf', name: 'protobuf-java', version: protobufVersion }

eclipse {
    sourceSets.main.java.srcDirs += ["build/generated/source/proto/main/java"]
}

Output of gradle build:

> Task :compileJava FAILED
C:\Users\seragiottoc\git\protobuf\src\main\java\Foo.java:1: error: package example does not exist
import example.Example;
              ^
C:\Users\seragiottoc\git\protobuf\src\main\java\Foo.java:4: error: package Example does not exist
    Example.MyEnum field;
           ^
2 errors

FAILURE: Build failed with an exception.

Partial output of gradle build --info:

...
Tasks to be executed: [task ':compileJava', task ':extractProto', task ':processResources', ...

Partial output of gradle build --info when using com.google.protobuf version 0.8.3:

...
Tasks to be executed: [task ':extractIncludeProto', task ':extractProto', task ':generateProto', task ':compileJava', task ':processResources', ...
ejona86 commented 10 months ago

Does it still fail if you remove the Eclipse sourceSets configuration? I'm suspicious of it. This is a very basic use-case, and it hasn't been failing elsewhere.

clo-vis commented 10 months ago

No, it doesn't fail without the Eclipse sourceSets configuration.

ejona86 commented 10 months ago

I figured it out. There is no eclipse configuration in that configuration. EclipseModel (the thing at project.eclipse) has no sourceSets: https://docs.gradle.org/current/dsl/org.gradle.plugins.ide.eclipse.model.EclipseModel.html

That means the sourceSets is just being applied to project, so it is the same as:

sourceSets {
  main {
    java.srcDirs += ["build/generated/source/proto/main/java"]
  }
}

And that is clearly broken, as it adds a folder generated by the task without the task.

Modifying sourceSets shouldn't be necessary any more for Eclipse. Just applying the eclipse plugin should be enough. So I'd recommend just deleting that eclipse block.

clo-vis commented 10 months ago

True! I just copied the first answer from StackOverflow that worked - it might be clearly broken but no error/warning is issued and it worked until ... it stopped working 🤷‍♂️

Just applying the eclipse plugin should be enough.

Perhaps it should. But it's not. I'm using noweclipse.classpath.file.whenMerged { classpath -> ... }and hoping it will work longer than the last answer I copied from StackOverflow.

ejona86 commented 10 months ago

Perhaps it should. But it's not.

How was it breaking?

clo-vis commented 10 months ago

build/generated/source/proto/main/java is not added as source folder to .classpath, that is, <classpathentry kind="src" path="build/generated/source/proto/main/java"> is missing in the file. If added manually, it is removed whenever the project is refreshed (with "Refresh Gradle project")

But this behavior, right or wrong, has nothing to do with this issue, which can be closed.

ejona86 commented 10 months ago

Hmm... That should have been improved by https://github.com/google/protobuf-gradle-plugin/pull/601 (add directories to sourceSet), and then fully fixed with https://github.com/google/protobuf-gradle-plugin/pull/590 (eagerly make the output directories) . I wonder if they had a recent behavior change. We do have a test that is passing, so we may need to try different versions.

Feel free to open another issue for that eclipse issue.