You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by hyunsik <gi...@git.apache.org> on 2017/12/02 22:37:12 UTC
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
GitHub user hyunsik opened a pull request:
https://github.com/apache/tajo/pull/1052
[TAJO-2185] Eliminate protoc binary dependency
See https://issues.apache.org/jira/browse/TAJO-2185.
Building Tajo project depends on protoc binary installed on a local machine. It is very inconvenient because users must install protoc in their local machine. It would be great if we eliminate this dependency. This patch uses [1] to replace the dependency of local binary by protoc-jar.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hyunsik/tajo TAJO-2185
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/1052.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1052
----
commit e1648a30f58ea95503bbca13a387ea1dafbcca2b
Author: hyunsik <hy...@coupang.com>
Date: 2017-12-02T22:35:24Z
[TAJO-2185] Eliminate protoc binary dependency
----
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/1052
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154541831
--- Diff: tajo-catalog/tajo-catalog-common/pom.xml ---
@@ -75,25 +75,27 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../../tajo-common/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/CatalogProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
--- End diff --
Don't have to include the input directory. Guess it's a copy-paste error?
---
[GitHub] tajo issue #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the issue:
https://github.com/apache/tajo/pull/1052
There was travis build error because of the changed travis environment. Travis seems to use busybox where ulimit is not an external command. So, I changed travis.yml file too.
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154542177
--- Diff: tajo-catalog/tajo-catalog-common/pom.xml ---
@@ -75,25 +75,27 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../../tajo-common/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/CatalogProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
--- End diff --
Right, I'll remove tajo-catalog-common.
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154542268
--- Diff: tajo-core/pom.xml ---
@@ -93,33 +93,29 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../tajo-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-catalog/tajo-catalog-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-client/src/main/proto</argument>
- <argument>--proto_path=../tajo-plan/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/ResourceTrackerProtocol.proto</argument>
- <argument>src/main/proto/QueryMasterProtocol.proto</argument>
- <argument>src/main/proto/QueryCoordinatorProtocol.proto</argument>
- <argument>src/main/proto/TajoWorkerProtocol.proto</argument>
- <argument>src/main/proto/InternalTypes.proto</argument>
- <argument>src/main/proto/ResourceProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-core/src/main/proto</includeDirectory>
--- End diff --
Interestingly, tajo-client/src/main/proto was not necessary. The protobuf files of tajo-client depend on only those of tajo-core, but it wasn't vise versa.
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154542285
--- Diff: tajo-storage/tajo-storage-hbase/pom.xml ---
@@ -112,26 +112,28 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../../tajo-common/src/main/proto</argument>
- <argument>--proto_path=../../tajo-catalog/tajo-catalog-common/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/StorageFragmentProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
+ <includeDirectory>../../tajo-core/src/main/proto</includeDirectory>
--- End diff --
I'll remove the tajo-core.
---
[GitHub] tajo issue #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the issue:
https://github.com/apache/tajo/pull/1052
@jihoonson Thanks for your quick review. I reflected your comments.
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154543606
--- Diff: tajo-core/pom.xml ---
@@ -93,33 +93,29 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../tajo-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-catalog/tajo-catalog-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-client/src/main/proto</argument>
- <argument>--proto_path=../tajo-plan/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/ResourceTrackerProtocol.proto</argument>
- <argument>src/main/proto/QueryMasterProtocol.proto</argument>
- <argument>src/main/proto/QueryCoordinatorProtocol.proto</argument>
- <argument>src/main/proto/TajoWorkerProtocol.proto</argument>
- <argument>src/main/proto/InternalTypes.proto</argument>
- <argument>src/main/proto/ResourceProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-core/src/main/proto</includeDirectory>
--- End diff --
I removed that too.
---
[GitHub] tajo issue #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the issue:
https://github.com/apache/tajo/pull/1052
Thanks. I'll merge it.
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154541970
--- Diff: tajo-storage/tajo-storage-hbase/pom.xml ---
@@ -112,26 +112,28 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../../tajo-common/src/main/proto</argument>
- <argument>--proto_path=../../tajo-catalog/tajo-catalog-common/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/StorageFragmentProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
+ <includeDirectory>../../tajo-core/src/main/proto</includeDirectory>
--- End diff --
This new directory should be included?
---
[GitHub] tajo issue #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the issue:
https://github.com/apache/tajo/pull/1052
I found a new bug filed at https://issues.apache.org/jira/browse/TAJO-2186. IMO, It seems not critical. So, I temporarily fixed this unit test issue.
---
[GitHub] tajo issue #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the issue:
https://github.com/apache/tajo/pull/1052
No problem. Would you address [this comment](https://github.com/apache/tajo/pull/1052#discussion_r154543103) as well?
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154541910
--- Diff: tajo-core/pom.xml ---
@@ -93,33 +93,29 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../tajo-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-catalog/tajo-catalog-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-client/src/main/proto</argument>
- <argument>--proto_path=../tajo-plan/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/ResourceTrackerProtocol.proto</argument>
- <argument>src/main/proto/QueryMasterProtocol.proto</argument>
- <argument>src/main/proto/QueryCoordinatorProtocol.proto</argument>
- <argument>src/main/proto/TajoWorkerProtocol.proto</argument>
- <argument>src/main/proto/InternalTypes.proto</argument>
- <argument>src/main/proto/ResourceProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-core/src/main/proto</includeDirectory>
--- End diff --
Has to be ```../tajo-client/src/main/proto```?
---
[GitHub] tajo pull request #1052: [TAJO-2185] Eliminate protoc binary dependency
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/1052#discussion_r154543103
--- Diff: tajo-core/pom.xml ---
@@ -93,33 +93,29 @@
</executions>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <version>1.2</version>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>3.5.0</version>
<executions>
<execution>
- <id>generate-sources</id>
<phase>generate-sources</phase>
- <configuration>
- <executable>protoc</executable>
- <arguments>
- <argument>-Isrc/main/proto/</argument>
- <argument>--proto_path=../tajo-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-catalog/tajo-catalog-common/src/main/proto</argument>
- <argument>--proto_path=../tajo-client/src/main/proto</argument>
- <argument>--proto_path=../tajo-plan/src/main/proto</argument>
- <argument>--java_out=target/generated-sources/proto</argument>
- <argument>src/main/proto/ResourceTrackerProtocol.proto</argument>
- <argument>src/main/proto/QueryMasterProtocol.proto</argument>
- <argument>src/main/proto/QueryCoordinatorProtocol.proto</argument>
- <argument>src/main/proto/TajoWorkerProtocol.proto</argument>
- <argument>src/main/proto/InternalTypes.proto</argument>
- <argument>src/main/proto/ResourceProtos.proto</argument>
- </arguments>
- </configuration>
<goals>
- <goal>exec</goal>
+ <goal>run</goal>
</goals>
+ <configuration>
+ <protocVersion>2.5.0</protocVersion>
+ <includeStdTypes>true</includeStdTypes>
+ <inputDirectories>
+ <include>src/main/proto/</include>
+ </inputDirectories>
+ <includeDirectories>
+ <includeDirectory>../tajo-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-catalog/tajo-catalog-common/src/main/proto</includeDirectory>
+ <includeDirectory>../tajo-core/src/main/proto</includeDirectory>
--- End diff --
Ok, then we don't have include the input directory here as well.
---