kitty/spotube
1
0
Fork 0
mirror of https://github.com/KRTirtho/spotube.git synced 2026-11-04 10:13:59 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix(desktop): stop non-composable string lookups from querying the system theme

Browse Source 
Compose Resources' getString/getPluralString/getStringArray call
getSystemResourceEnvironment() on every lookup, which on desktop asks
Skiko for the system theme. On Linux that is a blocking D-Bus call to the
desktop portal on the calling thread, and from a background coroutine it
segfaulted the GraalVM AppImage on Fedora GNOME when installing a plugin
from the Welcome screen.

Strings never depend on the theme, so app code now uses wrappers in
core/i18n that reuse the resource environment the UI already computed
(published by AppLocaleProvider), falling back to the library lookup
before anything is composed. A test guards against importing the library
getters again.

Fixes #3183
This commit is contained in:
Kingkor Roy Tirtho 2026-11-02 10:17:35 +06:00
parent 0541fd9d23
commit 1b089a5fec
26 changed files with 143 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
AGENTS.md
View File

@ -54,6 +54,7 @@
## UI component patterns
- **ALL user-facing strings MUST come from `composeApp/src/commonMain/composeResources/values/strings.xml` via `stringResource(Res.string.xxx)`.** Never hardcode a literal string in a `Text(...)`, dialog title, button label, content description, placeholder, or error message inside `@Composable` code. If the string you need doesn't exist yet, add a new `<string name="...">` entry to `strings.xml` (grouped near related keys, following the existing `feature_context_purpose` naming convention) and reference it — do not inline the text "just this once". This applies to every module, including ones you're about to add (dialogs, screens, snackbars, etc.), not just `modules/settings`. `strings.xml` is the single source of truth for copy, not a showcase file for one screen.
- Outside composables (ViewModels, services, snackbars), import `getString` / `getPluralString` / `getStringArray` from `org.spotube.spotube.core.i18n`, never from `org.jetbrains.compose.resources`. The library versions ask Skiko for the system theme on every call, which on Linux is a blocking D-Bus call that segfaulted the desktop app from background threads (#3183). `NonComposableStringLookupTest` enforces this.
- **ViewModels own ALL state and logic.** Composables are dumb renderers: they collect a single `StateFlow<UiState>` from the ViewModel and forward user events (clicks, text input, drag callbacks) back to ViewModel functions. No business logic, filtering, derivation, reordering buffers, or `LaunchedEffect`-based state syncing belongs in a composable — it goes in the ViewModel. The `combine`/`stateIn` flow chain in the ViewModel must produce fully-computed, ready-to-render UI state so the composable never needs intermediate `remember` derivations or `mutableStateListOf` mirrors.
- **AdaptiveDropdownBottomSheet** (`commonMain/.../core/ui/component/AdaptiveDropdownBottomSheet.kt`): switches between `DropdownMenu` (large screen) and `ModalBottomSheet` (small screen) via `currentWindowAdaptiveInfo()`. Do NOT use expect/actual — all adaptive components that rely ONLY on Compose/Material3 APIs belong in commonMain.
- **AdaptiveDialogBottomSheet** (`commonMain/.../core/ui/component/AdaptiveDialogBottomSheet.kt`): switches between `ThemedDialog` (large screen) and `ModalBottomSheet` (small screen) via `currentWindowAdaptiveInfo()`. Same rule — keep in commonMain unless platform-specific APIs are required.

2
composeApp/src/androidMain/kotlin/org/spotube/spotube/media/MediaBrowseHelper.kt
View File

@ -41,7 +41,7 @@ import org.spotube.spotube.modules.saved_tracks.SavedTracksRepository
import org.spotube.spotube.modules.search.SearchRepository
import org.spotube.spotube.modules.settings.SettingsRepository
import kotlinx.coroutines.flow.first
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.library_tab_albums
import spotube.composeapp.generated.resources.library_tab_artists

2
composeApp/src/androidMain/kotlin/org/spotube/spotube/media/PlaybackService.kt
View File

@ -55,7 +55,7 @@ import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.notification_playback_channel_description
import spotube.composeapp.generated.resources.notification_playback_channel_name

1
composeApp/src/commonMain/kotlin/org/spotube/spotube/core/i18n/AppLocale.kt
View File

@ -54,6 +54,7 @@ fun AppLocaleProvider(language: SupportedLanguages, content: @Composable () -> U
layoutDirection?.let { LocalLayoutDirection provides it })
CompositionLocalProvider(*locales.toTypedArray()) {
key(languageTag) {
PublishResourceEnvironment()
content()
}
}

71
composeApp/src/commonMain/kotlin/org/spotube/spotube/core/i18n/Strings.kt Normal file
View File

@ -0,0 +1,71 @@
/*
* Copyright (C) 2026 Kingkor Roy Tirtho and Spotube Contributors
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
package org.spotube.spotube.core.i18n
import androidx.compose.runtime.Composable
import androidx.compose.runtime.SideEffect
import org.jetbrains.compose.resources.PluralStringResource
import org.jetbrains.compose.resources.ResourceEnvironment
import org.jetbrains.compose.resources.StringArrayResource
import org.jetbrains.compose.resources.StringResource
import org.jetbrains.compose.resources.getSystemResourceEnvironment
import org.jetbrains.compose.resources.rememberResourceEnvironment
import kotlin.concurrent.Volatile
import org.jetbrains.compose.resources.getPluralString as composeGetPluralString
import org.jetbrains.compose.resources.getString as composeGetString
import org.jetbrains.compose.resources.getStringArray as composeGetStringArray
/*
* Non-composable string lookups (ViewModels, services, snackbars). Use these instead of
* org.jetbrains.compose.resources.getString/getPluralString/getStringArray.
*
* The library versions call getSystemResourceEnvironment() on every lookup. On desktop that
* also asks Skiko for the system theme, and on Linux Skiko answers with a blocking D-Bus call to
* the desktop portal on whatever thread is asking. Run from a background coroutine, that call
* segfaulted the GraalVM build on GNOME (#3183). Strings never depend on the theme, so these
* reuse the environment the UI already computed (on the UI thread, for stringResource) and only
* fall back to the library's lookup before anything has been composed.
*/
@Volatile
private var composedEnvironment: ResourceEnvironment? = null
/** Publishes the UI's resource environment for [getString] and friends. Called by [AppLocaleProvider]. */
@Composable
internal fun PublishResourceEnvironment() {
val environment = rememberResourceEnvironment()
SideEffect { composedEnvironment = environment }
}
private fun resourceEnvironment(): ResourceEnvironment =
composedEnvironment ?: getSystemResourceEnvironment()
suspend fun getString(resource: StringResource): String =
composeGetString(resourceEnvironment(), resource)
suspend fun getString(resource: StringResource, vararg formatArgs: Any): String =
composeGetString(resourceEnvironment(), resource, *formatArgs)
suspend fun getPluralString(resource: PluralStringResource, quantity: Int): String =
composeGetPluralString(resourceEnvironment(), resource, quantity)
suspend fun getPluralString(resource: PluralStringResource, quantity: Int, vararg formatArgs: Any): String =
composeGetPluralString(resourceEnvironment(), resource, quantity, *formatArgs)
suspend fun getStringArray(resource: StringArrayResource): List<String> =
composeGetStringArray(resourceEnvironment(), resource)

4
composeApp/src/commonMain/kotlin/org/spotube/spotube/core/jam/JamNames.kt
View File

@ -18,8 +18,8 @@
package org.spotube.spotube.core.jam
import okio.ByteString.Companion.encodeUtf8
import org.jetbrains.compose.resources.getString
import org.jetbrains.compose.resources.getStringArray
import org.spotube.spotube.core.i18n.getString
import org.spotube.spotube.core.i18n.getStringArray
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.jam_name_adjectives
import spotube.composeapp.generated.resources.jam_name_animals

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/core/jam/JamRoomClient.kt
View File

@ -58,7 +58,7 @@ import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonClassDiscriminator
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import org.spotube.spotube.core.network.HttpClientFactory
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.jam_error_banned

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/core/jam/JamRoomService.kt
View File

@ -59,7 +59,7 @@ import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withTimeoutOrNull
import kotlin.math.abs
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.jam_error_invalid_code
import spotube.composeapp.generated.resources.jam_error_no_server

4
composeApp/src/commonMain/kotlin/org/spotube/spotube/core/remote/RemotePlaybackController.kt
View File

@ -40,8 +40,8 @@ import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getPluralString
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getPluralString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.jam_queue_added_track
import spotube.composeapp.generated.resources.jam_queue_added_tracks

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/album/AlbumViewModel.kt
View File

@ -51,7 +51,7 @@ import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.common_unknown_error

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/artist/ArtistViewModel.kt
View File

@ -48,7 +48,7 @@ import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.*
sealed interface ArtistScreenState {

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/downloads/DownloadManager.kt
View File

@ -62,7 +62,7 @@ import okio.SYSTEM
import okio.buffer
import okio.use
import kotlin.time.Clock
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.*
data class DownloadItem(

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/home/HomeScreenViewModel.kt
View File

@ -35,7 +35,7 @@ import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.common_unknown_error

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/jam/JamViewModel.kt
View File

@ -37,7 +37,7 @@ import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import kotlinx.coroutines.delay
import kotlinx.coroutines.Job
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.*
enum class JamSetupMode { Create, Join }

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/library/album/LibraryAlbumsViewModel.kt
View File

@ -36,7 +36,7 @@ import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.common_unknown_error

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/library/artist/LibraryArtistsViewModel.kt
View File

@ -36,7 +36,7 @@ import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.common_unknown_error

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/library/local_tracks/LibraryLocalTracksViewModel.kt
View File

@ -34,7 +34,7 @@ import kotlinx.coroutines.launch
import kotlin.coroutines.cancellation.CancellationException
import org.spotube.spotube.PlatformType
import org.spotube.spotube.getPlatform
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.library_local_refresh_failed

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/library/playlist/LibraryPlaylistsViewModel.kt
View File

@ -37,7 +37,7 @@ import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import io.github.vinceglb.filekit.PlatformFile
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.common_unknown_error

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/playlist/PlaylistViewModel.kt
View File

@ -56,7 +56,7 @@ import org.spotube.spotube.modules.library.playlist.PlaylistFormUiState
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.common_unknown_error

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/plugin/PluginManager.kt
View File

@ -61,7 +61,7 @@ import okio.Path.Companion.toPath
import okio.SYSTEM
import okio.buffer
import okio.use
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import org.koin.core.component.KoinComponent
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.plugin_action_install

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/saved_tracks/SavedTracksViewModel.kt
View File

@ -59,7 +59,7 @@ import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import org.koin.compose.koinInject
import org.koin.core.component.KoinComponent
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.*
const val SAVED_TRACKS_COLLECTION_ID = "saved_tracks"

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/search/SearchRepository.kt
View File

@ -34,7 +34,7 @@ import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.launch
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.search_error_no_metadata_plugin

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/search/SearchScreenViewModel.kt
View File

@ -44,7 +44,7 @@ import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.joinAll
import kotlinx.coroutines.launch
import kotlinx.serialization.json.Json
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.search_error_capabilities
import spotube.composeapp.generated.resources.search_error_search

2
composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/shell/AppExpandedPlayer.kt
View File

@ -123,7 +123,7 @@ import org.koin.compose.koinInject
import org.koin.compose.viewmodel.koinViewModel
import org.koin.core.parameter.parametersOf
import kotlin.time.Duration.Companion.milliseconds
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import org.jetbrains.compose.resources.stringResource
import spotube.composeapp.generated.resources.Res
import spotube.composeapp.generated.resources.downloads_status_cancelled

2
composeApp/src/jvmTest/kotlin/org/spotube/spotube/core/i18n/AppLocaleProviderTest.kt
View File

@ -28,7 +28,7 @@ import androidx.compose.ui.unit.LayoutDirection
import androidx.compose.ui.use
import org.spotube.spotube.modules.settings.SupportedLanguages
import kotlinx.coroutines.runBlocking
import org.jetbrains.compose.resources.getString
import org.spotube.spotube.core.i18n.getString
import org.jetbrains.compose.resources.pluralStringResource
import org.jetbrains.compose.resources.stringResource
import spotube.composeapp.generated.resources.Res

46
composeApp/src/jvmTest/kotlin/org/spotube/spotube/core/i18n/NonComposableStringLookupTest.kt Normal file
View File

@ -0,0 +1,46 @@
/*
* Copyright (C) 2026 Kingkor Roy Tirtho and Spotube Contributors
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
package org.spotube.spotube.core.i18n
import java.io.File
import kotlin.test.Test
import kotlin.test.assertTrue
/**
* The library's non-composable getters query the system theme over D-Bus on Linux on every
* call, which crashed the desktop app (#3183). App code must use the ones in Strings.kt.
*/
class NonComposableStringLookupTest {
@Test
fun appCodeUsesTheAppStringLookups() {
val forbidden = Regex("""^import org\.jetbrains\.compose\.resources\.get(String|PluralString|StringArray)\s*$""")
val offenders = File("src").walkTopDown()
.filter { it.isFile && it.extension == "kt" }
.flatMap { file ->
file.readLines().withIndex()
.filter { (_, line) -> forbidden.matches(line) }
.map { (index, _) -> "${file.path}:${index + 1}" }
}
.toList()
assertTrue(
offenders.isEmpty(),
"Import getString/getPluralString/getStringArray from org.spotube.spotube.core.i18n instead:\n" +
offenders.joinToString("\n"))
}
}