Repository navigation
Conversation
Release CI unpacks JNI tarballs into the working tree before Maven runs, so the parent's source-release assembly would package those binaries (hundreds of MB). Set skipAssembly on the root source-release-assembly execution (same pattern as bom/pom.xml) and drop the forked descriptor. The artifact we vote on and publish remains the git-archive tarball. Closes apache#743. AI assistance disclosure: prepared with an AI coding assistant; reviewed locally.
|
The "Ensure PR format" check is only waiting for a label. Could a maintainer add |
| <!-- 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> |
There was a problem hiding this 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.
There was a problem hiding this comment.
Addressed in f5ed5ef by skipping the root source-release assembly (option 2) instead of extending an exclude list. With the zip no longer built, the jni-*.tar.gz files that rc.yml leaves at the repo root are not packaged. Understood that this can wait until after 20.0.0. (AI-assisted reply.)
| <directory>.</directory> | ||
| <outputDirectory></outputDirectory> | ||
| <useDefaultExcludes>true</useDefaultExcludes> | ||
| <excludes> |
There was a problem hiding this 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 oneartifacts/release-source/*.sha256and*.sha512, plus emptyartifacts/jni-*/directories- an empty
binaries/directory: the dist dirjni_full_build.shcreates 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.
There was a problem hiding this comment.
Same fix as the sibling thread (f5ed5ef): with skipAssembly there is no zip, so the nested apache-arrow-java-*.tar.gz, the artifacts/ checksums and the empty artifacts/jni-*/ and binaries/ directories are no longer packaged either, and there is no exclude list to keep in step with the CI job. (AI-assisted reply.)
| <!-- 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"/> |
There was a problem hiding this 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:
- 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.
- Set
<skipAssembly>true</skipAssembly>on this execution, asbom/pom.xmlalready does. The source release we vote on and publish to dist.apache.org is thegit archivetarball.
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.
There was a problem hiding this comment.
Went with option 2 in f5ed5ef: the root source-release-assembly execution in the apache-release profile now sets <skipAssembly>true</skipAssembly>, the same way bom/pom.xml does, and dev/release/source-release.xml is removed. Locally, mvn -N -Papache-release package -DskipTests runs the execution without writing a zip to target/, and spotless:check passes. (AI-assisted reply.)
a427b8c to
f5ed5ef
Compare
|
@jbonofre thanks for the detailed review. I rebased on
I checked it with (AI-assisted reply.) |
ump45nose
changed the title
What's Changed
The
apache-releaseprofile buildsarrow-java-root-<version>-source-release.zipwith the Apache parent'ssource-releaseassembly descriptor, which zips the whole working directory. In the release CI (rc.ymlbinariesjob), the JNI tarballs, their extractedjni/directory, the nestedgit archivesource tarball,artifacts/checksums and thebinaries/dist directory all sit in that working directory before Maven runs, so they all end up in the zip (the 19.0.0 zip is ~455 MB).The source release we vote on and publish to dist.apache.org is the
git archivetarball, not this zip. Following the review, this sets<skipAssembly>true</skipAssembly>on the rootsource-release-assemblyexecution in theapache-releaseprofile, the same waybom/pom.xmlalready does. No zip is built, so no descriptor fork or exclude list has to track what the CI job leaves behind.Testing:
mvn -N -Papache-release package -DskipTests -Dgpg.skip. Thesource-release-assemblyexecution runs but writes no zip totarget/, and the effective POM showsskipAssembly=true.mvn -N spotless:checkpasses.AI assistance disclosure: this change and description were prepared with the help of an AI coding assistant; the checks above were run locally.
Closes #743.