xolstice / protobuf-maven-plugin

Maven Plugin that executes the Protocol Buffers (protoc) compiler
https://www.xolstice.org/protobuf-maven-plugin/
Other
232 stars 76 forks source link

outputDirectory is ignored #3

Closed n8fisher closed 6 years ago

n8fisher commented 8 years ago

-DoutputDirectory is ignored in the compile-python plugin.

I have looked at the mojo plugin, and I see the parameter annotation on outputDirectory, but this does not work. I have also tried to set this in the pom. This property is ignored.

mvn --version Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-10T08:41:47-08:00) Maven home: /Users/n3fishe/apache-maven-3.3.9 Java version: 1.8.0_77, vendor: Oracle Corporation Java home: /Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/jre Default locale: en_US, platform encoding: UTF-8 OS name: "mac os x", version: "10.10.5", arch: "x86_64", family: "mac"


XXX:analytics fooUser$ mvn clean protobuf:compile-python -DoutputDirectory=/tmp/ [INFO] Scanning for projects... [INFO] ------------------------------------------------------------------------ [INFO] Detecting the operating system and CPU architecture [INFO] ------------------------------------------------------------------------ [INFO] os.detected.name: osx [INFO] os.detected.arch: x86_64 [INFO] os.detected.classifier: osx-x86_64 [INFO]
[INFO] ------------------------------------------------------------------------ [INFO] Building Analytics platform 1.0-SNAPSHOT [INFO] ------------------------------------------------------------------------ [INFO] [INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ ad-analytics --- [INFO] Deleting /Users/foo/dev/code/analytics/target [INFO] [INFO] --- protobuf-maven-plugin:0.5.0:compile-python (default-cli) @ ad-analytics --- [WARNING] No 'protocExecutable' parameter is configured, using the default: 'protoc' [INFO] Compiling 7 proto file(s) to /Users/foo/dev/code/analytics/target/generated-sources/protobuf/python [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 0.678 s [INFO] Finished at: 2016-04-17T21:22:01-07:00 [INFO] Final Memory: 12M/309M [INFO] ------------------------------------------------------------------------ XXX:analytics foo$

sergei-ivanov commented 8 years ago

I acknowledge it is a bug (or an underimplementation, if you like). It looks like at the moment one cannot control the outputDirectory parameter through system properties.

There is a workaround for the time being: define a configuration parameter and make it refer to a custom property in your POM:

<properties>
    <pythonOutputDirectory>${project.build.directory}/generated-sources/protobuf/python</pythonOutputDirectory>
</properties>
...
<plugin>
    <groupId>org.xolstice.maven.plugins</groupId>
    <artifactId>protobuf-maven-plugin</artifactId>
    <version>0.5.0</version>
    <executions>
        <execution>
            <goals>
                <goal>compile-python</goal>
            </goals>
            <configuration>
                <outputDirectory>${pythonOutputDirectory}</outputDirectory>
            </configuration>
        </execution>
    </executions>
</plugin>

This way, the property is set by default to the same value that the plugin would set it to, but you can override it from command line by passing e.g. -DpythonOutputDirectory=/tmp.

GhaTMA commented 6 years ago

I have a similar behaviour when I want to set additionalProtoPathElement

<configuration>
    <protocArtifact>com.google.protobuf:protoc:3.3.0:exe:${os.detected.classifier}</protocArtifact>
    <pluginId>grpc-java</pluginId>
    <pluginArtifact>io.grpc:protoc-gen-grpc-java:1.6.1:exe:${os.detected.classifier}</pluginArtifact>
    <additionalProtoPathElements>
        <additionalProtoPathElement>
            ${env.GOPATH}/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api/
        </additionalProtoPathElement>
    </additionalProtoPathElements>
</configuration>

I think this is a major problem if maven properties are not replaced correctly in the configuration. The workaround with "defining a property" did not work for me. Is someone working on this bug? are there any plans when this would be fixed?

sergei-ivanov commented 6 years ago

additionalProtoPathElements is a list, I do not think it is possible to specify a list of directories on a command line via -DadditionalProtoPathElements (it is a Maven limitation). Please correct me if I am wrong. If you have a fixed number of elements in the list (i.e. only one additional path), then you can always interpolate it through custom property.

GhaTMA commented 6 years ago

The maven limitation makes sense to me too. If you look at my configuration above (with one single <additionalProtoPathElement>), I would expect it to work. But it doesn't. It all goes down to the property ${env.GOPATH}. It doesn't get resolved. I tried the same config with other properties, I thought may be there is something wrong with my ${env} property. But it didn't work either.

sergei-ivanov commented 6 years ago

${env.GOPATH} refers to an environment variable named GOPATH, which is not the same as system property. Instead of passing it via -D you need to do set GOPATH=<path> (Windows) or export GOPATH=<path> (Linux) before running Maven.

GhaTMA commented 6 years ago

Sorry for the deplay! I didn't get notified about your reply on the 11th Sep.

I'm using windows and GOPATH is set as System Variable. That's why I'm using it with the ${env} Shouldn't this be exactly what set GOPATH=<path meant for. I tried out the scenario with a property called <go.path> and it didn't work. I'm really worried that no property gets replaced there. Do you have a test case covering replacing the variables/properties in the configuration?

sergei-ivanov commented 6 years ago

@GhaTMA Property interpolation is performed by Maven before the properties are injected into the plugin. If it does not work in your case, then you've got one of the three possible causes: 1) it is a configuration issue, 2) it is an environmental issue, and 3) it is a bug in Maven. I would not really bet on 3), because Maven has been stable for years and bugs like that would've been fixed long ago. I suspect it is either 1) or 2), but I cannot really help you without a minimal example project that reproduces the problem. I would suggest you run mvn -X help:system <goals>, capture and inspect the verbose debug output. You will see what values are being injected into plugin properties. help:system goal will additionally dump environment variables and system properties available to Maven for interpolation.

GhaTMA commented 6 years ago

Thanks @sergei-ivanov ! your interpretation sounds reasonable to me.

abhilashgoyal commented 5 years ago

Hello All. For me, it worked by using ${session.executionRootDirectory} as root directory from where you are building your project.

ericmandm commented 4 years ago

Hello, I have a multi-module maven project with several grpc modules and 'common' helper module. The common module has messages defined that need to be used in each of the grpc modules.

So I put the common.proto in common-module/src/main/resources. This allows import "common.proto" in the other module's proto files file to be resolved in the IntelliJ editor/tooling.

Then, I added the common module's path to the maven plugin in the [xyz]_grpc module

            <plugin>
                <groupId>org.xolstice.maven.plugins</groupId>
                <artifactId>protobuf-maven-plugin</artifactId>
                <version>0.6.1</version>
                <configuration>
                    <protocArtifact>com.google.protobuf:protoc:3.10.1:exe:${os.detected.classifier}</protocArtifact>
                    <pluginId>grpc-java</pluginId>
                    <pluginArtifact>io.grpc:protoc-gen-grpc-java:${grpc-version}:exe:${os.detected.classifier}</pluginArtifact>

                    <!-- add the path to the common api module to be able to import the common.proto file -->
                    <additionalProtoPathElements>
                        <additionalProtoPathElement>
                            ${project.parent.basedir}/common/src/main/resources/
                        </additionalProtoPathElement>
                    </additionalProtoPathElements>
                </configuration>
                <executions>
                    <execution>
                        <goals>
                            <goal>compile</goal>
                            <goal>compile-custom</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>

I ran: mvn -X help:system clean install and I can see that the additional path is added to the plugin:

[DEBUG] Configuring mojo 'org.xolstice.maven.plugins:protobuf-maven-plugin:0.6.1:compile' with basic configurator -->
[DEBUG]   (f) additionalProtoPathElements = [/home/tester2/IdeaProjects/myProject/common/src/main/resources]
[DEBUG]   (f) attachDescriptorSet = false
[DEBUG]   (f) attachProtoSources = true
[DEBUG]   (f) checkStaleness = false
[DEBUG]   (f) clearOutputDirectory = true

etc...

However, the messages defined in common.proto are not being generated. If I put the common.proto in the same folder as [xyz]_grpc.proto, then the messages get generated and compiled.

I'd really like to NOT have to put a copy of the common.proto in each grpc module that needs it... so any ideas about what I am doing wrong would be appreciated.

note: I'm using java 11

sergei-ivanov commented 4 years ago

@ericmandm Polite request: please do not hijack old closed issues. It's better to open a new one and link to the old one (if needed).

To your problem. The recommended way of doing what you're doing is to generate protobuf and gRPC bindings directly in your common module. Then you simply declare a compile dependency on your common module and then you can use imports in your .proto files. There's usually no good reason to use additionalProtoPathElements if you are using shared modules and dependencies properly.

A typical issue with non-working imports is a misuse of protobuf packages. If your common.proto declares package my.org, then it should be placed in ${project.parent.basedir}/common/src/main/proto/my/org/common.proto. That way it will be packaged into common.jar file with the correct path, and you can later use it in another module with import "my/org/common.proto".

Like in Java, using default package for protobuf sources is strongly discouraged. Using the same package namespace as your java sources is recommended.

ericmandm commented 4 years ago

Thanks for your response… and I’m very sorry about the hijacking… I knew the last post was old, but I did not notice that it had been closed.

Unfortunately, the importing proto file is still not able to compile.

Here is what I have now…. using real module names: My 'common’ module is ‘device-api’ with device-api-common.proto. I has this package defined:

package com.elevation.automation.device.api;

I put the proto file in the same package path, under the src/main/proto

Compiling the device-api module works as expected and generates the classes and jar file.

The importing proto file is: barcode-server.proto, which includes this import:

import "com/elevation/automation/device/api/device-api-common.proto";

Which matches the path in the device-api module’s jar file.

I added the device-api-commom as dependency into the barcode-server's pom.xml. However, when I run the mvn install on the barcode-server module, I see this in the console output:

barcode-server.proto:20:39: "DeviceStatusProto" is not defined.
barcode-server.proto:4:1: warning: Import com/elevation/automation/device/api/device-api-common.proto but not used.

Additional info: The intelliJ .proto editor cannot find the device-api-common.proto either. I removed the from the plugin config. I know the barcode-server module properly depends on the device-api module since I am able to import the generated protobuf classes into a barcode-server module class file.

Is there anything else I can try to make this work?

Thanks Eric-


Eric Hansen eric.hansen.hmm@gmail.com 805-712-0730

On Mar 29, 2020, at 7:20 PM, Sergei Ivanov notifications@github.com wrote:

@ericmandm https://github.com/ericmandm Polite request: please do not hijack old closed issues. It's better to open a new one and link to the old one (if needed).

To your problem. The recommended way of doing what you're doing is to generate protobuf and gRPC bindings directly in your common module. Then you simply declare a compile dependency on your common module and then you can use imports in your .proto files. There's usually no good reason to use additionalProtoPathElements if you are using shared modules and dependencies properly.

A typical issue with non-working imports is a misuse of protobuf packages. If your common.proto declares package my.org, then it should be placed in ${project.parent.basedir}/common/src/main/proto/my/org/common.proto. That way it will be packaged into common.jar file with the correct path, and you can later use it in another module with import "my/org/common.proto".

Like in Java, using default package for protobuf sources is strongly discouraged. Using the same package namespace as your java sources is recommended.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xolstice/protobuf-maven-plugin/issues/3#issuecomment-605747226, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJE3P4G74U767K4HOSHLRK3RJ76V3ANCNFSM4CBFP2OQ.

sergei-ivanov commented 4 years ago

@ericmandm I think you are on the right path. Are you sure that the message with the name DeviceStatusProto is defined in file device-api-common.proto? Maybe it's simply called DeviceStatus? I think the import works, but you have a problem with your references. Try simplifying your configuration by placing all .proto files under one root and making sure that the whole set of files compiles correctly and that all references are resolved. Once you've verified that, you can start moving .proto files across to the common module, keeping the paths.

ericmandm commented 4 years ago

I forgot to mention that I did put both proto files under one roof and everything compiles fine.
When I move the device-api-common.proto file to the same exact folder path in the device-api project, it won’t compile. It really seems to me that the plugin (and intelliJ plugin) is not respecting the maven

On Mar 31, 2020, at 3:28 PM, Sergei Ivanov notifications@github.com wrote:

@ericmandm https://github.com/ericmandm I think you are on the right path. Are you sure that the message with the name DeviceStatusProto is defined in file device-api-common.proto? Maybe it's simply called DeviceStatus? I think the import works, but you have a problem with your references. Try simplifying your configuration by placing all .proto files under one root and making sure that the whole set of files compiles correctly and that all references are resolved. Once you've verified that, you can start moving .proto files across to the common module, keeping the paths.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xolstice/protobuf-maven-plugin/issues/3#issuecomment-606918258, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJE3P4HTKJWHCF7YSZZZF53RKJVA3ANCNFSM4CBFP2OQ.

sergei-ivanov commented 4 years ago

There's no good reason why it would not work, because configurations like yours have been tried and tested in multiple large scale projects. If you can distil it into a small sample project that demonstrates the issue, I'll be able to take a closer look and say what is missing. I cannot really say anything about IntelliJ IDEA plugin, because I haven't used it for a long time. When I did use it, I think it was not able to resolve dependencies across modules. In any case, let's try to resolve the underlying issue with the project configuration first.

ericmandm commented 4 years ago

Thanks Sergei. I created a bare-bones project per your suggestion and the importing worked when compiling from Maven. The Intellij proto editor plugin did not, however, recognize the import from the other module.

I have not been able to determine why my production project is not working still… it seems that I have configured everything the same, but I will keep looking.

One thing of note… in my bare-bones project, the import did not work unless both proto files had the same package defined. Is that expected?

On Mar 31, 2020, at 4:13 PM, Sergei Ivanov notifications@github.com wrote:

There's no good reason why it would not work, because configurations like yours have been tried and tested in multiple large scale projects. If you can distil it into a small sample project that demonstrates the issue, I'll be able to take a closer look and say what is missing. I cannot really say anything about IntelliJ IDEA plugin, because I haven't used it for a long time. When I did use it, I think it was not able to resolve dependencies across modules. In any case, let's try to resolve the underlying issue with the project configuration first.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xolstice/protobuf-maven-plugin/issues/3#issuecomment-606933608, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJE3P4AMYEKXVUQGHXGYIU3RKJ2K7ANCNFSM4CBFP2OQ.

sergei-ivanov commented 4 years ago

The imports should work even if the packages are different (otherwise there'd be no value in the imports!). The key is to make sure that the .proto files under src/main/proto are sitting in the correct subfolder that matches their declared package (proto package, not java_package!). If you are still stuck, push your bare-bones project to github and let me know where I can find it. I'll take a look.