ADFA-2602: Resolve buildscript classpaths from the on-device Maven repo - #1646
ADFA-2602: Resolve buildscript classpaths from the on-device Maven repo#1646Daniel-ADFA wants to merge 1 commit into
Conversation
COTGSettingsPlugin injects the bundled repo into pluginManagement and
dependencyResolutionManagement only. A `buildscript { }` block resolves
against its own repositories, which nothing reached, so any project
declaring its build classpath that way could only be built online. That
is why plugin-template builds required network.
Inject the repo for both scopes, each before the script that consumes it
is evaluated:
- beforeSettings, for a buildscript block in settings.gradle.kts
(the plugin template declares AGP and Kotlin there)
- beforeProject, for a buildscript block in build.gradle.kts
settingsEvaluated is too late: by then the settings buildscript classpath
has already resolved.
A missing repo is not fatal. The directory does not exist until onboarding
has installed the assets, and failing there would break every build before
that point.
itsaky-adfa
left a comment
itsaky-adfa
left a comment
There was a problem hiding this comment.
Looks good to me so far.
Since this is a draft, I'll re-review when it's ready for review.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
📝 Walkthrough
WalkthroughThe Gradle init script now injects the existing local Maven repository into settings- and project-level buildscript repositories. Missing repositories are logged and skipped. ChangesLocal Maven repository injection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt`:
- Line 97: Update the missing-repository log in the beforeSettings/beforeProject
helper to use the parameterized Logger.info overload: keep the repository value
as a placeholder argument and pass MAVEN_LOCAL_REPOSITORY separately instead of
interpolating it into the message.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e799fca5-d220-4b64-b0b1-b463247aed47
📒 Files selected for processing (2)
gradle-plugin/src/main/java/com/itsaky/androidide/gradle/AndroidIDEInitScriptPlugin.ktgradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt
| private fun localMavenRepoDir(logger: Logger): File? { | ||
| val dir = File(MAVEN_LOCAL_REPOSITORY) | ||
| if (!dir.isDirectory) { | ||
| logger.info("Local maven repo not installed yet, skipping buildscript injection: $MAVEN_LOCAL_REPOSITORY") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file ---'
sed -n '1,130p' gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt
printf '%s\n' '--- declared Gradle versions and logger usage ---'
rg -n -C 2 'gradle|GradleVersion|logger\.(info|lifecycle|warn|error|debug)\(' gradle-plugin gradle/libs.versions.toml build.gradle.kts settings.gradle.kts 2>/dev/null | head -240
printf '%s\n' '--- relevant tests ---'
rg -n -C 3 'COTGSettingsPlugin|MAVEN_LOCAL_REPOSITORY|Local maven repo' --glob '*Test*' --glob '*.kt' --glob '*.java' .Repository: appdevforall/CodeOnTheGo
Length of output: 30725
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- buildscript helper call sites ---'
rg -n -C 5 'addLocalMavenRepoToBuildscript|localMavenRepoDir' --glob '*.kt' --glob '*.java' .
printf '%s\n' '--- Gradle wrapper and plugin configuration ---'
rg -n -C 3 'distributionUrl|gradleApi|gradleTestKit|org\.gradle' gradle/wrapper gradle-plugin/build.gradle.kts settings.gradle.kts build.gradle.kts 2>/dev/null
printf '%s\n' '--- local Gradle API artifacts, if present ---'
find . -type f \( -name 'gradle-api-*.jar' -o -name 'gradle-*.jar' \) -print | head -40
printf '%s\n' '--- source-level verifier for the claimed interpolation and call shape ---'
python3 - <<'PY'
from pathlib import Path
p = Path("gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt")
s = p.read_text()
line = next((x for x in s.splitlines() if 'Local maven repo not installed yet' in x), '')
print("interpolated_message:", '$MAVEN_LOCAL_REPOSITORY' in line)
print("parameterized_call_present:", 'logger.info(' in line and '{}" ,' in line)
print("helper_call_count:", s.count('localMavenRepoDir(logger)'))
PYRepository: appdevforall/CodeOnTheGo
Length of output: 7380
🌐 Web query:
Gradle org.gradle.api.logging.Logger info(String, Object...) API
💡 Result:
The org.gradle.api.logging.Logger.info(String, Object...) method is part of the Gradle logging API, which extends the standard SLF4J Logger interface [1][2]. It is used to log messages at the 'info' level [1][3]. Usage and Details: - Functionality: This method logs a message at the info level, where the String parameter represents the log message, potentially containing placeholders (e.g., {}), and the Object... parameter provides the arguments to replace those placeholders [1][4][2]. - SLF4J Pattern: The method supports the standard SLF4J formatting pattern, allowing you to use curly braces as placeholders for values [4][2]. For example: logger.info("A {} log message", "info") [4][2]. - Availability: It is available within Gradle build scripts via the logger property and can also be used in custom classes (such as those in buildSrc) by obtaining an SLF4J logger instance [4][2]. - Security Note: Gradle advises caution when logging sensitive information (such as credentials, tokens, or environment variables). It is a security vulnerability to log such information at any level above debug [1][3]. The Logger interface is the primary entry point for Gradle's logging system and includes additional Gradle-specific log levels like 'quiet' and 'lifecycle', beyond the standard SLF4J levels [2][5][6].
Citations:
- 1: https://docs.gradle.org/current/javadoc/org/gradle/api/logging/Logger.html
- 2: https://docs.gradle.org/current/userguide/logging.html
- 3: https://docs.gradle.org/8.12/kotlin-dsl/gradle/org.gradle.api.logging/-logger/index.html
- 4: https://docs.gradle.org/8.1/userguide/logging.html
- 5: https://docs.gradle.org/9.0.0/kotlin-dsl/gradle/org.gradle.api.logging/-logger/index.html
- 6: https://docs.gradle.org/current/javadoc/org/gradle/api/logging/package-summary.html
Use the parameterized Logger.info overload.
When the repository is missing, pass MAVEN_LOCAL_REPOSITORY as a logger parameter instead of interpolating it before the call. The helper runs during beforeSettings and beforeProject.
Proposed fix
- logger.info("Local maven repo not installed yet, skipping buildscript injection: $MAVEN_LOCAL_REPOSITORY")
+ logger.info(
+ "Local maven repo not installed yet, skipping buildscript injection: {}",
+ MAVEN_LOCAL_REPOSITORY,
+ )📝 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.
| logger.info("Local maven repo not installed yet, skipping buildscript injection: $MAVEN_LOCAL_REPOSITORY") | |
| logger.info( | |
| "Local maven repo not installed yet, skipping buildscript injection: {}", | |
| MAVEN_LOCAL_REPOSITORY, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt`
at line 97, Update the missing-repository log in the
beforeSettings/beforeProject helper to use the parameterized Logger.info
overload: keep the repository value as a placeholder argument and pass
MAVEN_LOCAL_REPOSITORY separately instead of interpolating it into the message.
Sources: Coding guidelines, MCP tools
3 participants
COTGSettingsPlugin injects the bundled repo into pluginManagement and
dependencyResolutionManagement only. A
buildscript { }block resolvesagainst its own repositories, which nothing reached, so any project
declaring its build classpath that way could only be built online. That
is why plugin-template builds required network.
Inject the repo for both scopes, each before the script that consumes it
is evaluated:
(the plugin template declares AGP and Kotlin there)
settingsEvaluated is too late: by then the settings buildscript classpath
has already resolved.
A missing repo is not fatal. The directory does not exist until onboarding
has installed the assets, and failing there would break every build before
that point.
Stack created with GitHub Stacks CLI • Give Feedback 💬