Skip to content

ADFA-2780: use root project directory to fetch git info#944

Merged
itsaky-adfa merged 7 commits intostagefrom
fix/ADFA-2780
Feb 13, 2026
Merged

ADFA-2780: use root project directory to fetch git info#944
itsaky-adfa merged 7 commits intostagefrom
fix/ADFA-2780

Conversation

@itsaky-adfa
Copy link
Contributor

@itsaky-adfa itsaky-adfa commented Feb 9, 2026

See ADFA-2780 for more details.

Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@itsaky-adfa itsaky-adfa requested review from a team and jomen-adfa February 9, 2026 11:49
@itsaky-adfa itsaky-adfa self-assigned this Feb 9, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Release Notes

  • Modified git metadata resolution to use the Gradle project context: branch and commit info are now retrieved via CI.branchName(project) and CI.commitHash(project) so CI metadata is resolved relative to the Gradle Project.
  • Updated build-info generation (build-info/build.gradle.kts) and ProjectConfig.publishingVersion to pass the current Project when querying CI metadata.
  • Added CI API surface:
    • commitHash(project: Project): String
    • branchName(project: Project): String
    • New lazy properties: isCiBuild and isTestEnv for CI/test environment detection.
  • Centralized command execution: introduced Project.cmdOutput(workDir, failOnError, vararg args) which returns trimmed output or null on failure.
  • Improved robustness: git command failures return null and are handled (falling back to "unknown") instead of causing the build to fail.

Risks & Best Practices Violations

  • ⚠️ workDir misuse: Project.cmdOutput accepts a workDir parameter but the implementation currently runs commands from File("."), so commands may execute from the wrong directory. This can return incorrect git metadata when Gradle is invoked from subprojects or non-root working directories.
  • ⚠️ Caching & thread-safety: CI-level caching of branch/commit values uses mutable state/lazy init that may not be safe under parallel or multi-project Gradle builds, risking race conditions or inconsistent values across projects.
  • ⚠️ Insufficient diagnostic logging: When git commands fail the code falls back to "unknown" and may not log command, working directory, exit code, and stderr with sufficient detail, making debugging harder.
  • ⚠️ API clarity: The unused/ignored workDir parameter is confusing; keeping it without functioning behavior increases maintenance burden and invites misuse.

Recommendations

  • Use the provided workDir when creating the ProcessBuilder (e.g., project.rootProject.projectDir or the specific project.projectDir) so git commands run from the intended repository directory.
  • Make caching thread-safe and per-project (e.g., compute-once with synchronization or store results in per-project Gradle properties) to avoid race conditions in parallel builds.
  • Enhance error logging to include the invoked command, resolved working directory, exit code, stdout/stderr to aid troubleshooting.
  • Remove or clearly document the workDir parameter if its use is unnecessary to avoid developer confusion.

Walkthrough

CI 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

Cohort / File(s) Summary
CI Configuration API
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt
Rewrote CI helpers to be project-scoped: added commitHash(project: Project) and branchName(project: Project), added val isCiBuild and val isTestEnv, introduced Project.cmdOutput(...) for command execution with logging/error handling, and removed prior parameterless lazy getters.
Build info / Project config updates
build-info/build.gradle.kts, composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/ProjectConfig.kt
Replaced parameterless calls CI.branchName() / CI.commitHash() with project-aware calls CI.branchName(project) / CI.commitHash(project) in build-info generation and publishing-version construction.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • jomen-adfa
  • jatezzz

Poem

🐰 I hopped through Gradle fields at dawn,

chased branchy trails and short-hash brawn,
Now every project knows its place,
CI and I keep tidy pace,
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ⚠️ Unable to check for merge conflicts: Stream setup permanently failed: 14 UNAVAILABLE: read ECONNRESET
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: updating CI-related getters to use the root project directory when fetching git information.
Description check ✅ Passed The description references the associated issue (ADFA-2780) which provides context for the changes, making it sufficiently related to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/ADFA-2780
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch fix/ADFA-2780
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/CI.kt (1)

30-48: Thread-unsafe caching on a singleton object.

commitHash and branchName are plain vars checked and set without synchronization. Gradle can evaluate build scripts in parallel, so two threads could race through the == null check and both execute the git command. The result is deterministic so this is unlikely to cause incorrect behavior, but it's a latent defect worth cleaning up—e.g., using a synchronized block or Kotlin lazy with a project-based initializer.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

commitHash and branchName are written from commitHash(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 @Volatile or a synchronized block, or to restructure as a lazy-like pattern per the existing isCiBuild/isTestEnv properties.


22-22: Remove unnecessary import kotlin.getOrDefault.

The getOrDefault extension function is a standard Kotlin stdlib member on Result<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.

Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

commitHash and branchName are unsynchronized mutable fields on a singleton object. 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 @Synchronized or a simple lock.

@itsaky-adfa itsaky-adfa merged commit 9a60900 into stage Feb 13, 2026
2 checks passed
@itsaky-adfa itsaky-adfa deleted the fix/ADFA-2780 branch February 13, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants