[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326915036 ## File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-csi/pom.xml ## @@ -150,12 +150,12 @@ protobuf-maven-plugin ${protobuf-maven-plugin.version} - com.google.protobuf:protoc:3.6.1:exe:${os.detected.classifier} grpc-java io.grpc:protoc-gen-grpc-java:1.15.1:exe:${os.detected.classifier} +src-compile-protoc Review comment: This will change the default output directory "target/generate-sources/protobuf/java" to "target/generate-sources/java" So, next plugin "build-helper-maven-plugin" in this project may fail. yarn-csi project was not affected by the protobuf upgrade. So better to keep it as is. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326914634 ## File path: hadoop-project/pom.xml ## @@ -1715,9 +1715,56 @@ frontend-maven-plugin ${frontend-maven-plugin.version} + + org.xolstice.maven.plugins + protobuf-maven-plugin + ${protobuf-maven-plugin.version} + true + + + com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier} + +false + + + + src-compile-protoc + generate-sources + +compile + + + false +${basedir}/src/main/proto + ${project.build.directory}/generated-sources/java +false + + + + src-compile-test-protoc Review comment: Id of this execution has been changed in #1494 to "test-compile-protoc". Please update the corresponding Id in applicable changes in this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326914640 ## File path: hadoop-project/pom.xml ## @@ -1715,9 +1715,56 @@ frontend-maven-plugin ${frontend-maven-plugin.version} + + org.xolstice.maven.plugins + protobuf-maven-plugin + ${protobuf-maven-plugin.version} + true + + + com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier} + +false + + + + src-compile-protoc + generate-sources + +compile + + + false +${basedir}/src/main/proto + ${project.build.directory}/generated-sources/java +false + + + + src-compile-test-protoc + generate-test-sources + +test-compile + + + false +${basedir}/src/test/proto Review comment: for "test-compile" goal src proto files will be referred using "protoTestSourceRoot". Not "protoSourceRoot". Mentioned here https://www.xolstice.org/protobuf-maven-plugin/test-compile-mojo.html#protoTestSourceRoot This is also updated in #1494. Please rebase. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326914827 ## File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/pom.xml ## @@ -118,45 +118,21 @@ -org.apache.hadoop -hadoop-maven-plugins - +org.xolstice.maven.plugins + protobuf-maven-plugin + -compile-protoc - - protoc - +src-compile-protoc - ${protobuf.version} - ${protoc.path} - - ${basedir}/../../../hadoop-common-project/hadoop-common/src/main/proto -${basedir}/src/main/proto -${basedir}/src/main/proto/server - - -${basedir}/src/main/proto - - yarn_protos.proto - yarn_service_protos.proto - applicationmaster_protocol.proto - applicationclient_protocol.proto - containermanagement_protocol.proto - server/yarn_server_resourcemanager_service_protos.proto - server/resourcemanager_administration_protocol.proto - application_history_client.proto - server/application_history_server.proto - client_SCM_protocol.proto - server/SCM_Admin_protocol.proto - yarn_csi_adaptor.proto - YarnCsiAdaptor.proto - - + + + ${basedir}/../../../hadoop-common-project/hadoop-common/src/main/proto Review comment: `${basedir}/src/main/proto/server` missed from additional paths. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326914809 ## File path: hadoop-project/pom.xml ## @@ -1715,9 +1715,56 @@ frontend-maven-plugin ${frontend-maven-plugin.version} + + org.xolstice.maven.plugins + protobuf-maven-plugin + ${protobuf-maven-plugin.version} + true + + + com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier} + +false + + + + src-compile-protoc + generate-sources + +compile + + + false Review comment: in #1494 this execution is made to skip by default. Reason is, every module can specifically enable this execution whenever required. Anyway every module needs to provide specific dependency information. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326952511 ## File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-csi/pom.xml ## @@ -150,12 +150,12 @@ protobuf-maven-plugin ${protobuf-maven-plugin.version} - com.google.protobuf:protoc:3.6.1:exe:${os.detected.classifier} grpc-java io.grpc:protoc-gen-grpc-java:1.15.1:exe:${os.detected.classifier} +src-compile-protoc Review comment: Yes, follow on would be better. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326953058 ## File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/pom.xml ## @@ -118,45 +118,21 @@ -org.apache.hadoop -hadoop-maven-plugins - +org.xolstice.maven.plugins + protobuf-maven-plugin + -compile-protoc - - protoc - +src-compile-protoc - ${protobuf.version} - ${protoc.path} - - ${basedir}/../../../hadoop-common-project/hadoop-common/src/main/proto -${basedir}/src/main/proto -${basedir}/src/main/proto/server - - -${basedir}/src/main/proto - - yarn_protos.proto - yarn_service_protos.proto - applicationmaster_protocol.proto - applicationclient_protocol.proto - containermanagement_protocol.proto - server/yarn_server_resourcemanager_service_protos.proto - server/resourcemanager_administration_protocol.proto - application_history_client.proto - server/application_history_server.proto - client_SCM_protocol.proto - server/SCM_Admin_protocol.proto - yarn_csi_adaptor.proto - YarnCsiAdaptor.proto - - + + + ${basedir}/../../../hadoop-common-project/hadoop-common/src/main/proto Review comment: I don't think so. It just adds them as arguments to protoc command. Just checks that it should be a directory. No extra processing done on that. https://github.com/xolstice/protobuf-maven-plugin/blob/master/src/main/java/org/xolstice/maven/plugin/protobuf/AbstractProtocMojo.java https://github.com/xolstice/protobuf-maven-plugin/blob/master/src/main/java/org/xolstice/maven/plugin/protobuf/Protoc.java This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326971079 ## File path: hadoop-project/pom.xml ## @@ -1715,9 +1715,56 @@ frontend-maven-plugin ${frontend-maven-plugin.version} + + org.xolstice.maven.plugins + protobuf-maven-plugin + ${protobuf-maven-plugin.version} + true + + + com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier} + +false + + + + src-compile-protoc + generate-sources + +compile + + + false +${basedir}/src/main/proto + ${project.build.directory}/generated-sources/java +false + + + + src-compile-test-protoc Review comment: Done as "src-test-compile-protoc" in #1494 and merged. Please update the same here. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat…
vinayakumarb commented on a change in pull request #1496: HADOOP-16560. [YARN + MAPREDUCE] use protobuf-maven-plugin to generat… URL: https://github.com/apache/hadoop/pull/1496#discussion_r326973365 ## File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/pom.xml ## @@ -118,45 +118,21 @@ -org.apache.hadoop -hadoop-maven-plugins - +org.xolstice.maven.plugins + protobuf-maven-plugin + -compile-protoc - - protoc - +src-compile-protoc - ${protobuf.version} - ${protoc.path} - - ${basedir}/../../../hadoop-common-project/hadoop-common/src/main/proto -${basedir}/src/main/proto -${basedir}/src/main/proto/server - - -${basedir}/src/main/proto - - yarn_protos.proto - yarn_service_protos.proto - applicationmaster_protocol.proto - applicationclient_protocol.proto - containermanagement_protocol.proto - server/yarn_server_resourcemanager_service_protos.proto - server/resourcemanager_administration_protocol.proto - application_history_client.proto - server/application_history_server.proto - client_SCM_protocol.proto - server/SCM_Admin_protocol.proto - yarn_csi_adaptor.proto - YarnCsiAdaptor.proto - - + + + ${basedir}/../../../hadoop-common-project/hadoop-common/src/main/proto Review comment: Oh okay. Got it. Makes sense. `${basedir}/src/main/proto/server` is also having source protos. Based on previous \ section I was thinking it as just a dependency. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org