Sixteen kb pagesize #5
Conversation
WalkthroughThis PR updates root build tooling (Kotlin → 2.1.20, AGP → 8.5.1, Gradle wrapper → 8.7), adds gradlePluginPortal() and removes the Bintray plugin entry, raises Android and Java toolchain targets to Java 17 across modules, adds the org.jreleaser plugin and refactors publishing to Maven publications with signing/staging, upgrades NDK/CMake, introduces a GitHub Actions workflow that verifies 16KB alignment of native libraries and archives artifacts, and adds a README section for publishing to Maven Central. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Gradle as Gradle/gradlew
participant NDK as Android NDK (llvm-readelf)
participant Artifacts as Artifact Storage
GH->>Repo: checkout code
GH->>GH: setup JDK 17 & Android SDK/NDK
GH->>Gradle: ./gradlew assemble / build
Gradle-->>GH: build outputs (.so, .aar)
GH->>NDK: run llvm-readelf on each .so (inspect LOAD segments)
NDK-->>GH: report segment sizes
alt all 16KB aligned
GH->>Artifacts: upload AARs/.so/lint artifacts
GH-->>Repo: success
else any misaligned
GH-->>Repo: fail job with alignment summary
end
sequenceDiagram
participant Dev as Developer
participant Gradle as Gradle Build
participant JReleaser as JReleaser
participant Sonatype as Maven Central (staging)
Dev->>Gradle: publish mavenJava (sources/javadoc attached)
Gradle->>JReleaser: invoke release/deploy
JReleaser->>Sonatype: create staging repo, upload signed artifacts
Sonatype-->>JReleaser: staging response
JReleaser-->>Dev: publish result (staged/released)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dashj-x11/build.gradle (1)
171-221: Remove large commented-out legacy deployment code.This 50+ line block of old deployment configuration should be removed rather than kept as commented code. It adds maintenance burden and clutters the file.
Delete lines 171-221 entirely.
🧹 Nitpick comments (4)
dashj-x11/build.gradle (1)
42-45: Consider adding withSourcesJar() to publishing block for consistency.The publishing.singleVariant block at lines 42-45 is empty (or only has withSourcesJar removed). While the sources and javadoc JARs are manually added to the publication at lines 87-88, adding
withSourcesJar()to the singleVariant block would align with dashj-scrypt and follow Android Gradle Plugin best practices.publishing { singleVariant("release") { + withSourcesJar() } }dashj-bls/build.gradle (2)
59-60: Inconsistency: withSourcesJar/withJavadocJar commented out but artifacts still added manually.The publishing.singleVariant block has withSourcesJar/withJavadocJar commented out (lines 59-60), yet the publication block (lines 115-116) explicitly adds these artifacts. This is inconsistent and makes the intent unclear.
Either:
- Uncomment lines 59-60 and let the
singleVariantblock handle source/javadoc declaration, or- Keep them commented and rely on manual
artifactdeclarations in the publicationChoose one approach for clarity. Option 1 is more idiomatic for Android Gradle Plugin 8.5.1+:
publishing { singleVariant("release") { // if you don't want sources/javadoc, remove these lines - //withSourcesJar() - //withJavadocJar() + withSourcesJar() + // Note: withJavadocJar omitted due to empty stub; fix javadocJar task first } }
77-83: Javadoc task with failOnError false may hide build issues.The javadoc task uses
failOnError false(line 82), which silently suppresses javadoc generation errors. Combined with the empty javadocJar stub (lines 85-88), this creates a situation where broken javadoc will go unnoticed.Set
failOnError trueand address any javadoc errors, so that the build properly fails if documentation cannot be generated:- failOnError false + failOnError trueIf javadoc generation is deliberately disabled, add a clear comment explaining why and whether it should be generated in the future.
dashj-scrypt/build.gradle (1)
65-136: Remove large commented-out code section (lines 65-136).The 72-line block of old publishing configuration is entirely commented out and can be safely deleted. Active code immediately follows, and no dependencies reference this commented section. Removing it eliminates technical debt.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
build.gradle(1 hunks)dashj-bls/build.gradle(6 hunks)dashj-scrypt/build.gradle(5 hunks)dashj-x11/build.gradle(5 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)
🔇 Additional comments (5)
build.gradle (1)
4-4: Major version bumps require verification across the full build.Kotlin and AGP have significant version increases (1.5.20→1.9.10 and 7.2.2→8.5.1). While individually reasonable, verify that all submodules and build tasks work correctly with these combinations, especially given the concurrent Gradle 8.7 upgrade.
Have you validated that the full build completes successfully with these toolchain versions? If CI tests exist, confirm they pass on the target branch.
Also applies to: 11-11
dashj-scrypt/build.gradle (1)
188-188: Verify staging-deploy directory works with jreleaser sonatype configuration.The publishing repository (line 188) uses
layout.buildDirectory.dir('staging-deploy')as a local staging directory, while jreleaser sonatype config (lines 218-223) also referencesstagingRepository("$buildDir/staging-deploy"). Ensure the jreleaser integration properly consumes the artifacts from this directory during Maven Central deployment.Confirm that the local staging directory path matches between publishing (line 188) and jreleaser (line 222), and that jreleaser correctly reads from this location during the release workflow.
Also applies to: 199-223
dashj-x11/build.gradle (1)
86-88: Publication includes correct component and artifacts.The mavenJava publication properly includes
from components.release(line 86) along with sources and javadoc artifacts, which ensures the AAR library is published alongside documentation artifacts.dashj-bls/build.gradle (2)
111-120: Publication correctly includes component and proper URLs.The mavenJava publication includes
from components.release(line 114) and specifies correct URLs pointing to dashj-android. The artifactId correctly identifies this as dashj-bls-android module.
156-195: Verify version value used in jreleaser block.The jreleaser block at line 157 captures
def projectVersion = version, which will use whichever version is final after all declarations. Due to the version conflict at line 65, confirm thatprojectVersionresolves to the intended value (1.0.1-SNAPSHOT or 1.0.0).After resolving the version conflict at line 65, verify that jreleaser uses the correct version by checking the output of
./gradlew properties | grep versionor by inspecting the generated POM.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-and-verify-alignment.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/build-and-verify-alignment.yml
20-20: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
29-29: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
155-155: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
165-165: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
.github/workflows/build-and-verify-alignment.yml (1)
65-65: Verify robustness of alignment value extraction.Line 65 extracts alignment using
awk '{print $NF}', which assumes the last field is the alignment value. The parsing on lines 70-71 relies on bash's arithmetic expansion to convert hex (e.g.,0x4000) to decimal. While bash does support this, verify that:
- The
readelf -loutput consistently places alignment in the last field- All possible readelf output formats are handled (different NDK versions may differ)
- Consider adding error handling or logging the raw output if conversion fails
Add defensive logging to help debug issues:
# Check LOAD segments alignment local alignments=$($readelf -l "$lib_path" | grep "LOAD" | awk '{print $NF}') + echo " Raw alignments from readelf: $alignments" local all_aligned=true for alignment in $alignments; do # Convert hex to decimal - local decimal_alignment=$((alignment)) + local decimal_alignment=$((alignment)) || { + echo " ⚠️ Failed to parse alignment value: $alignment" + continue + }Also applies to: 70-71
| aar_files=$(find . -name "*.aar" -path "*/build/outputs/aar/*" -type f) | ||
| if [ -z "$aar_files" ]; then | ||
| echo "❌ No AAR files found" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Reconsider the failure condition for missing AAR files.
The workflow exits with error if no AAR files are found (line 134). This will cause CI failures for Android app projects that don't produce AARs (libraries produce AARs, but apps produce APKs). The AAR verification step should be optional or conditional.
Example fix:
if [ -z "$aar_files" ]; then
- echo "❌ No AAR files found"
- exit 1
+ echo "ℹ️ No AAR files found (this may be an app project rather than a library)"
+ exit 0
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| aar_files=$(find . -name "*.aar" -path "*/build/outputs/aar/*" -type f) | |
| if [ -z "$aar_files" ]; then | |
| echo "❌ No AAR files found" | |
| exit 1 | |
| fi | |
| aar_files=$(find . -name "*.aar" -path "*/build/outputs/aar/*" -type f) | |
| if [ -z "$aar_files" ]; then | |
| echo "ℹ️ No AAR files found (this may be an app project rather than a library)" | |
| exit 0 | |
| fi |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
dashj-bls/build.gradle (1)
83-86: CRITICAL: PopulatejavadocJarwith generated documentation.The
javadocJartask (lines 83–86) creates an empty JAR with only the classifier. It does not include the javadoc output generated by thejavadoctask above (lines 75–81). This means the published artifact will lack documentation.Populate the JAR with the javadoc content:
task javadocJar(type: Jar) { archiveClassifier = 'javadoc' - // Create empty javadoc jar for now + from javadoc.destinationDir }Alternatively, compare with dashj-x11/build.gradle lines 57–60, which correctly includes
from javadoc.destinationDir.dashj-scrypt/build.gradle (1)
157-159: CRITICAL: Uncommentfrom components.releaseto include AAR artifact.Line 157 has
from components.releasecommented out, which excludes the Android library component (AAR) from the Maven publication. Only sourcesJar and javadocJar will be published, making the AAR unavailable to Maven Central consumers.Uncomment line 157:
mavenJava(MavenPublication) { artifactId = 'dashj-scrypt-android' version "$version" - // from components.release + from components.release artifact sourcesJar artifact javadocJar
🧹 Nitpick comments (1)
dashj-scrypt/build.gradle (1)
65-137: Remove large block of commented-out legacy publishing code.Lines 65–137 contain ~70 lines of commented-out old publishing configuration. Since the new Maven publication model is now active, this legacy code should be removed to improve maintainability and reduce confusion.
Delete the commented-out legacy publishing block (lines 65–137) to clean up the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
build.gradle(1 hunks)dashj-bls/build.gradle(5 hunks)dashj-scrypt/build.gradle(5 hunks)dashj-x11/build.gradle(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- build.gradle
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-verify
🔇 Additional comments (2)
dashj-x11/build.gradle (1)
42-45: Verify emptysingleVariantblock is intentional.The
publishing.singleVariant("release")block is empty. Compare this to dashj-scrypt/build.gradle line 40–42, which includeswithSourcesJar(). Since you're explicitly addingartifact sourcesJarandartifact javadocJarto the publication at lines 87–88, the empty block may be intentional. However, verify that the release variant properly publishes sources; otherwise, align with the pattern used in dashj-scrypt.dashj-bls/build.gradle (1)
9-9: Verify version1.0.1-SNAPSHOTis intentional across all modules.This file declares version
1.0.1-SNAPSHOT, while dashj-x11 and dashj-scrypt both use1.0.0-SNAPSHOT. If these modules are released together, they should share the same version to maintain consistency and simplify release management.Confirm whether this version difference is intentional, or whether all modules should be synchronized to
1.0.0-SNAPSHOT(or another common version).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
dashj-bls/build.gradle (1)
83-86: CRITICAL: Empty javadocJar defeats Maven Central publishing requirements.The javadocJar task creates a JAR with only a classifier but no actual Javadoc content. The comment explicitly states "Create empty javadoc jar for now", which is a placeholder. This is the same issue flagged in the previous review and must be fixed before publishing.
Maven Central and quality standards require that javadoc JARs contain actual documentation. Compare this with dashj-x11/build.gradle (lines 57-60), which correctly includes the generated Javadoc.
Apply this diff to include the generated Javadoc:
-task javadocJar(type: Jar) { +task javadocJar(type: Jar, dependsOn: javadoc) { archiveClassifier = 'javadoc' - // Create empty javadoc jar for now + from javadoc.destinationDir }
🧹 Nitpick comments (1)
dashj-scrypt/build.gradle (1)
65-136: Large commented-out block should be removed or moved to version control history.Lines 65–136 contain ~70 lines of commented-out legacy publishing configuration. While this preserves migration context, it significantly bloats the file and should be removed entirely since git history preserves the previous implementation. If this reference is useful for future maintainers, consider documenting the migration path in a PR description or ADR (Architecture Decision Record) instead.
Remove the commented block to keep the build script lean:
-//artifacts { -// archives javadocJar -// archives sourcesJar -//} - - - -//afterEvaluate { [... entire 70-line block ...] -//} - -//task javadocJar(type: Jar, dependsOn: javadoc) { -// archiveClassifier = 'javadoc' -// from javadoc.destinationDir -//} -// -//task sourcesJar(type: Jar, dependsOn: classes) { -// archiveClassifier = 'sources' -// from sourceSets.main.allSource -// duplicatesStrategy = DuplicatesStrategy.EXCLUDE // or DuplicatesStrategy.WARN -//} - artifacts { archives sourcesJar archives javadocJar }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)build.gradle(1 hunks)dashj-bls/build.gradle(5 hunks)dashj-scrypt/build.gradle(5 hunks)dashj-x11/build.gradle(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-verify
🔇 Additional comments (8)
README.md (1)
13-20: Maven Central deployment documentation looks good.The new section clearly documents the jReleaser deployment workflow and aligns well with the publishing infrastructure changes in this PR (org.jreleaser plugin addition, Maven publication configuration, and staging setup).
build.gradle (1)
1-16: Version upgrades are properly positioned and necessary for downstream module changes.The Kotlin 2.1.20 and AGP 8.5.1 upgrades align with the Java 17 toolchain shift and JReleaser plugin support. The
gradlePluginPortal()addition correctly enables plugin resolution for JReleaser. No critical issues identified.dashj-x11/build.gradle (3)
42-45: Verify whether sources and javadoc should be included in the single variant.The
publishing.singleVariant("release")block does not callwithSourcesJar()orwithJavadocJar(). While this is handled separately via theartifactsblock (lines 67-70) and explicit publication (line 89from components.release), consider whether auto-inclusion viawithSourcesJar()andwithJavadocJar()would simplify the publishing pipeline and ensure consistency with Maven Central expectations.This is not a blocking issue if the manual artifact configuration is intentional, but it's worth verifying against your publishing requirements.
57-60: Javadoc artifact is properly populated.The javadocJar correctly depends on the javadoc task and includes the generated documentation directory. This ensures the published artifact contains Javadoc content.
96-97: Packaging override tojarmay conflict with AAR distribution.Lines 96-97 explicitly set
packaging = 'jar'in the POM to achieve Maven Central compatibility, and line 156 includes the commentapplyMavenCentralRules = false // must be false for aar. This creates a semantic inconsistency: the artifact is an AAR (Android library) but declared as JAR in the POM.Verify that Maven Central and consuming clients correctly handle this override. If the intent is to publish as a JAR, ensure the actual artifact produced by
from components.release(line 89) is JAR-compatible; otherwise, clarify the packaging mismatch in a comment.Also applies to: 122-123
dashj-bls/build.gradle (1)
107-114: Verify that projectVersion is correctly set and propagates to all deployment targets.The afterEvaluate block captures
versionintoprojectVersion(line 107) and uses it throughout the publication and JReleaser configuration. Confirm thatversion(set at line 9) is stable and not redefined elsewhere, and that this value is correctly reflected in the POM and JReleaser metadata sent to Maven Central / Sonatype.dashj-scrypt/build.gradle (2)
55-58: Javadoc artifact is properly configured.Unlike dashj-bls, this file correctly includes the generated Javadoc in the javadocJar. The task depends on javadoc and includes the output directory. This ensures Maven Central receives proper documentation.
156-160: Publication correctly includes Android library component and uses updated artifact ID.The
from components.releaseinclusion ensures the AAR is published. The artifactId 'dashj-scrypt-android' and dynamic version handling via projectVersion are properly set. The POM metadata references the correct GitHub URL.
| ``` | ||
|
|
||
| Publish to Maven Local | ||
| ### Publish to Maven Local |
There was a problem hiding this comment.
Fix markdown heading hierarchy.
The sections use H3 (###) immediately after H1 (#), but markdown heading levels should increment by one level at a time. Change both to H2 (##) for proper hierarchy.
-### Publish to Maven Local
+## Publish to Maven Local-### Publish to Maven Central
+## Publish to Maven CentralAlso applies to: 13-13
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
In README.md around lines 8 and 13, the headings jump from H1 (#) directly to H3
(###); change both H3 headings at those lines to H2 (##) so the markdown heading
hierarchy increments correctly by one level from the H1 and ensures consistent
structure.
| ``` | ||
| ./gradlew publishToMavenLocal | ||
| ``` |
There was a problem hiding this comment.
Add language specification to code block for consistency.
The bash code block is missing a language tag. Add bash to match the other code blocks in this file (lines 3 and 16).
-```
+```bash
./gradlew publishToMavenLocal
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In README.md around lines 9 to 11, the fenced code block for the gradle command
lacks a language tag; update the opening fence to include "bash" (i.e., change
``` to ```bash) so the block is consistent with other language-tagged blocks in
the file and save the file.
Summary by CodeRabbit