ADFA-2781: use protoc from PATH when building on-device

[itsaky-adfa]

See ADFA-2781 for more details.

[coderabbitai]

📝 Walkthrough

Release Notes

Features

• On-device protoc support: Implements detection and use of protoc executable from system PATH for on-device builds (Termux environment), with fallback to Maven artifact-based protoc for standard Android builds
• Shell utilities: Adds ShellUtils module providing shell command execution capabilities (which() and shC() functions) for system-level operations

Changes

• New protoc configuration infrastructure: Introduces centralized configureProtoc() function that intelligently selects between on-device and artifact-based protoc based on build environment
• Gradle dependency updates: Adds protobuf Gradle plugin (com.google.protobuf:protobuf-gradle-plugin) to dependency management
• Build file refactoring: Updates aapt2-proto and project-models subprojects to use centralized protoc configuration
• Signing warning suppression: Improves SigningConfigPlugin to emit signing-info warning only once instead of on every build

Risks & Best Practices Considerations

• ⚠️ Shell command execution: New shell utilities execute arbitrary commands via /bin/sh -c. Ensure proper input validation and sanitization to prevent command injection vulnerabilities
• ⚠️ Platform-specific logic: On-device detection relies on Termux package name and JDK path checks. This approach may be fragile across different Termux configurations or updates
• ⚠️ File system assumptions: The which() function depends on PATH environment variable availability and shell behavior, which may vary across development environments and CI/CD systems
• ⚠️ Error handling: The functions return null on failure; callers should explicitly handle null cases rather than relying on implicit defaults

Testing Recommendations

• Validate shell execution behavior across different system configurations (Linux, macOS, Windows with WSL)
• Test on-device protoc detection in actual Termux environment
• Verify fallback to Maven artifact works correctly when on-device protoc is unavailable
• Ensure build reproducibility and consistency across different developer machines and CI/CD pipelines

Walkthrough

This PR introduces shell command execution utilities and centralizes protoc configuration for protobuf builds. It adds ShellUtils for running shell commands, creates ProtocConf to handle protoc setup (both on-device and via Maven artifacts), upgrades SigningConfigPlugin to suppress repeated warnings, and updates build configurations across the project.

Changes

Cohort / File(s)	Summary
Shell Execution Utilities composite-builds/build-logic/common/src/.../ShellUtils.kt	New utility object providing which() and shC() functions for executing shell commands, with support for working directories and error stream redirection.
Protoc Configuration composite-builds/build-logic/plugins/src/.../conf/ProtocConf.kt, subprojects/aapt2-proto/build.gradle.kts, subprojects/project-models/build.gradle.kts	New centralized protoc configuration logic that detects on-device protoc (Termux) or falls back to Maven artifacts. Subprojects updated to use configureProtoc() instead of inline protoc artifact specifications.
Build Configuration & Dependencies composite-builds/build-logic/plugins/build.gradle.kts, gradle/libs.versions.toml	Protobuf Gradle plugin dependency added and wired into build configuration with formatting improvements.
Plugin Improvements composite-builds/build-logic/plugins/src/.../SigningConfigPlugin.kt, composite-builds/build-logic/plugins/src/.../util/SigningKeyUtils.kt	SigningConfigPlugin now suppresses repeated warnings via a companion object flag; SigningKeyUtils internal map renamed for consistency.

Sequence Diagram(s)

sequenceDiagram
    participant Gradle as Gradle Build
    participant ProtocConf as ProtocConf
    participant ShellUtils as ShellUtils
    participant FileSystem as File System
    participant ProtobufExt as Protobuf Extension

    Gradle->>ProtocConf: configureProtoc(protobuf, protocVersion)
    ProtocConf->>ProtocConf: isTermuxAppPackageNameSet() && isTermuxJdk()?
    
    alt On-Device Protoc Available
        ProtocConf->>ShellUtils: which("protoc")
        ShellUtils->>FileSystem: Execute: which protoc
        FileSystem-->>ShellUtils: protoc path or null
        ShellUtils-->>ProtocConf: protoc path
        ProtocConf->>FileSystem: Verify file exists & executable
        FileSystem-->>ProtocConf: true/false
        ProtocConf->>ProtobufExt: protoc { path = "/path/to/protoc" }
    else Use Maven Artifact
        ProtocConf->>ProtocConf: configureProtocArtifact()
        ProtocConf->>ProtobufExt: protoc { artifact = "com.google.protobuf:protoc:version" }
    end
    
    ProtobufExt-->>Gradle: Configuration complete

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• Daniel-ADFA
• jomen-adfa
• jatezzz

Poem

🐰 Hops and cheers for shells so bright,
Protoc configured left and right,
From Termux paths to Maven's store,
Build logic better than before!
With warnings hushed and code so neat, ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main objective of the PR: enabling the use of protoc from the system PATH when building on-device, which aligns with the key changes introducing ShellUtils for shell command execution and ProtocConf for on-device protoc detection.
Description check	✅ Passed	The description references the ADFA-2781 ticket for additional details and is related to the changeset, though it provides minimal inline detail about the implementation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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-2781

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/SigningConfigPlugin.kt (1)

72-83: The let block returns null in the else branch, which is silently discarded.

The extensions.getByType(...).let { ... } expression evaluates to null on the else branch (line 81), but the result is never used. Consider using run or also to make the intent clearer, or simply use an if/else statement instead of let to avoid the dangling null.

composite-builds/build-logic/common/src/main/java/com/itsaky/androidide/build/config/ShellUtils.kt (2)

28-36: Reading stdout after waitFor() can deadlock for commands with large output.

proc.waitFor() blocks until the process exits, but if the process writes enough to stdout to fill the OS pipe buffer (~64KB), it will block waiting for the reader — creating a deadlock. This is safe for the current which usage, but shC is a public general-purpose API.

Read the stream before (or concurrently with) waitFor():

Proposed fix

-		val exitCode = proc.waitFor()
-		if (exitCode != 0) {
-			return null
-		}
-
-		return proc.inputStream
-			.bufferedReader()
-			.readText()
-			.trim()
+		val output = proc.inputStream
+			.bufferedReader()
+			.readText()
+
+		val exitCode = proc.waitFor()
+		if (exitCode != 0) {
+			return null
+		}
+
+		return output.trim()

---

17-28: No timeout on waitFor() — a hung command will block the Gradle build indefinitely.

Consider adding a timeout to prevent indefinite hangs during configuration:

Proposed fix

-		val exitCode = proc.waitFor()
+		val finished = proc.waitFor(30, java.util.concurrent.TimeUnit.SECONDS)
+		if (!finished) {
+			proc.destroyForcibly()
+			return null
+		}
+		val exitCode = proc.exitValue()

composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/ProtocConf.kt (1)

73-77: Consider making these detection functions internal rather than public top-level.

isTermuxAppPackageNameSet() and isTermuxJdk() are implementation details of the on-device detection logic. Unless they're needed outside this module, narrowing their visibility would keep the public API surface smaller.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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