fix: prevent double escaping of cli arguments

- other minor improvements and code cleanup Signed-off-by: Artyom Shendrik <[email protected]>
fluxo-kt · Feb 22, 2024 · e264fbb · e264fbb
1 parent 1550237
commit e264fbb
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,9 @@
 - return the plugin artifact shrinking with R8 (saved 32.719%, 265.9 KB).
 - update compatibility methods `NamedDomainObjectSet<T>.named*` for Gradle 8.6+ and older.
 
+### Fixed
+- prevent double escaping of cli arguments.
+
 
 ## [0.7.0-alpha2] - 2024-02-22
 

diff --git a/checks/gradle-plugin/pg/keep-api-autogenerated.pro b/checks/gradle-plugin/pg/keep-api-autogenerated.pro
@@ -4,7 +4,7 @@
 # - api\check-gradle-plugin.api
 # DO NOT EDIT MANUALLY!
 
--keep,allowoptimization public final class NewPlugin {
+-keep,includedescriptorclasses public final class NewPlugin {
     public <init>();
     public synthetic void apply(java.lang.Object);
     public void apply(org.gradle.api.Project);

diff --git a/fluxo-kmp-conf/api/plugin.api b/fluxo-kmp-conf/api/plugin.api
@@ -82,8 +82,6 @@ public final class PropsAndEnvKt {
 	public static final fun isMaxDebugEnabled (Lorg/gradle/api/Project;)Lorg/gradle/api/provider/Provider;
 	public static final fun isR8Disabled (Lorg/gradle/api/Project;)Lorg/gradle/api/provider/Provider;
 	public static final fun isRelease (Lorg/gradle/api/Project;)Lorg/gradle/api/provider/Provider;
-	public static final fun scmTag (Lorg/gradle/api/Project;Z)Lorg/gradle/api/provider/Provider;
-	public static synthetic fun scmTag$default (Lorg/gradle/api/Project;ZILjava/lang/Object;)Lorg/gradle/api/provider/Provider;
 	public static final fun signingKey (Lorg/gradle/api/Project;)Ljava/lang/String;
 	public static final fun useKotlinDebug (Lorg/gradle/api/Project;)Lorg/gradle/api/provider/Provider;
 }

diff --git a/fluxo-kmp-conf/build.gradle.kts b/fluxo-kmp-conf/build.gradle.kts
@@ -25,7 +25,7 @@ setupGradlePlugin(
     githubProject = "fluxo-kt/fluxo-kmp-conf"
     enableSpotless = true
     setupCoroutines = false
-    shrinkArtifacts = true
+    shrinkArtifacts = false
     shrinkingConfig.r8FullMode = true
     experimentalLatestCompilation = true
 

diff --git a/fluxo-kmp-conf/detekt-baseline.xml b/fluxo-kmp-conf/detekt-baseline.xml
@@ -26,6 +26,7 @@
     <ID>ForbiddenComment:DependencyUtils.kt$// TODO: Support rich VersionConstraint</ID>
     <ID>ForbiddenComment:FluxoConfigurationExtensionAndroidImpl.kt$FluxoConfigurationExtensionAndroidImpl$// TODO: Avoid casting</ID>
     <ID>ForbiddenComment:FluxoConfigurationExtensionKotlin.kt$FluxoConfigurationExtensionKotlin$// FIXME: koverReport settings</ID>
+    <ID>ForbiddenComment:FluxoConfigurationExtensionPublicationImpl.kt$FluxoConfigurationExtensionPublicationImpl$// FIXME: Should be converted to a lazy provider!</ID>
     <ID>ForbiddenComment:FluxoConfigurationExtensionPublicationImpl.kt$FluxoConfigurationExtensionPublicationImpl$// TODO: Add validation for value. Shouldn't be url, but `namespace/name`</ID>
     <ID>ForbiddenComment:FluxoKmpConfContext.kt$FluxoKmpConfContext$// TODO: Better integration with `gradle-idea-ext-plugin` or `idea` plugins.</ID>
     <ID>ForbiddenComment:FluxoKmpConfPlugin.kt$FluxoKmpConfPlugin$// TODO: Support version catalog declarations if available</ID>
@@ -39,6 +40,7 @@
     <ID>ForbiddenComment:LoadAndApplyPluginIfNotApplied.kt$// TODO: Check version for correctness</ID>
     <ID>ForbiddenComment:MultiplatformSourceSetUtils.kt$// TODO: Create bundles once and reuse or at least make them lazy evaluatable.</ID>
     <ID>ForbiddenComment:NodeGraphsRenderer.kt$NodeGraphsRenderer.LegendRenderer$// FIXME: This is not implemented yet.</ID>
+    <ID>ForbiddenComment:PropsAndEnv.kt$// FIXME: Called 3 times with same error when no git installed or global config error.</ID>
     <ID>ForbiddenComment:PropsAndEnv.kt$// TODO: Optimize, make lazy accessible via root project</ID>
     <ID>ForbiddenComment:SetupAndroid.kt$// FIXME: Implement cleaner 2-levels lazy API: asAndroidApp { android { ... } }</ID>
     <ID>ForbiddenComment:SetupAndroid.kt$// FIXME: Implement cleaner 2-levels lazy API: asAndroidLib { android { ... } }</ID>
@@ -60,6 +62,13 @@
     <ID>ForbiddenComment:SetupDetekt.kt$// TODO: Add option to ignore baselines completely and fail on anything,</ID>
     <ID>ForbiddenComment:SetupDetekt.kt$// TODO: Improve detekt task platform detection.</ID>
     <ID>ForbiddenComment:SetupDetekt.kt$DetectedTaskPlatform$// TODO: Detect common/metadata tasks?</ID>
+    <ID>ForbiddenComment:SetupGradlePlugin.kt$// FIXME: Check BuildConfig tasks (not called on IDE sync!)</ID>
+    <ID>ForbiddenComment:SetupGradlePlugin.kt$// FIXME: Disambiguate existing javadoc and sources tasks</ID>
+    <ID>ForbiddenComment:SetupGradlePlugin.kt$// FIXME: Spotless stup</ID>
+    <ID>ForbiddenComment:SetupGradlePlugin.kt$// FIXME: check all features</ID>
+    <ID>ForbiddenComment:SetupGradlePlugin.kt$// FIXME: detekt plugins https://detekt.dev/marketplace/</ID>
+    <ID>ForbiddenComment:SetupGradlePlugin.kt$// FIXME: git hooks</ID>
+    <ID>ForbiddenComment:SetupGradlePlugin.kt$// FIXME: https://github.com/topjohnwu/libsu/blob/01570d643af91b0e271de018465a219eed8db322/service/build.gradle.kts#L21</ID>
     <ID>ForbiddenComment:SetupGradlePlugin.kt$// TODO: Retry create pluginId from configuration if null?</ID>
     <ID>ForbiddenComment:SetupGradlePlugin.kt$// TODO: Try avoid this cast?</ID>
     <ID>ForbiddenComment:SetupKotlin.kt$// TODO: Check KSP setup for KMP modules</ID>
@@ -100,7 +109,6 @@
     <ID>ForbiddenComment:TargetAndroidContainer.kt$TargetAndroidContainer$// FIXME: Implement API for source sets.</ID>
     <ID>ForbiddenComment:TargetAndroidContainer.kt$TargetAndroidContainer$// TODO: provide a `setupParentSourceSet` with a single SourceSet arg</ID>
     <ID>ForbiddenComment:TargetAndroidContainer.kt$TargetAndroidContainer$// TODO: should androidUnitTestDebug depend on androidUnitTest?</ID>
-    <ID>ForbiddenComment:TaskContainerUtils.kt$// TODO: Add method usage when compile against Gradle 8.6+.</ID>
     <ID>Indentation:DependencyUpdatesPlugin.kt.kt$ </ID>
     <ID>Indentation:FluxoConfigurationExtensionAndroidImpl.kt$FluxoConfigurationExtensionAndroidImpl$ </ID>
     <ID>Indentation:PropsAndEnv.kt$ </ID>

diff --git a/fluxo-kmp-conf/src/main/kotlin/PropsAndEnv.kt b/fluxo-kmp-conf/src/main/kotlin/PropsAndEnv.kt
@@ -4,10 +4,12 @@ import fluxo.conf.dsl.container.impl.KmpTargetCode.Companion.KMP_TARGETS_ALL_PRO
 import fluxo.conf.dsl.container.impl.KmpTargetCode.Companion.KMP_TARGETS_PROP
 import fluxo.conf.dsl.container.impl.KmpTargetCode.Companion.SPLIT_TARGETS_PROP
 import fluxo.conf.feat.LOAD_KMM_CODE_COMPLETION_FLAG
+import fluxo.conf.impl.e
 import fluxo.conf.impl.envOrPropFlag
 import fluxo.conf.impl.envOrPropFlagValue
 import fluxo.conf.impl.envOrPropValue
 import fluxo.conf.impl.envOrPropValueLenient
+import fluxo.conf.impl.i
 import fluxo.conf.impl.memoizeSafe
 import java.util.regex.Pattern
 import org.gradle.api.Incubating
@@ -74,11 +76,13 @@ public fun Project?.buildNumberSuffix(default: String = "", delimiter: String =
 
 @Incubating
 @Suppress("ComplexCondition", "MagicNumber")
-public fun Project.scmTag(allowBranch: Boolean = true): Provider<String?> {
+internal fun Project.scmTag(allowBranch: Boolean = true): Provider<String?> {
     // TODO: Optimize, make lazy accessible via root project
     //  see com.diffplug.gradle.spotless.GitRatchetGradle
     //  com.diffplug.gradle.spotless.SpotlessTask.getRatchet
 
+    // FIXME: Called 3 times with same error when no git installed or global config error.
+
     return provider {
         var result = envOrPropValue("SCM_TAG")
         if (result.isNullOrBlank()) {
@@ -103,21 +107,29 @@ public fun Project.scmTag(allowBranch: Boolean = true): Provider<String?> {
 
 @Suppress("UnstableApiUsage")
 private fun Project.runCommand(command: String): String? {
-    // https://docs.gradle.org/7.5.1/userguide/configuration_cache.html#config_cache:requirements:external_processes
+    // https://docs.gradle.org/8.6/userguide/configuration_cache.html#config_cache:requirements:external_processes
     return try {
         val exec = providers.exec {
-            commandLine(command.split("\\s".toRegex()))
+            isIgnoreExitValue = true
+            commandLine(command.split(COMMAND_LINE_DELIMITER))
+            logger.i("Executing command: `{}`", command)
         }
+        val result = exec.result.get()
+        val exitCodeIsNormal = result.exitValue == 0
         val error = exec.standardError.asText.get()
         when {
-            error.isEmpty() -> exec.standardOutput.asText.get().trim().ifEmpty { null }
+            error.isEmpty() && exitCodeIsNormal -> exec.standardOutput.asText.get()
+                .trim().ifEmpty { null }
+
             else -> {
-                logger.error("Error running command `$command`: $error")
+                logger.e("Error running command `{}`: {}", command, error)
                 null
             }
         }
     } catch (e: Throwable) {
-        logger.error("Error running command `$command`: $e", e)
+        logger.e("Error running command `$command`: $e", e)
         null
     }
 }
+
+private val COMMAND_LINE_DELIMITER = "\\s".toRegex()
diff --git a/...mp-conf/src/main/kotlin/fluxo/conf/dsl/impl/FluxoConfigurationExtensionPublicationImpl.kt b/...mp-conf/src/main/kotlin/fluxo/conf/dsl/impl/FluxoConfigurationExtensionPublicationImpl.kt
@@ -150,6 +150,7 @@ internal interface FluxoConfigurationExtensionPublicationImpl :
         var scmUrl: String? = null
         var publicationUrl: String? = null
 
+        // FIXME: Should be converted to a lazy provider!
         val scmTag = when {
             !isSnapshot -> "v$version"
             else -> project.scmTag().orNull ?: defaultGitBranchName

diff --git a/fluxo-kmp-conf/src/main/kotlin/fluxo/conf/impl/LoggerUtils.kt b/fluxo-kmp-conf/src/main/kotlin/fluxo/conf/impl/LoggerUtils.kt
@@ -79,4 +79,6 @@ internal fun Logger.e(message: String, e: Throwable?) = error("$E$message", e)
 
 internal fun Logger.e(message: String, arg1: Any?) = error("$E$message", arg1)
 
+internal fun Logger.e(message: String, arg1: Any?, arg2: Any?) = error("$E$message", arg1, arg2)
+
 internal fun Logger.e(message: String, vararg args: Any?) = error("$E$message", *args)
diff --git a/fluxo-kmp-conf/src/main/kotlin/fluxo/gradle/CliArgUtils.kt b/fluxo-kmp-conf/src/main/kotlin/fluxo/gradle/CliArgUtils.kt
@@ -33,12 +33,11 @@ internal fun MutableCollection<String>.javaOption(value: String) =
 
 private fun <T : Any?> defaultToString(base: File? = null): (T) -> String =
     {
-        val asString = when (it) {
+        when (it) {
             is FileSystemLocation -> it.asFile.normalizedPath(base)
             is File -> it.normalizedPath(base)
             else -> it.toString()
         }
-        cliEscaped(asString)
     }
 
 /**
@@ -57,7 +56,7 @@ private fun cliEscaped(value: String): String {
 private fun String.requiresQuotes(): Boolean {
     return isEmpty() || any {
         it.isWhitespace() || when (it) {
-            '*', '?', '[', ']', '(', ')', '$', '|', ';', '&', '"', '\'' -> true
+            '*', '?', '[', ']', '(', ')', '$', '|', '\\', ';', '&', '"', '\'' -> true
             else -> false
         }
     }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -79,4 +79,6 @@ internal fun Logger.e(message: String, e: Throwable?) = error("$E$message", e)

		internal fun Logger.e(message: String, arg1: Any?) = error("$E$message", arg1)

		internal fun Logger.e(message: String, arg1: Any?, arg2: Any?) = error("$E$message", arg1, arg2)

		internal fun Logger.e(message: String, vararg args: Any?) = error("$E$message", *args)