Copilot commented on code in PR #4738:
URL: https://github.com/apache/bookkeeper/pull/4738#discussion_r3055179812
##########
native-io/pom.xml:
##########
@@ -85,141 +74,79 @@
<profiles>
<profile>
- <!-- from JDK10 javah command is not available
- see http://openjdk.java.net/jeps/313
+ <!--
+ Cross-compile for Linux amd64 + arm64 (glibc) via cargo-zigbuild.
+ Produces a single JAR containing two variants:
+ lib/rust/linux-x86_64-gnu/libnative-io.so (glibc, amd64)
+ lib/rust/linux-aarch64-gnu/libnative-io.so (glibc, arm64)
+
+ Prerequisites: zig, cargo-zigbuild, and both Rust targets
+ added via `rustup target add`.
-->
- <id>jdk-without-javah</id>
+ <id>cargo-zigbuild</id>
<activation>
- <jdk>[10,)</jdk>
+ <activeByDefault>true</activeByDefault>
</activation>
<build>
<plugins>
<plugin>
- <groupId>com.github.maven-nar</groupId>
- <artifactId>nar-maven-plugin</artifactId>
- <extensions>true</extensions>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
<executions>
- <execution>
- <!-- javah is not present in JDK10 onwards,
- you have to to use javac -h -->
- <id>default-nar-javah</id>
- <phase>none</phase>
- </execution>
- </executions>
- </plugin>
- <plugin>
- <groupId>org.apache.maven.plugins</groupId>
- <artifactId>maven-compiler-plugin</artifactId>
- <configuration>
- <compilerArgs>
- <!-- add -h flag to javac -->
- <compilerArg>-h</compilerArg>
-
<compilerArg>${project.build.directory}/nar/javah-include</compilerArg>
- </compilerArgs>
- </configuration>
- </plugin>
- </plugins>
- </build>
- </profile>
- <profile>
- <id>mac</id>
- <activation>
- <os>
- <name>Mac OS X</name>
- </os>
- </activation>
- <build>
- <plugins>
- <plugin>
- <groupId>com.github.maven-nar</groupId>
- <artifactId>nar-maven-plugin</artifactId>
- <extensions>true</extensions>
- <configuration>
- <runtime>${nar.runtime}</runtime>
- <output>native-io</output>
- <libraries>
- <library>
- <type>jni</type>
-
<narSystemPackage>org.apache.bookkeeper.util.nativeio</narSystemPackage>
- </library>
- </libraries>
- <cpp>
- <optionSet>${nar.cpp.optionSet}</optionSet>
- <exceptions>false</exceptions>
- <rtti>false</rtti>
- <optimize>full</optimize>
- </cpp>
- </configuration>
- </plugin>
- </plugins>
- </build>
- </profile>
-
- <profile>
- <id>Linux</id>
- <activation>
- <os>
- <name>Linux</name>
- </os>
- </activation>
- <build>
- <plugins>
- <plugin>
- <groupId>com.github.maven-nar</groupId>
- <artifactId>nar-maven-plugin</artifactId>
- <extensions>true</extensions>
- <configuration>
- <runtime>${nar.runtime}</runtime>
- <output>native-io</output>
- <libraries>
- <library>
- <type>jni</type>
-
<narSystemPackage>org.apache.bookkeeper.util.nativeio</narSystemPackage>
- </library>
- </libraries>
- <cpp>
- <optionSet>${nar.cpp.optionSet}</optionSet>
- <exceptions>false</exceptions>
- <rtti>false</rtti>
- <optimize>full</optimize>
- </cpp>
- </configuration>
+ <execution>
+ <id>cargo-zigbuild-amd64-gnu</id>
+ <phase>compile</phase>
+ <goals><goal>exec</goal></goals>
+ <configuration>
+ <executable>${rust.executable}</executable>
+ <workingDirectory>${rust.dir}</workingDirectory>
+ <arguments>
+ <argument>zigbuild</argument>
+ <argument>--release</argument>
+ <argument>--target</argument>
+ <argument>${rust.zig.target.amd64.gnu}</argument>
+ </arguments>
+ </configuration>
+ </execution>
+ <execution>
+ <id>cargo-zigbuild-arm64-gnu</id>
+ <phase>compile</phase>
+ <goals><goal>exec</goal></goals>
+ <configuration>
+ <executable>${rust.executable}</executable>
+ <workingDirectory>${rust.dir}</workingDirectory>
+ <arguments>
+ <argument>zigbuild</argument>
+ <argument>--release</argument>
+ <argument>--target</argument>
+ <argument>${rust.zig.target.arm64.gnu}</argument>
+ </arguments>
+ </configuration>
+ </execution>
+ </executions>
</plugin>
- </plugins>
- </build>
- </profile>
- <profile>
- <id>Windows</id>
- <activation>
- <os>
- <family>Windows</family>
- </os>
- </activation>
- <build>
- <plugins>
<plugin>
- <groupId>com.github.maven-nar</groupId>
- <artifactId>nar-maven-plugin</artifactId>
- <extensions>true</extensions>
- <configuration>
- <runtime>${nar.runtime}</runtime>
- <output>native-io</output>
- <libraries>
- <library>
- <type>jni</type>
-
<narSystemPackage>org.apache.bookkeeper.util.nativeio</narSystemPackage>
- </library>
- </libraries>
- <cpp>
- <optionSet>${nar.cpp.optionSet}</optionSet>
- <exceptions>false</exceptions>
- <rtti>false</rtti>
- <optimize>full</optimize>
- </cpp>
- <linker>
- <name>g++</name>
- </linker>
- </configuration>
+ <artifactId>maven-antrun-plugin</artifactId>
+ <version>3.1.0</version>
+ <executions>
+ <execution>
+ <id>copy-zig-libs</id>
+ <phase>process-classes</phase>
+ <goals><goal>run</goal></goals>
+ <configuration>
+ <target>
+ <mkdir dir="${rust.lib.dir}/rust/linux-x86_64-gnu"/>
+ <mkdir dir="${rust.lib.dir}/rust/linux-aarch64-gnu"/>
+ <copy
file="${rust.target.dir}/${rust.zig.output.target.amd64.gnu}/${rust.release.lib.relative.path}"
+
tofile="${rust.lib.dir}/rust/linux-x86_64-gnu/libnative-io.so"
+ failonerror="true" verbose="true"/>
+ <copy
file="${rust.target.dir}/${rust.zig.output.target.arm64.gnu}/${rust.release.lib.relative.path}"
Review Comment:
The copy step uses `${rust.zig.output.target.*}` (`x86_64-unknown-linux-gnu`
/ `aarch64-unknown-linux-gnu`) to locate the built artifacts, but the build
executions use `${rust.zig.target.*}` (`...-gnu.2.17`). Cargo’s target output
directory is based on the exact `--target` string, so this mismatch is likely
to make the `<copy>` fail (library will be under `target/<...gnu.2.17>/...`).
Consider using the same value for both (or deriving the copy path directly from
`${rust.zig.target.*}`) so the packaged JAR reliably includes the JNI .so files.
```suggestion
<copy
file="${rust.target.dir}/${rust.zig.target.amd64.gnu}/${rust.release.lib.relative.path}"
tofile="${rust.lib.dir}/rust/linux-x86_64-gnu/libnative-io.so"
failonerror="true" verbose="true"/>
<copy
file="${rust.target.dir}/${rust.zig.target.arm64.gnu}/${rust.release.lib.relative.path}"
```
##########
native-io/pom.xml:
##########
@@ -24,13 +24,21 @@
</parent>
<artifactId>native-io</artifactId>
- <packaging>nar</packaging>
+ <packaging>jar</packaging>
<name>Apache BookKeeper :: Native IO Library</name>
<description>Native IO Library</description>
<properties>
- <nar.runtime>dynamic</nar.runtime>
- <nar.cpp.optionSet>-msse4.2 -mpclmul</nar.cpp.optionSet>
+ <rust.dir>${project.basedir}/src/main/native-io-jni/rust</rust.dir>
+ <rust.target.dir>${rust.dir}/target</rust.target.dir>
+ <rust.executable>cargo</rust.executable>
+
<rust.release.lib.relative.path>release/deps/libnative_io.so</rust.release.lib.relative.path>
Review Comment:
`rust.release.lib.relative.path` is set to `release/deps/libnative_io.so`,
but Cargo typically emits `cdylib` artifacts at
`target/<triple>/release/libnative_io.so` (not under `release/deps/`). If this
path is wrong, the subsequent packaging step will fail to find the built
library. Consider updating this property to the actual cdylib output path
produced by `cargo zigbuild` for this crate.
```suggestion
<rust.release.lib.relative.path>release/libnative_io.so</rust.release.lib.relative.path>
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
