aantono / gradle-plugin-protobuf

Protobuf Plugin for Gradle has moved to
https://github.com/google/protobuf-gradle-plugin
BSD 3-Clause "New" or "Revised" License
40 stars 24 forks source link

Make the appending of ${sourceSet.name} to ${generatedFileDir} optional #10

Closed estan closed 11 years ago

estan commented 11 years ago

In my case I have the directory layout:

src/main/java
src/test/java

And I actually want my generated sources to end up inside src/main/java, alongside my normal sources. Currently it's impossible to turn off the appending of ${sourceSet.name} to ${generatedFileDir}, so with

generatedFileDir = "src/main/java"

my sources would end up in "src/main/java/main" (!).

This pull request adds the appendSourceSetName boolean property for controlling the appending of the current ${sourceSet.name} to ${generatedFileDir}. The default value is true (it will be appended).

MRylander commented 11 years ago

Another approach would be to give users the ability to change the value of ${sourceSet.name}, although I am not sure of the feasibility of doing that.

aantono commented 11 years ago

Can you elaborate as to why would you want your generated content to live in the same place as your 'static' content? Those files are typically not destined to survive post-compile steps, you are not (likely) going to put them in VCS, and all IDEs support multiple source directory configurations, so it does not really matter where those files live. It is, however, a Gradle convention to put all build-generated content into sub-folders of the 'build' directory and not to mess with the static resources.

estan commented 11 years ago

Hm. I think you have me convinced Alex :) I'll think I'll try to re-structure my project to deal with the default behavior of the plugin (putting it in ${project.buildDir}/generated-sources/${sourceSet.name}). It would be nice if the testProject or the example in README.md showed how to handle the compiling (and cleanup) of the generated sources. But perhaps it already does and I'm just too new to the terseness of Gradle to notice? (I'm a newcomer to Gradle).

But that said, I still think it's wrong to force something on the user, even if it is Gradle convention. A way to override it would be nice. Perhaps the patch would look better if it was a property called generatedFileDirSuffix, which defaults to "/${sourceSet.name}" but could be overridden (to e.g. the empty string)?

estan commented 11 years ago

I just realized that it will magically compile the generated sources and I will have them on the class path, so yes it was just magic I didn't understand :) I'll close this pull request now. But I still think it would be nice to be able to override the default behavior.

aantono commented 11 years ago

The gradle 'clean' task will also delete all the generated artifacts in the 'build' directory, so no need to manually clean that up either. :)

Guilaume commented 11 years ago

Please add the option, as estan I use src/main/java

Can you elaborate as to why would you want your generated content to live in the same place as your 'static' content?

Ok this is not the best way to manage it but I store the generated java in the VCS because 1- I want to track change even if these changes are related to protoc version 2- On production environment, protoc is not installed and gradle will not try to compile *.proto file so I will be sure of what is use for building environment (ie see point 1) 3- Not sure if this point could not be done with source code in "generated source" but

and even if we change for what you suggest , I am not sure we will wants to have 2 folders for main and test

Thanks if you can add this

aantono commented 11 years ago

In your builds you should have a separation between "manual" code and "generated" code. If you look inside the generated proto stubs (java files), the very first line says "DO NOT EDIT", so you should not be touching those files, other than regenerating them using protoc or other compiler. Those are the Google guidelines, and this plugin complies with those. This, however, does not prevent you from adding the ./build/generated-sources as your code source in IDE (I have done that successfully in IntelliJ and others I work with have done it in Eclipse as well). Your IDE will just scan the content of ALL source folders and create a common code graph, where the changes will be applied all across, regardless of which folder the changed java files resides in. As for tracking the version, the "correct" approach, IMHO, should be versioning the proto files and your build file (this way you will be able to track the evolution of the proto structure as well as any changes to the use of protoc, which will be reflected in your build.gradle) Also, to comply with the design intent of the build system (not only Gradle, but Maven and others are the same way), the "generated" artifacts should not be mixed with the static ones, and upon running "clean" they will be removed and re-generated upon command. (Just like you regenerate .class files, and you exclude them from your VCS, you should do so with all the other "products" of the build tool). I am not entirely sure why you execute builds in your production environment, instead of deploying officially released, signed and published artifacts (jars, or other forms of packaging), but, IMHO, running builds in a production environment is not the best option for ensuring traceability and transparency of the change.

Guilaume commented 11 years ago

Hello,

-I do not edit generated file

-I would be very happy if you explain me how you can guarantee the result of generation of code from proto file give the same result on 2 differents environments when you don't know the version of protoc installed (if this is installed)

We already have this discussion internally, and we agree on most of you points but we are not really hot to manage different version of protoc installed in different environment (more we don't even installed protoc on one jenkins that is hosted externally.

and I would really appreciate that the generatedFileDir is the generatedFileDir