jbonofre commented on code in PR #1383:
URL: https://github.com/apache/arrow-java/pull/1383#discussion_r4493473595


##########
dev/release/source-release.xml:
##########
@@ -0,0 +1,105 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<!--
+  Assembly descriptor for the arrow-java-root-*-source-release.zip artifact
+  built by the apache-release profile.
+
+  This is the same as the "source-release" descriptor shipped in
+  org.apache.apache.resources:apache-source-release-assembly-descriptor:1.8
+  (used by the Apache parent POM), except that it also excludes jni/.
+-->
+<assembly xmlns="http://maven.apache.org/ASSEMBLY/2.2.0"; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; 
xsi:schemaLocation="http://maven.apache.org/ASSEMBLY/2.2.0 
https://maven.apache.org/xsd/assembly-2.2.0.xsd";>
+  <id>source-release</id>
+  <formats>
+    <format>zip</format>
+  </formats>
+  <fileSets>
+    <!-- main project directory structure -->
+    <fileSet>
+      <directory>.</directory>
+      <outputDirectory></outputDirectory>
+      <useDefaultExcludes>true</useDefaultExcludes>
+      <excludes>

Review Comment:
   The same job leaves a few other non source files in the working directory. 
They are also in the 19.0.0 zip and are not covered here:
   
   - `apache-arrow-java-19.0.0.tar.gz`: the git archive source release, nested 
inside this one
   - `artifacts/release-source/*.sha256` and `*.sha512`, plus empty 
`artifacts/jni-*/` directories
   - an empty `binaries/` directory: the dist dir `jni_full_build.sh` creates 
before running Maven
   
   We could add an exclude for each of these, but then the list has to track 
whatever the CI job happens to leave behind.



##########
dev/release/source-release.xml:
##########
@@ -0,0 +1,105 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<!--
+  Assembly descriptor for the arrow-java-root-*-source-release.zip artifact
+  built by the apache-release profile.
+
+  This is the same as the "source-release" descriptor shipped in
+  org.apache.apache.resources:apache-source-release-assembly-descriptor:1.8
+  (used by the Apache parent POM), except that it also excludes jni/.
+-->
+<assembly xmlns="http://maven.apache.org/ASSEMBLY/2.2.0"; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; 
xsi:schemaLocation="http://maven.apache.org/ASSEMBLY/2.2.0 
https://maven.apache.org/xsd/assembly-2.2.0.xsd";>
+  <id>source-release</id>
+  <formats>
+    <format>zip</format>
+  </formats>
+  <fileSets>
+    <!-- main project directory structure -->
+    <fileSet>
+      <directory>.</directory>
+      <outputDirectory></outputDirectory>
+      <useDefaultExcludes>true</useDefaultExcludes>
+      <excludes>
+        <!-- Pre-built JNI libraries that release CI puts in jni/ before
+             building the release artifacts. They are binaries, not source,
+             and are several hundred MB. See GH-743. -->
+        <exclude>jni/**</exclude>

Review Comment:
   This covers the extracted `jni/` directory, but the `binaries` job in 
`rc.yml` also leaves the compressed tarballs at the repo root and never deletes 
them:
   
   ```
   mv artifacts/*/*.tar.gz .
   tar -xf apache-arrow-java-*.tar.gz --strip-components=1
   tar -xvzf jni-linux-x86_64.tar.gz
   ...
   ```
   
   So they are still packaged. From the published 19.0.0 zip:
   
   ```
   61.6 MB  jni-linux-aarch_64.tar.gz
   66.0 MB  jni-linux-x86_64.tar.gz
   36.5 MB  jni-macos-aarch_64.tar.gz
   44.8 MB  jni-macos-x86_64.tar.gz
   14.6 MB  jni-windows-x86_64.tar.gz
   ```
   
   With the current matrix, the next RC zip would still carry about 100MB of 
the same native libraries, just tarred. The local test with a fake `jni/` 
directory doesn't reproduce the CI layout, which is why this didn't show up.
   
   NB: I don't think we should consider that for 20.0.0, we can do the cleanup 
after this release.



##########
pom.xml:
##########
@@ -786,6 +786,32 @@ under the License.
   </reporting>
 
   <profiles>
+    <profile>
+      <id>apache-release</id>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-assembly-plugin</artifactId>
+            <executions>
+              <execution>
+                <id>source-release-assembly</id>
+                <configuration>
+                  <!-- Use our own descriptor instead of the Apache parent's
+                       "source-release" one so that pre-built JNI libraries
+                       in jni/ aren't included in the source release. -->
+                  <descriptorRefs combine.self="override"/>

Review Comment:
   Rather than forking the descriptor, I would fix this where the files come 
from. The `source-release` descriptor zips the whole working directory, and the 
`binaries` job unpacks its downloads into that same directory. Two options:
   
   1. Build from a clean subdirectory in `rc.yml`, so downloads and outputs sit 
next to the source tree instead of inside it:
   ```
   mv artifacts/*/*.tar.gz .
   mkdir arrow-java
   tar -xf apache-arrow-java-*.tar.gz -C arrow-java --strip-components=1
   tar -xvzf jni-linux-x86_64.tar.gz
   ...
   ```
   then run `ci/scripts/jni_full_build.sh arrow-java jni binaries`, checkout 
`apache/arrow-testing` to `arrow-java/testing`, and point "Prepare docs" at 
`arrow-java/target/site/apidocs`. That handles the tarballs, `artifacts/`, 
`binaries/` and the nested source tarball in one go.
   
   2. Set `<skipAssembly>true</skipAssembly>` on this execution, as 
`bom/pom.xml` already does. The source release we vote on and publish to 
dist.apache.org is the `git archive` tarball.
   
   Either way we drop `dev/release/source-release.xml`. Otherwise we would have 
to keep it in sync with the upstream descriptor on every `org.apache:apache` 
parent bump. 19.0.0 already leaked `.mvn/.develocity/` because of that kind of 
drift.



-- 
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]

Reply via email to