ADFA-2780: use root project directory to fetch git info#944
ADFA-2780: use root project directory to fetch git info#944itsaky-adfa merged 7 commits intostagefrom
Conversation
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
📝 WalkthroughRelease Notes
Risks & Best Practices Violations
Recommendations
WalkthroughCI metadata resolution was made project-aware: branch and commit getters now accept a Gradle Project parameter; command execution was centralized into a project-scoped helper. Build-info generation and publishing version now call the new project-aware CI methods. Changes
Sequence Diagram(s)sequenceDiagram
participant Gradle as Gradle build (build.gradle.kts)
participant CI as CI.kt
participant Project as Project (cmd runner)
participant Git as Git / Environment
Gradle->>CI: call CI.branchName(project)
CI->>Project: Project.cmdOutput("git", "rev-parse", "--abbrev-ref", "HEAD")
Project->>Git: execute git command in repo
Git-->>Project: branch name or null
Project-->>CI: command output (trimmed) or null
CI-->>Gradle: resolved branch name
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt`:
- Around line 70-93: The Project.cmdOutput function ignores the provided workDir
and hardcodes File("."), and the error message prints the raw args array; fix
cmdOutput by passing the workDir parameter to ProcessBuilder.directory(...)
instead of File("."), and change the RuntimeException / logger messages to
format the command using args.joinToString(" ") (or args.contentToString()) so
the executed command is human-readable; update references in Project.cmdOutput
accordingly (the method signature, ProcessBuilder(...) block, and the error/log
lines).
🧹 Nitpick comments (2)
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt (2)
30-47: Thread-safety: cached fields are unguarded mutable state.
commitHashandbranchNameare written fromcommitHash(project)/branchName(project)without synchronization. Gradle can configure projects in parallel (org.gradle.parallel=true), so concurrent calls could race. In practice the computed values will be the same, so this is low-risk, but worth noting.A simple fix would be to use
@Volatileor a synchronized block, or to restructure as alazy-like pattern per the existingisCiBuild/isTestEnvproperties.
22-22: Remove unnecessary importkotlin.getOrDefault.The
getOrDefaultextension function is a standard Kotlin stdlib member onResult<T>and is automatically available at line 93 without an explicit import. Extension functions do not require top-level imports—they're resolved based on their receiver type.
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt
Show resolved
Hide resolved
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt`:
- Around line 84-93: The current code reads process output after calling
process.waitFor(), which can deadlock if the child fills its stdout/stderr
buffers; change the execution in the helper (the block using process, args and
failOnError) to consume stdout and stderr concurrently before or while waiting:
either set redirectErrorStream(true) and read process.inputStream into a
StringBuilder on a separate thread (or use Kotlin coroutine/async) and capture
the output, or spawn two reader threads to read process.inputStream and
process.errorStream into strings, then waitFor() and check exitCode (throwing
the RuntimeException with args and exitCode if failOnError), returning the
captured trimmed stdout; ensure readers are closed/used safely to avoid leaks.
- Around line 84-87: The current logic treats non-zero process exits as valid
results by returning trimmed output even when failOnError is false; change the
behavior in the method around process.waitFor() so that after val exitCode =
process.waitFor() you check if exitCode != 0 and in that case return null (or an
explicit failure sentinel) and only additionally throw when failOnError is true
(i.e., if exitCode != 0 { if (failOnError) throw RuntimeException("Command
'$args' failed with exit code $exitCode") else return null }). Update callers
that expect values like commitHash to handle null accordingly.
🧹 Nitpick comments (1)
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt (1)
30-48: Minor thread-safety concern on cached fields.
commitHashandbranchNameare unsynchronized mutable fields on a singletonobject. In Gradle's parallel configuration phase, two projects could race and both spawn the git process. The result is benign (same value written twice), but you could avoid the redundant work with@Synchronizedor a simple lock.
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt
Show resolved
Hide resolved
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt
Show resolved
Hide resolved
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
See ADFA-2780 for more details.