-
-
Notifications
You must be signed in to change notification settings - Fork 58
ADFA-4128 (10/11): gradle-plugin — generating the proxy app #1722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3760bad
ceb17c5
b235bdf
2656ec9
3231a6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,11 +18,14 @@ | |||||
| package com.itsaky.androidide.gradle | ||||||
|
|
||||||
| import com.itsaky.androidide.buildinfo.BuildInfo | ||||||
| import org.adfa.constants.ANDROIDIDE_HOME | ||||||
| import org.adfa.constants.COGO_GRADLE_PLUGIN_JAR_NAME | ||||||
| import org.adfa.constants.COGO_GRADLE_PLUGIN_PATH | ||||||
| import org.gradle.api.GradleException | ||||||
| import org.gradle.api.Plugin | ||||||
| import org.gradle.api.invocation.Gradle | ||||||
| import org.gradle.api.logging.Logging | ||||||
| import java.io.File | ||||||
| import java.net.URLClassLoader | ||||||
|
|
||||||
| const val MAX_LOGFILE_COUNT = 2 | ||||||
|
|
||||||
|
|
@@ -34,6 +37,32 @@ const val MAX_LOGFILE_COUNT = 2 | |||||
| class AndroidIDEInitScriptPlugin : Plugin<Gradle> { | ||||||
| companion object { | ||||||
| private val logger = Logging.getLogger(AndroidIDEInitScriptPlugin::class.java) | ||||||
|
|
||||||
| /** | ||||||
| * Picks what to put on the root buildscript classpath so subprojects can resolve | ||||||
| * [BuildInfo.PACKAGE_NAME] by plugin ID: an init script's own classpath does NOT reach | ||||||
| * project plugin resolution, so this injection is the sole mechanism that makes | ||||||
| * `pluginManager.apply(id)` work below. Prefers the jar the IDE ships, else whatever the | ||||||
| * init script was loaded from; empty fails loud, since a missing path is a silent no-op. | ||||||
| */ | ||||||
| internal fun resolvePluginClasspath( | ||||||
| bundledJar: File, | ||||||
| initScriptClasspath: List<File>, | ||||||
| ): List<File> { | ||||||
| if (bundledJar.isFile) { | ||||||
| return listOf(bundledJar) | ||||||
| } | ||||||
|
|
||||||
| val fallback = initScriptClasspath.filter(File::exists) | ||||||
| if (fallback.isNotEmpty()) { | ||||||
| return fallback | ||||||
| } | ||||||
|
|
||||||
| throw GradleException( | ||||||
| "Cannot inject the '${BuildInfo.PACKAGE_NAME}' plugin: no plugin jar at " + | ||||||
| "'${bundledJar.absolutePath}' and the init script classpath is empty.", | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| override fun apply(target: Gradle) { | ||||||
|
|
@@ -44,14 +73,13 @@ class AndroidIDEInitScriptPlugin : Plugin<Gradle> { | |||||
| } | ||||||
|
|
||||||
| target.rootProject { rootProject -> | ||||||
| rootProject.buildscript.apply { | ||||||
| dependencies.apply { | ||||||
| add( | ||||||
| "classpath", | ||||||
| rootProject.files("$ANDROIDIDE_HOME/plugin/cogo-plugin.jar"), | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
| val classpath = | ||||||
| resolvePluginClasspath( | ||||||
| File(COGO_GRADLE_PLUGIN_PATH, COGO_GRADLE_PLUGIN_JAR_NAME), | ||||||
| initScriptClasspath(), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MINOR: this file changes behaviour for every ordinary build, which the PR description says cannot happen. The body says "Ordinary Gradle builds are untouched by it" and the diagram's "no" branch says "this PR's code does not run"; the review guide says to start at No regression: I checked that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right; this is the PR description, corrected there rather than in the branch. |
||||||
| ) | ||||||
| logger.info("Injecting plugin classpath into the root buildscript: $classpath") | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win Do not log the resolved classpath. Line 81 logs absolute paths. These paths can contain account names. Build logs can expose that data outside the device. Log a count or a non-sensitive source label instead. As per coding guidelines, "No secrets/PII." 🤖 Prompt for AI AgentsSource: Coding guidelines
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not taking it. These are Android app-private paths on an on-device IDE, with no per-user home directory names in them, so there is no account name to leak. It is logger.info, suppressed unless the user passes --info, and the GradleException twenty lines above already prints bundledJar.absolutePath unconditionally.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NITPICK: this logger call interpolates the classpath into the message instead of using the
Suggested change
|
||||||
| rootProject.buildscript.dependencies.add("classpath", rootProject.files(classpath)) | ||||||
| } | ||||||
|
|
||||||
| target.projectsLoaded { gradle -> | ||||||
|
|
@@ -71,18 +99,20 @@ class AndroidIDEInitScriptPlugin : Plugin<Gradle> { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** The files this plugin itself was loaded from, i.e. the init script's classpath. */ | ||||||
| private fun initScriptClasspath(): List<File> { | ||||||
| val loader = javaClass.classLoader as? URLClassLoader ?: return emptyList() | ||||||
| return loader.urLs.mapNotNull { url -> runCatching { File(url.toURI()) }.getOrNull() } | ||||||
| } | ||||||
|
|
||||||
| private fun removeDaemonLogs(gradle: Gradle) { | ||||||
| // Get the Gradle user home directory | ||||||
| val gradleUserHomeDir = gradle.gradleUserHomeDir | ||||||
|
|
||||||
| // Get the current Gradle version | ||||||
| val currentGradleVersion = gradle.gradleVersion | ||||||
| val logsDir = File(gradleUserHomeDir, "daemon/$currentGradleVersion") | ||||||
|
|
||||||
| if (logsDir.exists() && logsDir.isDirectory) { | ||||||
| logger.lifecycle("Code On the Go clean logs of gradle ($currentGradleVersion) task running....") | ||||||
|
|
||||||
| // Filter and iterate over log files, sorted by last modified date | ||||||
| logsDir | ||||||
| .listFiles() | ||||||
| ?.filter { it.isFile && it.name.endsWith(".log") } | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MINOR: the
testtask writesrepos.txtindoFirstbut declares neither it nor the staged maven-local repos as inputs or outputs, sotestcan report UP-TO-DATE after those repos change and the functional suite silently does not run.Edit
:logsender, then run:gradle-plugin:test.publishAllPublicationsToBuildMavenLocalRepositoryre-runs, but nothing intest's input snapshot moved - the staged repos are not on its classpath - so Gradle skips it. The TestKit builds that would have resolved the new artifact throughrepos.txtnever execute, and a regression they exist to catch reports green.Declare them, e.g.
inputs.files(stagedRepos)andoutputs.file(reposFile).