You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/09 08:48:12 UTC
[GitHub] [spark] WolverineJiang opened a new pull request, #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
WolverineJiang opened a new pull request, #39000:
URL: https://github.com/apache/spark/pull/39000
### What changes were proposed in this pull request?
This PR use profile named `-Puser-defined-protoc` to support that users can build and test `core` module by specifying custom `protoc` executables.
### Why are the changes needed?
As described in [SPARK-41461](https://issues.apache.org/jira/browse/SPARK-41461), the latest versions of `protoc` have the minimum version requirements for basic libraries such as `glibc` and `glibcxx`. Because of that it is not possible to compile the `core` module out of the box on CentOS 6 or CentOS 7. Instead the following error messages is shown:
```
[ERROR] /home/disk1/spark-ut/spark/core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto [0:0]: /tmp/protoc5792311590243222147.exe: /lib64/libc.so.6: version `GLIBC_2.14' not found (required by /tmp/protoc5792311590243222147.exe)
[ERROR] /home/disk1/spark-ut/spark/core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto [0:0]: /tmp/protoc5792311590243222147.exe: /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.18' not found (required by /tmp/protoc5792311590243222147.exe)
[ERROR] /home/disk1/spark-ut/spark/core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto [0:0]: /tmp/protoc5792311590243222147.exe: /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.14' not found (required by /tmp/protoc5792311590243222147.exe)
[ERROR] /home/disk1/spark-ut/spark/core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto [0:0]: /tmp/protoc5792311590243222147.exe: /usr/lib64/libstdc++.so.6: version `CXXABI_1.3.5' not found (required by /tmp/protoc5792311590243222147.exe)
```
### Does this PR introduce _any_ user-facing change?
No, the way to using official pre-release `protoc` binary files is activated by default.
### How was this patch tested?
- Pass GitHub Actions
- Manual test on CentOS6u3 and CentOS7u4
```bash
export CORE_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/mvn clean install -pl core -Puser-defined-protoc -am -DskipTests
./build/mvn clean test -pl core -Puser-defined-protoc
```
and
```bash
export CORE_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/sbt clean "core/compile" -Puser-defined-protoc
./build/sbt "core/test" -Puser-defined-protoc
```
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
URL: https://github.com/apache/spark/pull/39000
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1344150106
cc @HyukjinKwon @gengliangwang FYI
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1345792300
> After [SPARK-41247](https://issues.apache.org/jira/browse/SPARK-41247) unify the protobuf versions in Spark, I think we can unify `CORE_PROTOC_EXEC_PATH`, `CONNECT_PROTOC_EXEC_PATH` and `PROTOBUF_PROTOC_EXEC_PATH` into one environment variable.
@WolverineJiang Do you mind to unify this environment variable? At present, there are 3 similar ones, but they use the same pb version. If they will always be consistent in compilation, I think we can unify the environment variable names
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #39000:
URL: https://github.com/apache/spark/pull/39000#discussion_r1044927630
##########
core/pom.xml:
##########
@@ -713,6 +693,71 @@
</plugins>
</build>
</profile>
+ <profile>
+ <id>default-protoc</id>
+ <activation>
+ <property>
+ <name>!skipDefaultProtoc</name>
+ </property>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>${protoc-jar-maven-plugin.version}</version>
+ <executions>
+ <execution>
+ <phase>generate-sources</phase>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <configuration>
+ <protocArtifact>com.google.protobuf:protoc:${protobuf.version}</protocArtifact>
+ <protocVersion>${protobuf.version}</protocVersion>
+ <inputDirectories>
+ <include>src/main/protobuf</include>
+ </inputDirectories>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
+ <profile>
+ <id>user-defined-protoc</id>
+ <properties>
+ <core.protoc.executable.path>${env.CORE_PROTOC_EXEC_PATH}</core.protoc.executable.path>
+ </properties>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>${protoc-jar-maven-plugin.version}</version>
+ <executions>
+ <execution>
+ <phase>generate-sources</phase>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <configuration>
+ <protocArtifact>com.google.protobuf:protoc:${protobuf.version}</protocArtifact>
+ <protocVersion>${protobuf.version}</protocVersion>
+ <protocCommand>${core.protoc.executable.path}</protocCommand>
+ <inputDirectories>
+ <include>src/main/protobuf</include>
+ </inputDirectories>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
</profiles>
+
Review Comment:
nit: remove empty lines?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1345426339
Merged to master.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WolverineJiang commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
WolverineJiang commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1345825511
> Do you mind to unify this environment variable? At present, there are 3 similar ones, but they use the same pb version. If they will always be consistent in compilation, I think we can unify the environment variable names
Can't agree more. I'll do it.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WolverineJiang commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
WolverineJiang commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1344724455
The pom of the core module has active profiles, and activeByDefault does not take effect. Therefore, property is used instead.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WolverineJiang commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
WolverineJiang commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1345825935
> > After [SPARK-41247](https://issues.apache.org/jira/browse/SPARK-41247) unify the protobuf versions in Spark, I think we can unify `CORE_PROTOC_EXEC_PATH`, `CONNECT_PROTOC_EXEC_PATH` and `PROTOBUF_PROTOC_EXEC_PATH` into one environment variable.
>
> @WolverineJiang Do you mind to unify this environment variable? At present, there are 3 similar ones, but they use the same pb version. If they will always be consistent in compilation, I think we can unify the environment variable names
@LuciferYang Can't agree more. I'll do it.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1345234544
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WolverineJiang commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
WolverineJiang commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1345776739
Thanks @HyukjinKwon @LuciferYang @gengliangwang ~
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #39000:
URL: https://github.com/apache/spark/pull/39000#issuecomment-1344153591
After SPARK-41247 unify the protobuf versions in Spark, I think we can unify `CORE_PROTOC_EXEC_PATH`, `CONNECT_PROTOC_EXEC_PATH` and `PROTOBUF_PROTOC_EXEC_PATH` into one environment variable.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WolverineJiang commented on a diff in pull request #39000: [SPARK-41461][BUILD][CORE] Support user configurable protoc executables when building Spark Core.
Posted by GitBox <gi...@apache.org>.
WolverineJiang commented on code in PR #39000:
URL: https://github.com/apache/spark/pull/39000#discussion_r1045072671
##########
core/pom.xml:
##########
@@ -713,6 +693,71 @@
</plugins>
</build>
</profile>
+ <profile>
+ <id>default-protoc</id>
+ <activation>
+ <property>
+ <name>!skipDefaultProtoc</name>
+ </property>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>${protoc-jar-maven-plugin.version}</version>
+ <executions>
+ <execution>
+ <phase>generate-sources</phase>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <configuration>
+ <protocArtifact>com.google.protobuf:protoc:${protobuf.version}</protocArtifact>
+ <protocVersion>${protobuf.version}</protocVersion>
+ <inputDirectories>
+ <include>src/main/protobuf</include>
+ </inputDirectories>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
+ <profile>
+ <id>user-defined-protoc</id>
+ <properties>
+ <core.protoc.executable.path>${env.CORE_PROTOC_EXEC_PATH}</core.protoc.executable.path>
+ </properties>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.github.os72</groupId>
+ <artifactId>protoc-jar-maven-plugin</artifactId>
+ <version>${protoc-jar-maven-plugin.version}</version>
+ <executions>
+ <execution>
+ <phase>generate-sources</phase>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <configuration>
+ <protocArtifact>com.google.protobuf:protoc:${protobuf.version}</protocArtifact>
+ <protocVersion>${protobuf.version}</protocVersion>
+ <protocCommand>${core.protoc.executable.path}</protocCommand>
+ <inputDirectories>
+ <include>src/main/protobuf</include>
+ </inputDirectories>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
</profiles>
+
Review Comment:
done
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org