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.

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

generateProto always up-to-date #180

Closed
amc6 opened this issue Nov 7, 2017 · 13 comments
Closed

generateProto always up-to-date #180

amc6 opened this issue Nov 7, 2017 · 13 comments
Labels

Comments

@amc6
Copy link

amc6 commented Nov 7, 2017

Expected: Editing a protobuf file and then running ./gradlew generateProto should regenerate the code for the changed proto.
Actual: The task is considered "up-to-date" by gradle and doesn't run.
protoc version: 3.4.0
protobuf-gradle-plugin version: 0.8.3

As a workaround I've configured the task so that it is never considered up to date, but that isn't ideal since it will cause all dependent tasks to be not up to date. But if I'm missing something here, please let me know.

protobuf {
    protoc {
        artifact = "com.google.protobuf:protoc:$protobuf_version"
    }
    generateProtoTasks {
        all().each { task ->
            task.outputs.upToDateWhen { false }
        }
    }
}
@zpencer zpencer self-assigned this Nov 17, 2017
@zhangkun83 zhangkun83 added the bug label Nov 17, 2017
@zpencer
Copy link
Contributor

zpencer commented Nov 30, 2017

@amc6 can you provide some steps on how to reproduce this problem?
I am trying to produce the bug using the exampleProject without success:

  1. run ../gradlew clean
  2. run ../gradlew generateProto and saw 3 actionable tasks: 3 executed
  3. edit exampleProject/src/main/proto/com/example/tutorial/sample.proto
  4. run ../gradlew generateProto --info and saw that generateProto was rerun.

@shevek
Copy link

shevek commented Jan 1, 2018

Additionally, if you change the proto to remove a message, the old java files are left in the generated-sources directory.

GenerateProtoTask needs a @OutputDirs (or whatever it is) annotation on getAllOutputDirs(), also @Input on getOptions(), etc, etc, to make this work properly in gradle.

@shevek
Copy link

shevek commented Jan 1, 2018

You may be able to solve this issue almost trivially by extending SourceTask instead of DefaultTask.

@zpencer
Copy link
Contributor

zpencer commented Feb 16, 2018

Regarding clearing the output dir before generating new code: this seems a bit risky to me, because we would have to make sure that directory does not contain anything we should not delete. If SourceTask already solves this problem, I think we should extend it.

@shevek
Copy link

shevek commented Feb 16, 2018

The way gradle works, every task MUST have a unique output directory, which is then added separately to the target sourceset. The entire premise gradle's input and output tracking relies on this. If the directory contains anything other than that generated by the generateProto task, it's an invalid build configuration, which in gradle 3 will cause performance issues, and in gradle 4/5 with build cache will cause bad builds.

@zpencer
Copy link
Contributor

zpencer commented Jun 6, 2018

The plugin allows people to specify an output dir that is outside of build though. For example, the plugin can be configured to output the code to src/generated/main to be checked in to the repo, and for that case deleting the base dir can be dangerous. It sounds like overriding the default build directory output with a custom generatedFilesBaseDir may interfere with gradle's output tracking.

Maybe this can be fixed just for the non overridden case.

@amc6 @shevek can either of you contribute a test case for the always up to date issue? I am still unsuccessful in reproducing it.

@davidburstrom
Copy link

@zpencer Another option than generating the protos straight into a directory to be checked into the repo (which still has the problem of not removing any stale output) is to generate the code into a fixed output directory but have another task that can sync the output into a configurable source directory.

@ejona86
Copy link
Collaborator

ejona86 commented Apr 29, 2020

@davidburstrom, see also #332. I think we should be deprecating generatedFilesBaseDir and we instead emulate the old behavior by auto-creating a Copy task if the option is specified.

@shevek
Copy link

shevek commented May 16, 2020

Gradle tasks should not output into a directory which contains any other files, as this breaks build caching, and a break of build caching in the middle of an enterprise build costs a LOT of money. It's basically moving towards being a rule these days that the situation @zpencer describes is forbidden.

If you want to have a dir which contains the outputs of multiple tasks, you make each of those tasks output into its own output dir, and then sync them all together using from(task0), from(task1), etc.

Octogonapus added a commit to CommonWealthRobotics/bowler-kernel that referenced this issue Aug 8, 2020
Octogonapus added a commit to CommonWealthRobotics/bowler-kernel that referenced this issue Aug 8, 2020
* Add proto java source generation

* Workaround for google/protobuf-gradle-plugin#180. Update to latest proto versions.

* Update build.gradle.kts
@davidburstromspotify
Copy link

A simple workaround in Gradle is to do

// given that com.google.protobuf:protobuf-gradle-plugin is present on the script classpath

tasks.withType(com.google.protobuf.gradle.GenerateProtoTask).configureEach { task ->
    doFirst {
        file(outputBaseDir).deleteDir()
    }   
}

With that snippet in place, output will never be stale and the cache artifacts won't get poisoned.

If you've configured the outputBaseDir to be shared with other tasks as outlined in #180 (comment), do NOT use this workaround.

@zpencer zpencer removed their assignment Sep 9, 2020
@zpencer
Copy link
Contributor

zpencer commented Sep 9, 2020

Sorry I don't have cycles to work on this. I have unassigned myself so others can feel free to take this issue.

@ejona86 ejona86 closed this as not planned Won't fix, can't repro, duplicate, stale Nov 5, 2022
@shevek
Copy link

shevek commented Nov 16, 2022

Should this remain open as a valid bug, even if we don't plan to fix it?

@ejona86
Copy link
Collaborator

ejona86 commented Nov 16, 2022

@shevek, this was simply a duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants