Skip to content

Commit

Permalink
[SPARK-41461][BUILD][CORE][CONNECT][PROTOBUF] Unify the environment v…
Browse files Browse the repository at this point in the history
…ariable of *_PROTOC_EXEC_PATH

### What changes were proposed in this pull request?
This PR unify the environment variable of `*_PROTOC_EXEC_PATH` to support that users can use the same environment variable to build and test `core`, `connect`, `protobuf` module by use profile named `-Puser-defined-protoc` with specifying custom `protoc` executables.

### Why are the changes needed?
As described in [SPARK-41485](https://issues.apache.org/jira/browse/SPARK-41485), at present, there are 3 similar environment variable of `*_PROTOC_EXEC_PATH`, but they use the same `pb` version. Because they are consistent in compilation, so I unify the environment variable names to simplify.

### 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 SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/mvn clean install -pl core -Puser-defined-protoc -am -DskipTests -DskipDefaultProtoc
./build/mvn clean install -pl connector/connect/common -Puser-defined-protoc -am -DskipTests
./build/mvn clean install -pl connector/protobuf -Puser-defined-protoc -am -DskipTests
./build/mvn clean test -pl core -Puser-defined-protoc -DskipDefaultProtoc
./build/mvn clean test -pl connector/connect/common -Puser-defined-protoc
./build/mvn clean test -pl connector/protobuf -Puser-defined-protoc
```
and
```bash
export SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/sbt clean "core/compile" -Puser-defined-protoc
./build/sbt clean "connect-common/compile" -Puser-defined-protoc
./build/sbt clean "protobuf/compile" -Puser-defined-protoc
./build/sbt "core/test" -Puser-defined-protoc
./build/sbt  "connect-common/test" -Puser-defined-protoc
./build/sbt  "protobuf/test" -Puser-defined-protoc
```

Closes #39036 from WolverineJiang/master.

Authored-by: jianghaonan <jianghaonan@baidu.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
  • Loading branch information
jianghaonan authored and HyukjinKwon committed Dec 13, 2022
1 parent 3275555 commit f590f87
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 33 deletions.
4 changes: 2 additions & 2 deletions connector/connect/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ for example, compiling `connect` module on CentOS 6 or CentOS 7 which the defaul
specifying the user-defined `protoc` and `protoc-gen-grpc-java` binary files as follows:

```bash
export CONNECT_PROTOC_EXEC_PATH=/path-to-protoc-exe
export SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
export CONNECT_PLUGIN_EXEC_PATH=/path-to-protoc-gen-grpc-java-exe
./build/mvn -Phive -Puser-defined-protoc clean package
```

or

```bash
export CONNECT_PROTOC_EXEC_PATH=/path-to-protoc-exe
export SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
export CONNECT_PLUGIN_EXEC_PATH=/path-to-protoc-gen-grpc-java-exe
./build/sbt -Puser-defined-protoc clean package
```
Expand Down
4 changes: 2 additions & 2 deletions connector/connect/common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@
<profile>
<id>user-defined-protoc</id>
<properties>
<connect.protoc.executable.path>${env.CONNECT_PROTOC_EXEC_PATH}</connect.protoc.executable.path>
<spark.protoc.executable.path>${env.SPARK_PROTOC_EXEC_PATH}</spark.protoc.executable.path>
<connect.plugin.executable.path>${env.CONNECT_PLUGIN_EXEC_PATH}</connect.plugin.executable.path>
</properties>
<build>
Expand All @@ -203,7 +203,7 @@
<artifactId>protobuf-maven-plugin</artifactId>
<version>0.6.1</version>
<configuration>
<protocExecutable>${connect.protoc.executable.path}</protocExecutable>
<protocExecutable>${spark.protoc.executable.path}</protocExecutable>
<pluginId>grpc-java</pluginId>
<pluginExecutable>${connect.plugin.executable.path}</pluginExecutable>
<protoSourceRoot>src/main/protobuf</protoSourceRoot>
Expand Down
5 changes: 2 additions & 3 deletions connector/protobuf/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ for example, compiling `protobuf` module on CentOS 6 or CentOS 7 which the defau
specifying the user-defined `protoc` binary files as follows:

```bash
export PROTOBUF_PROTOC_EXEC_PATH=/path-to-protoc-exe
export SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/mvn -Phive -Puser-defined-protoc clean package
```

or

```bash
export PROTOBUF_PROTOC_EXEC_PATH=/path-to-protoc-exe
export CONNECT_PLUGIN_EXEC_PATH=/path-to-protoc-gen-grpc-java-exe
export SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/sbt -Puser-defined-protoc clean package
```

Expand Down
4 changes: 2 additions & 2 deletions connector/protobuf/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
<profile>
<id>user-defined-protoc</id>
<properties>
<protobuf.protoc.executable.path>${env.PROTOBUF_PROTOC_EXEC_PATH}</protobuf.protoc.executable.path>
<spark.protoc.executable.path>${env.SPARK_PROTOC_EXEC_PATH}</spark.protoc.executable.path>
</properties>
<build>
<plugins>
Expand All @@ -173,7 +173,7 @@
<configuration>
<protocArtifact>com.google.protobuf:protoc:${protobuf.version}</protocArtifact>
<protocVersion>${protobuf.version}</protocVersion>
<protocCommand>${protobuf.protoc.executable.path}</protocCommand>
<protocCommand>${spark.protoc.executable.path}</protocCommand>
<inputDirectories>
<include>src/test/resources/protobuf</include>
</inputDirectories>
Expand Down
4 changes: 2 additions & 2 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@
<profile>
<id>user-defined-protoc</id>
<properties>
<core.protoc.executable.path>${env.CORE_PROTOC_EXEC_PATH}</core.protoc.executable.path>
<spark.protoc.executable.path>${env.SPARK_PROTOC_EXEC_PATH}</spark.protoc.executable.path>
</properties>
<build>
<plugins>
Expand All @@ -799,7 +799,7 @@
<configuration>
<protocArtifact>com.google.protobuf:protoc:${protobuf.version}</protocArtifact>
<protocVersion>${protobuf.version}</protocVersion>
<protocCommand>${core.protoc.executable.path}</protocCommand>
<protocCommand>${spark.protoc.executable.path}</protocCommand>
<inputDirectories>
<include>src/main/protobuf</include>
</inputDirectories>
Expand Down
4 changes: 2 additions & 2 deletions docs/building-spark.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,14 +323,14 @@ To build and run tests on IPv6-only environment, the following configurations ar
When the user cannot use the official `protoc` binary files to build the `core` module in the compilation environment, for example, compiling `core` module on CentOS 6 or CentOS 7 which the default `glibc` version is less than 2.14, we can try to compile and test by specifying the user-defined `protoc` binary files as follows:

```bash
export CORE_PROTOC_EXEC_PATH=/path-to-protoc-exe
export SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/mvn -Puser-defined-protoc -DskipDefaultProtoc clean package
```

or

```bash
export CORE_PROTOC_EXEC_PATH=/path-to-protoc-exe
export SPARK_PROTOC_EXEC_PATH=/path-to-protoc-exe
./build/sbt -Puser-defined-protoc clean package
```

Expand Down
34 changes: 14 additions & 20 deletions project/SparkBuild.scala
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,13 @@ object SparkBuild extends PomBuild {
sys.props.put("test.jdwp.enabled", "true")
}
if (profiles.contains("user-defined-protoc")) {
val connectProtocExecPath = Properties.envOrNone("CONNECT_PROTOC_EXEC_PATH")
val sparkProtocExecPath = Properties.envOrNone("SPARK_PROTOC_EXEC_PATH")
val connectPluginExecPath = Properties.envOrNone("CONNECT_PLUGIN_EXEC_PATH")
val protobufProtocExecPath = Properties.envOrNone("PROTOBUF_PROTOC_EXEC_PATH")
val coreProtocExecPath = Properties.envOrNone("CORE_PROTOC_EXEC_PATH")
if (connectProtocExecPath.isDefined && connectPluginExecPath.isDefined) {
sys.props.put("connect.protoc.executable.path", connectProtocExecPath.get)
sys.props.put("connect.plugin.executable.path", connectPluginExecPath.get)
}
if (protobufProtocExecPath.isDefined) {
sys.props.put("protobuf.protoc.executable.path", protobufProtocExecPath.get)
if (sparkProtocExecPath.isDefined) {
sys.props.put("spark.protoc.executable.path", sparkProtocExecPath.get)
}
if (coreProtocExecPath.isDefined) {
sys.props.put("core.protoc.executable.path", coreProtocExecPath.get)
if (connectPluginExecPath.isDefined) {
sys.props.put("connect.plugin.executable.path", connectPluginExecPath.get)
}
}
profiles
Expand Down Expand Up @@ -649,10 +643,10 @@ object Core {
Seq(propsFile)
}.taskValue
) ++ {
val coreProtocExecPath = sys.props.get("core.protoc.executable.path")
if (coreProtocExecPath.isDefined) {
val sparkProtocExecPath = sys.props.get("spark.protoc.executable.path")
if (sparkProtocExecPath.isDefined) {
Seq(
PB.protocExecutable := file(coreProtocExecPath.get)
PB.protocExecutable := file(sparkProtocExecPath.get)
)
} else {
Seq.empty
Expand Down Expand Up @@ -722,15 +716,15 @@ object SparkConnectCommon {
case _ => MergeStrategy.first
}
) ++ {
val connectProtocExecPath = sys.props.get("connect.protoc.executable.path")
val sparkProtocExecPath = sys.props.get("spark.protoc.executable.path")
val connectPluginExecPath = sys.props.get("connect.plugin.executable.path")
if (connectProtocExecPath.isDefined && connectPluginExecPath.isDefined) {
if (sparkProtocExecPath.isDefined && connectPluginExecPath.isDefined) {
Seq(
(Compile / PB.targets) := Seq(
PB.gens.java -> (Compile / sourceManaged).value,
PB.gens.plugin(name = "grpc-java", path = connectPluginExecPath.get) -> (Compile / sourceManaged).value
),
PB.protocExecutable := file(connectProtocExecPath.get)
PB.protocExecutable := file(sparkProtocExecPath.get)
)
} else {
Seq(
Expand Down Expand Up @@ -880,10 +874,10 @@ object SparkProtobuf {
case _ => MergeStrategy.first
},
) ++ {
val protobufProtocExecPath = sys.props.get("protobuf.protoc.executable.path")
if (protobufProtocExecPath.isDefined) {
val sparkProtocExecPath = sys.props.get("spark.protoc.executable.path")
if (sparkProtocExecPath.isDefined) {
Seq(
PB.protocExecutable := file(protobufProtocExecPath.get)
PB.protocExecutable := file(sparkProtocExecPath.get)
)
} else {
Seq.empty
Expand Down

0 comments on commit f590f87

Please sign in to comment.