Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ abstract class BaseBuildAction : EditorActivityAction() {

/**
* Refuses a tap while an internal build (Quick Build's proxy app build) holds the one Gradle
* slot, and flashes why. [prepare] leaves these actions enabled in that window, and starting a
* second build would throw BuildInProgressException deep in the service and surface as a raw
* error. Reads the raw [BuildService.isBuildInProgress], because this guards the slot rather
* slot, and flashes why. [prepare] leaves these actions enabled in that window, and the service
* would refuse a second build silently, so the tap would seem to do nothing. Reads the raw

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR: This now says the service refuses a second build silently, but a build that loses the slot claim shows the user "Task execution failed: BUILD_IN_PROGRESS".

GradleBuildService now answers a lost claim with TaskExecutionResult(false, BUILD_IN_PROGRESS) (GradleBuildService.kt:841). BuildViewModel.kt:137, outside this diff, turns any non-cancelled failure into RuntimeException("Task execution failed: ${result?.failure}"). On stage the same race threw BuildInProgressException("A build is already running!"), which read fine. It only happens when another build claims the slot between this action's isBuildInProgress check and executeTasks, so it is rare.

Fix: map BUILD_IN_PROGRESS in BuildViewModel to the slot-busy message, as BUILD_CANCELLED is at line 132, and correct this comment.

* [BuildService.isBuildInProgress], because this guards the slot rather
* than presenting state.
*
* @return true when the tap was refused and the caller must start nothing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.itsaky.androidide.plugins.PluginLogger
import com.itsaky.androidide.plugins.base.PluginFragmentHelper
import com.itsaky.androidide.plugins.manager.core.PluginManager
import com.itsaky.androidide.plugins.manager.services.IdeLogServiceImpl
import com.itsaky.androidide.plugins.manager.services.IdeTerminalServiceImpl
import com.itsaky.androidide.preferences.internal.DevOpsPreferences
import com.itsaky.androidide.preferences.internal.GeneralPreferences
import com.itsaky.androidide.resources.localization.LocaleProvider
Expand Down Expand Up @@ -393,6 +394,7 @@ internal object CredentialProtectedApplicationLoader : ApplicationLoader {
setupBuildServiceProviders()
setupProjectManipulationProviders()
IdeLogServiceImpl.getInstance().setLogReader(LogsProvider::read)
IdeTerminalServiceImpl.setSessionLauncher(PluginTerminalLauncher { application.foregroundActivity })
logger.info("Plugin services configured successfully")
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package com.itsaky.androidide.app

import android.app.Activity
import android.content.Intent
import android.os.Handler
import android.os.Looper
import com.itsaky.androidide.activities.TerminalActivity
import com.itsaky.androidide.plugins.manager.services.TerminalSessionLauncher
import com.itsaky.androidide.plugins.services.TerminalCommandResult
import com.itsaky.androidide.terminal.TerminalCommandRequests
import com.itsaky.androidide.utils.applyMultiWindowFlags
import org.slf4j.LoggerFactory
import java.io.File

/**
* Opens a plugin's command in a new Terminal session, through the same activity the Terminal
* sidebar action opens.
*/
internal class PluginTerminalLauncher(
private val foregroundActivity: () -> Activity?,
) : TerminalSessionLauncher {
private val mainHandler = Handler(Looper.getMainLooper())

override fun launch(
command: String,
workingDirectory: File?,
sessionName: String,
onResult: (TerminalCommandResult) -> Unit,
): () -> Unit {
// Android blocks activity starts from the background, so a plugin can only open the
// Terminal while the IDE is on screen.
val activity = foregroundActivity()
if (activity == null) {
onResult(TerminalCommandResult.NotStarted("Code On the Go is not in the foreground"))
return {}
}

val requestId =
TerminalCommandRequests.enqueue(
command = command,
workingDirectory = workingDirectory?.absolutePath,
sessionName = sessionName,
onExit = { exitCode, transcript -> onResult(TerminalCommandResult.Completed(exitCode, transcript)) },
onNotStarted = { reason -> onResult(TerminalCommandResult.NotStarted(reason)) },
)
val intent =
Intent(activity, TerminalActivity::class.java)
.putExtra(TerminalCommandRequests.EXTRA_COMMAND_REQUEST_ID, requestId)
.applyMultiWindowFlags(activity)
try {
activity.startActivity(intent)
} catch (e: Exception) {
logger.error("Failed to open the Terminal for a plugin command", e)
if (TerminalCommandRequests.withdraw(requestId)) {
onResult(TerminalCommandResult.NotStarted("The Terminal could not be opened: ${e.message}"))
}
return {}
}

// The activity can fail to start or finish before its service connects; without this the
// plugin would wait forever for a session that never comes.
mainHandler.postDelayed({
if (TerminalCommandRequests.withdraw(requestId)) {
onResult(TerminalCommandResult.NotStarted("The Terminal did not open"))
}
}, OPEN_TIMEOUT_MS)

return { TerminalCommandRequests.cancel(requestId) }
}

private companion object {
private val logger = LoggerFactory.getLogger(PluginTerminalLauncher::class.java)

const val OPEN_TIMEOUT_MS = 15_000L
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import com.itsaky.androidide.tooling.api.messages.result.BuildResult
import com.itsaky.androidide.tooling.api.messages.result.GradleWrapperCheckResult
import com.itsaky.androidide.tooling.api.messages.result.InitializeResult
import com.itsaky.androidide.tooling.api.messages.result.TaskExecutionResult
import com.itsaky.androidide.tooling.api.messages.result.TaskExecutionResult.Failure.BUILD_IN_PROGRESS
import com.itsaky.androidide.tooling.api.models.ToolingServerMetadata
import com.itsaky.androidide.tooling.events.ProgressEvent
import com.itsaky.androidide.utils.Environment
Expand Down Expand Up @@ -94,6 +95,7 @@ import java.util.UUID
import java.util.concurrent.CompletableFuture
import java.util.concurrent.CompletionException
import java.util.concurrent.TimeoutException
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicLong
import kotlin.coroutines.cancellation.CancellationException

Expand All @@ -111,11 +113,11 @@ class GradleBuildService :
private var mBinder: GradleServiceBinder? = null
private var isToolingServerStarted = false

// Volatile: written on the Tooling API's CompletableFuture pool, read cross-thread
// by Quick Build's slot pre-check.
@Volatile
override var isBuildInProgress = false
private set
// Atomic: two callers may race for the slot, and Quick Build's pre-check reads it cross-thread.
private val buildSlot = AtomicBoolean(false)

override val isBuildInProgress: Boolean
get() = buildSlot.get()

/**
* Gradle output captured while the editor's listener is suppressed, oldest line first.
Expand Down Expand Up @@ -237,8 +239,7 @@ class GradleBuildService :
/**
* The RPC future of the build holding the slot, failed by [onServerExited]: the RPC layer never
* completes a request whose server process died, and only that completion clears
* [isBuildInProgress]. Never nulled - completing a finished future is a no-op, and a clear in
* [markBuildAsFinished] would also run for a request rejected while another build still ran.
* [isBuildInProgress]. Never nulled - completing a finished future is a no-op.
*/
@Volatile
private var pendingBuild: CompletableFuture<*>? = null
Expand Down Expand Up @@ -817,7 +818,7 @@ class GradleBuildService :
checkServerStarted()
Objects.requireNonNull(params)
return try {
performBuildTasks(server!!.initialize(params))
performBuildTasks(refused = InitializeResult.Failure(BUILD_IN_PROGRESS)) { server!!.initialize(params) }
} catch (_: ScanPluginMissingException) {
log.info("Retrying initialization without --scan option...")
initializeProject(params)
Expand All @@ -836,7 +837,10 @@ class GradleBuildService :
override fun executeTasks(message: TaskExecutionMessage): CompletableFuture<TaskExecutionResult> {
checkServerStarted()

val future = performBuildTasks(server!!.executeTasks(message))
val future =
performBuildTasks(refused = TaskExecutionResult(false, BUILD_IN_PROGRESS)) {
server!!.executeTasks(message)
}

return future.handle { result, exception ->
if (exception != null) {
Expand All @@ -856,9 +860,29 @@ class GradleBuildService :
return server!!.cancelCurrentBuild()
}

private fun <T> performBuildTasks(future: CompletableFuture<T>): CompletableFuture<T> {
private fun <T> performBuildTasks(
refused: T,
dispatch: () -> CompletableFuture<T>,
): CompletableFuture<T> {
// Claimed before the request is sent and released only by the build that claimed it, so a
// refused request can neither reach the tooling server nor free another build's slot.
if (!buildSlot.compareAndSet(false, true)) {
logBuildInProgress()
// Not null: markBuildAsFinished turns a failed build into null, and callers must tell a
// build that never started from one that failed.
return CompletableFuture.completedFuture(refused)
}
val future =
try {
dispatch()
} catch (e: Throwable) {
buildSlot.set(false)
throw e
}
pendingBuild = future

return CompletableFuture
.runAsync { onPrepareBuildRequest(future) }
.runAsync { ensureTmpdir() }
.handleAsync { _, _ ->
try {
return@handleAsync future.get()
Expand Down Expand Up @@ -917,17 +941,6 @@ class GradleBuildService :
return false
}

private fun onPrepareBuildRequest(future: CompletableFuture<*>) {
checkServerStarted()
ensureTmpdir()
if (isBuildInProgress) {
logBuildInProgress()
throw BuildInProgressException()
}
isBuildInProgress = true
pendingBuild = future
}

@Throws(ToolingServerNotStartedException::class)
private fun checkServerStarted() {
if (!isToolingServerStarted()) {
Expand All @@ -948,7 +961,7 @@ class GradleBuildService :
result: T,
throwable: Throwable?,
): T {
isBuildInProgress = false
buildSlot.set(false)
return result
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ class GradleBuildServiceServerExitTest {
service.onListenerStarted(server, ByteArrayInputStream(ByteArray(0)))

val build = service.executeTasks(listOf(":app:assembleDebug"))
// The slot is taken on the build's own future chain, not on the caller's thread.
awaitUntil { service.isBuildInProgress }
// The slot is taken on the caller's thread, before the request is sent.
assertThat(service.isBuildInProgress).isTrue()

service.onServerExited(137)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package com.itsaky.androidide.services.builder

import com.google.common.truth.Truth.assertThat
import com.itsaky.androidide.tooling.api.IToolingApiServer
import com.itsaky.androidide.tooling.api.messages.result.TaskExecutionResult
import com.itsaky.androidide.utils.Environment
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
import org.junit.runner.RunWith
import org.robolectric.Robolectric
import org.robolectric.RobolectricTestRunner
import java.io.ByteArrayInputStream
import java.util.concurrent.CompletableFuture
import java.util.concurrent.TimeUnit

/**
* The Gradle build slot when a second build is requested while one runs.
*
* The defects this pins: the slot was checked and taken on a pool thread after the request had
* already been sent, so two callers could both reach the tooling server; and a refused request
* still cleared the slot when it completed, while the first build was running.
*/
@RunWith(RobolectricTestRunner::class)
class GradleBuildServiceSlotTest {
@get:Rule
val tmp = TemporaryFolder()

private lateinit var service: GradleBuildService
private val rpc = CompletableFuture<TaskExecutionResult>()

// A second request that reaches the server is refused there, as ToolingApiServerImpl does.
private val server =
mockk<IToolingApiServer> {
every { executeTasks(any()) } returnsMany
listOf(rpc, CompletableFuture.failedFuture(IllegalStateException("Build is already in progress")))
}

@Before
fun setUp() {
Environment.TMP_DIR = tmp.newFolder("tmp")
service = Robolectric.buildService(GradleBuildService::class.java).get()
service.onListenerStarted(server, ByteArrayInputStream(ByteArray(0)))
}

@Test
fun `a second build is refused without reaching the tooling server`() {
service.executeTasks(listOf(":app:assembleDebug"))
val second = service.executeTasks(listOf(":app:test"))

// Distinct from a failed build, which completes as null.
assertThat(second.get(5, TimeUnit.SECONDS))
.isEqualTo(TaskExecutionResult(false, TaskExecutionResult.Failure.BUILD_IN_PROGRESS))
verify(exactly = 1) { server.executeTasks(any()) }
}

@Test
fun `a refused build leaves the running build's slot taken`() {
val first = service.executeTasks(listOf(":app:assembleDebug"))
service.executeTasks(listOf(":app:test")).get(5, TimeUnit.SECONDS)

assertThat(service.isBuildInProgress).isTrue()

rpc.complete(TaskExecutionResult(isSuccessful = true, failure = null))
first.get(5, TimeUnit.SECONDS)
assertThat(service.isBuildInProgress).isFalse()
}

@Test
fun `a request that fails to send frees the slot`() {
every { server.executeTasks(any()) } throws IllegalStateException("closed")

runCatching { service.executeTasks(listOf(":app:assembleDebug")) }

assertThat(service.isBuildInProgress).isFalse()
}
}
29 changes: 29 additions & 0 deletions docs/PLUGIN_API_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,35 @@ milestone. **[verified]** = read from the checked-in ABI dump. **[reconstructed]
= diffed from `plugin-api/src` history (predates the dump; symbol-accurate).

### 26.41 — unreleased
- **added — Terminal readiness and visible terminal commands** _(ADFA-6373)_ **[verified]**
`IdeTerminalService.isTerminalReady()` reports whether the terminal environment is installed
and bash runs; it needs no permission. `runInTerminal(command, workingDirectory)` opens a new
session in the visible Terminal, runs the command with bash and suspends until it exits,
returning `TerminalCommandResult.Completed(exitCode, output)` with the session transcript, or
`NotStarted(reason)` (environment missing, IDE not in the foreground). The session stays open
so the user sees what ran; cancelling the caller kills the command. Needs `system.commands`;
the working directory must lie inside the project. Floor `plugin.min_ide_version` at `26.41`:
an older IDE has no `IdeTerminalService` class, so referencing it fails to load.
- **added — Run Gradle tasks with arguments, get a structured result, cancel** _(ADFA-6373)_ **[verified]**
`IdeBuildService.executeTasks(tasks: List<String>, arguments: List<String>)` runs on the
IDE's tooling server, so `--tests`, `-P` and `--info` work and the output reaches the Build
Output pane (read it with `getBuildOutput()`). It completes with a `GradleTaskResult`:
`Success`, `Failed(reason)`, `Refused(reason)` when the build never started (another build
running, tooling server down), or `Cancelled`. `IdeBuildService.cancelBuild()` cancels the
running build, whoever started it. `executeTasks(vararg String)` is unchanged. Floor

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

F07 (Medium): executeTasks(vararg String) did change. executeTasks() with no tasks used to run Gradle's default tasks; now it returns false straight away ("no tasks were given"). A task string starting with - (--offline, -I x) used to be rejected as a task name; now it is applied as an option. Please document both changes, or keep the old behaviour.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MINOR: Still open at 822d366e3. The changelog still says executeTasks(vararg String) is unchanged. On stage an empty call ran Gradle's default tasks through forTasks(); now it returns false ("no tasks were given", IdeBuildServiceImpl.kt:127). A string starting with - used to be a failing task name and is now applied as an option, because tasks go into addArguments (GradleBuildExts.kt). A plugin author reading the changelog will expect neither.

Fix: document both changes under 26.41, or keep the vararg overload on forTasks.

`plugin.min_ide_version` at `26.41`: an older IDE has neither method.
- **breaking — `CommandSpec.GradleTask` runs on the tooling server** _(ADFA-6373)_
It used to start `./gradlew` as a separate process: a second Gradle daemon on the device,
with output that never reached the Build Output pane. It now runs like `executeTasks` above.
Output arrives in one batch of `StdOut` lines when the build ends instead of streaming; the
exit code is 0 on success and 1 on a failed build; a refused build fails with exit code -1
and the reason in `CommandResult.Failure.error`. No source change is needed. A plugin that
ran a Gradle task while another build was running now gets that refusal instead of a
second build.
- **breaking — `CommandSpec.ShellCommand.workingDirectory` needs an open project** _(ADFA-6373)_
With no project open, a `workingDirectory` used to be accepted unchecked; `executeCommand` now
throws `SecurityException`, as it does for one outside the project. Pass null to run in the
default directory.
- **added — Plugin languages: tree-sitter highlighting and a language server** _(ADFA-4851)_ **[verified]**
A plugin implementing `LanguageExtension` returns `LanguageDefinition`s, each claiming file
extensions and optionally carrying a `TreeSitterGrammar` and a `LanguageServerDefinition`.
Expand Down
Loading
Loading