From 6fe076028be64cdf0c8eeafe377a7f4444beaa55 Mon Sep 17 00:00:00 2001 From: Dmitriy Voronin Date: Mon, 7 Sep 2020 22:26:35 +0300 Subject: [PATCH] remove system properties reading in :gradle:android (#568) * remove system properties reading in :gradle:android * remove compileSdkVersion mentions from build-check docs --- docs/content/docs/projects/BuildChecks.md | 18 --- .../kotlin/com/avito/android/AndroidSdk.kt | 128 ++++++------------ .../kotlin/com/avito/android/ApkSigner.kt | 33 ----- .../kotlin/com/avito/android/ApkSignerTest.kt | 54 -------- .../build_param_check/BuildChecksExtension.kt | 2 +- .../BuildParamCheckPlugin.kt | 3 +- .../CheckAndroidSdkVersionTask.kt | 6 +- .../kotlin/com/avito/ci/OutputsVerifier.kt | 2 +- .../kotlin/com/avito/ci/VerifyOutputsTask.kt | 21 ++- .../com/avito/ci/steps/VerifyArtifactsStep.kt | 13 +- .../kotlin/com/avito/plugin/SignVerifier.kt | 4 +- 11 files changed, 67 insertions(+), 217 deletions(-) delete mode 100644 subprojects/gradle/android/src/main/kotlin/com/avito/android/ApkSigner.kt delete mode 100644 subprojects/gradle/android/src/test/kotlin/com/avito/android/ApkSignerTest.kt diff --git a/docs/content/docs/projects/BuildChecks.md b/docs/content/docs/projects/BuildChecks.md index 00a8a44851..fa572cb88c 100644 --- a/docs/content/docs/projects/BuildChecks.md +++ b/docs/content/docs/projects/BuildChecks.md @@ -19,18 +19,6 @@ plugins { {{%plugins-setup%}} -Some checks will require settings to work properly. -You'll get an error about missing settings: - -```text -> ./gradlew - -A problem occurred configuring root project -> buildChecks.androidSdk.compileSdkVersion must be set -``` - -After adding missing properties you will get something like this: - {{< tabs "build checks example" >}} {{< tab "Kotlin" >}} @@ -39,7 +27,6 @@ After adding missing properties you will get something like this: ```kotlin buildChecks { androidSdk { - compileSdkVersion = 29 revision = 4 } javaVersion { @@ -59,7 +46,6 @@ buildChecks { ```groovy buildChecks { androidSdk { - compileSdkVersion = 29 revision = 4 } javaVersion { @@ -149,7 +135,6 @@ buildChecks { enableByDefault = false androidSdk { - compileSdkVersion = 29 revision = 4 } } @@ -164,7 +149,6 @@ buildChecks { enableByDefault = false androidSdk { - compileSdkVersion = 29 revision = 4 } } @@ -221,7 +205,6 @@ Different revisions lead to Gradle remote cache misses. This check forces the sa ```kotlin buildChecks { androidSdk { - compileSdkVersion = 29 revision = 4 } } @@ -234,7 +217,6 @@ buildChecks { ```groovy buildChecks { androidSdk { - compileSdkVersion = 29 revision = 4 } } diff --git a/subprojects/gradle/android/src/main/kotlin/com/avito/android/AndroidSdk.kt b/subprojects/gradle/android/src/main/kotlin/com/avito/android/AndroidSdk.kt index b01b0f9d38..7318f2d99e 100644 --- a/subprojects/gradle/android/src/main/kotlin/com/avito/android/AndroidSdk.kt +++ b/subprojects/gradle/android/src/main/kotlin/com/avito/android/AndroidSdk.kt @@ -1,23 +1,10 @@ package com.avito.android import com.avito.utils.ExistingDirectory -import com.avito.utils.ExistingFile import com.avito.utils.ProcessRunner -import org.funktionale.tries.Try import org.gradle.api.Project import java.io.File import java.util.Properties -import kotlin.contracts.ExperimentalContracts -import kotlin.contracts.contract - -/** - * Используется для доступа к ресурсам SDK(executables, чтение параметров) из build-скриптов - */ -@Suppress("unused") -val Project.androidHome: String - get() = System.getenv("ANDROID_HOME") - ?: androidHomeFromLocalProperties(rootProject.file("local.properties")) { logger.error(it) } - ?: error("Can't resolve ANDROID_HOME") val Project.androidSdk: AndroidSdk get() = AndroidSdk(this, ProcessRunner.Real()) @@ -25,53 +12,47 @@ val Project.androidSdk: AndroidSdk class AndroidSdk( private val project: Project, private val processRunner: ProcessRunner -) : ApkSigner, Aapt { +) { - @Suppress("unused") - private val targetSdkVersion: Int - get() = requireNotNull(System.getProperty("targetSdkVersion").toIntOrNull()) { - "targetSdkVersion system property should be set" + private val buildToolsVersion: String + get() { + require(project.isAndroid()) { "Trying to get android.buildToolsVersion object on non-android project: ${project.path}" } + return project.androidBaseExtension.buildToolsVersion } private val compileSdkVersion: Int - get() = requireNotNull(System.getProperty("compileSdkVersion").toIntOrNull()) { - "compileSdkVersion system property should be set" - } - - - private val buildToolsVersion: String get() { - val value = System.getProperty("buildToolsVersion") - require(value.hasContent()) { "buildToolsVersion env must be set" } - return value + require(project.isAndroid()) { "Trying to get android.compileSdkVersion object on non-android project: ${project.path}" } + return requireNotNull( + value = project.androidBaseExtension.compileSdkVersion, + lazyMessage = { "compileSdkVersion is null for project: ${project.path}" }).apply { + if (this.isBlank()) { + error("compileSdkVersion not set for project: ${project.path}") + } + }.toInt() } - private val buildToolsPath: ExistingDirectory + /** + * it's not part of android sdk, provided here for convenience + */ + val keytool = KeyTool(processRunner) + + val androidHome: ExistingDirectory get() { - val dir = File(androidHome.dir, "/build-tools/$buildToolsVersion") - require(dir.exists()) { - """========= ERROR ========= - Android Build tools are not found in ${dir.path} - Please install it or update. - """.trimIndent() - } + val dir = System.getenv("ANDROID_HOME") + ?: androidHomeFromLocalProperties( + localPropertiesLocation = project.rootProject.file("local.properties"), + logger = { project.logger.error(it) }) + ?: error("Can't resolve ANDROID_HOME") return ExistingDirectory.Impl(dir) } - private val apkSigner: ApkSigner - get() = ApkSigner.Impl(buildToolsPath, processRunner) - - private val aapt: Aapt - get() = Aapt.Impl(buildToolsPath, processRunner) - - private val keytool = KeyTool(processRunner) - - val androidHome: ExistingDirectory - get() = ExistingDirectory.Impl(project.androidHome) + val aapt: Aapt + get() = Aapt.Impl(buildToolsPath(buildToolsVersion), processRunner) val androidJar: File get() { - val file = File(platformDir, "android.jar") + val file = File(platformDir(compileSdkVersion), "android.jar") require(file.exists()) { sdkNotFoundMessage(file.path) } @@ -79,29 +60,29 @@ class AndroidSdk( } val platformSourceProperties: File - get() = platformSourceProperties() + get() { + val file = File(platformDir(compileSdkVersion), "source.properties") + require(file.exists()) { + sdkNotFoundMessage(file.path) + } + return file + } - fun platformSourceProperties(compileSdkVersion: Int = this.compileSdkVersion): File { - val file = File(platformDir(compileSdkVersion), "source.properties") - require(file.exists()) { - sdkNotFoundMessage(file.path) + private fun buildToolsPath(buildToolsVersion: String): ExistingDirectory { + val dir = File(androidHome.dir, "/build-tools/$buildToolsVersion") + require(dir.exists()) { + """========= ERROR ========= + Android Build tools are not found in ${dir.path} + Please install it or update. + """.trimIndent() } - return file + return ExistingDirectory.Impl(dir) } - private fun platformDir(compileSdkVersion: Int = this.compileSdkVersion): File { + private fun platformDir(compileSdkVersion: Int): File { return File(androidHome.dir, "platforms/android-$compileSdkVersion") } - private val platformDir: File - get() = platformDir(compileSdkVersion) - - override fun getApkSha1(apk: ExistingFile): Try = apkSigner.getApkSha1(apk) - - override fun getPackageName(apk: File): Try = aapt.getPackageName(apk) - - fun getBundleSha1(aab: ExistingFile): Try = keytool.getJarSha1(aab) - private fun sdkNotFoundMessage(message: String): String { return """========= ERROR ========= Android SDK tools are not found. @@ -112,16 +93,8 @@ class AndroidSdk( } /** - * Используем в тестах, когда по какой-то причине не удалось зарезолвить env ANDROID_HOME - * Нужно только в тех местах где нет доступа к project, в нашем случае это на этапе создания android проекта для - * Gradle Test kit - * - * Все равно остается вариант при котором тесты не заработают: - * local.properties первый раз генерируется во время открытия проекта avito-android в Android Studio - * и не хранится под version control, поэтому если у разработчика не установлена ANDROID_HOME и он открывает - * напрямую проект в любой IDE, его ждет ошибка про "no sdk found" - * - * @return path до Android Sdk если получилось найти таким способом + * Used in tests, when for any reason ANDROID_HOME environment variable was not resolved + * Required in places where is no access to Project, mostly on Gradle Test Kit project creation */ fun androidHomeFromLocalPropertiesFallback(): String { val env = System.getenv("ANDROID_HOME") @@ -146,14 +119,3 @@ private fun androidHomeFromLocalProperties( null } } - -@OptIn(ExperimentalContracts::class) -private fun String?.hasContent(): Boolean { - contract { - returns(true) implies (this@hasContent != null) - } - - if (isNullOrBlank()) return false - if (this == "null") return false - return true -} diff --git a/subprojects/gradle/android/src/main/kotlin/com/avito/android/ApkSigner.kt b/subprojects/gradle/android/src/main/kotlin/com/avito/android/ApkSigner.kt deleted file mode 100644 index f4de092dbc..0000000000 --- a/subprojects/gradle/android/src/main/kotlin/com/avito/android/ApkSigner.kt +++ /dev/null @@ -1,33 +0,0 @@ -package com.avito.android - -import com.avito.utils.ExistingDirectory -import com.avito.utils.ExistingFile -import com.avito.utils.ProcessRunner -import org.funktionale.tries.Try - -interface ApkSigner { - - fun getApkSha1(apk: ExistingFile): Try - - class Impl(buildToolsPath: ExistingDirectory, private val processRunner: ProcessRunner) : ApkSigner { - - private val signatureRegex: Regex = Regex("SHA-1 digest: ([a-fA-F0-9]+)", RegexOption.MULTILINE) - - private val apkSignerPath: ExistingFile = buildToolsPath.file("/apksigner") - - override fun getApkSha1(apk: ExistingFile): Try { - return processRunner.run("$apkSignerPath verify --print-certs $apk") - .flatMap { - val result = signatureRegex.find(it) - ?.groupValues - ?.get(1) - - if (result != null) { - Try.Success(result) - } else { - Try.Failure(IllegalStateException("Can't parse signature")) - } - } - } - } -} diff --git a/subprojects/gradle/android/src/test/kotlin/com/avito/android/ApkSignerTest.kt b/subprojects/gradle/android/src/test/kotlin/com/avito/android/ApkSignerTest.kt deleted file mode 100644 index 8e6d038703..0000000000 --- a/subprojects/gradle/android/src/test/kotlin/com/avito/android/ApkSignerTest.kt +++ /dev/null @@ -1,54 +0,0 @@ -package com.avito.android - -import com.avito.utils.ExistingDirectory -import com.avito.utils.ExistingFile -import com.avito.utils.FakeProcessRunner -import com.google.common.truth.Truth.assertThat -import org.funktionale.tries.Try -import org.junit.jupiter.api.Test - -internal class ApkSignerTest { - - private val irrelevant = ExistingFile.Stub - - @Test - fun `parseApkSignature - returns SHA-1 from apksigner output`() { - val apkSignerOutput = """ - Signer #1 certificate DN: CN=Unknown, OU=Unknown, O=Unknown, L=Unknown, ST=Unknown, C=RU - Signer #1 certificate SHA-256 digest: 5ad4ea6ec6e8a9c8d5b8769a1bda93e7a19dd0748380d18a3759b9713f2301e1 - Signer #1 certificate SHA-1 digest: 6aef8d82c935cb0ae597ac31418df7f4f28505d6 - Signer #1 certificate MD5 digest: 1622ce002d84e5430ab59486f725c4b1 - WARNING: META-INF/android.arch.core_runtime.version not protected by signature. Unauthorized modifications to this JAR entry will not be detected. Delete or move the entry outside of META-INF/. - WARNING: META-INF/android.arch.lifecycle_extensions.version not protected by signature. Unauthorized modifications to this JAR entry will not be detected. Delete or move the entry outside of META-INF/. - """.trimIndent() - - val expected = "6aef8d82c935cb0ae597ac31418df7f4f28505d6" - - val processRunner = FakeProcessRunner() - - val apkSigner = ApkSigner.Impl(ExistingDirectory.Stub, processRunner) - - processRunner.result = Try.Success(apkSignerOutput) - - val actual = apkSigner.getApkSha1(irrelevant) - - assertThat(actual).isEqualTo(Try.Success(expected)) - } - - @Test - fun `parseApkSignature - returns null - apksigner is incorrect`() { - val apkSignerOutput = """ - There is no valid output - """.trimIndent() - - val processRunner = FakeProcessRunner() - - val apkSigner = ApkSigner.Impl(ExistingDirectory.Stub, processRunner) - - processRunner.result = Try.Success(apkSignerOutput) - - val actual = apkSigner.getApkSha1(irrelevant) - - assertThat(actual).isInstanceOf(Try.Failure::class.java) - } -} diff --git a/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildChecksExtension.kt b/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildChecksExtension.kt index 0dbd3fd7b3..d9b65f8f64 100644 --- a/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildChecksExtension.kt +++ b/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildChecksExtension.kt @@ -57,11 +57,11 @@ open class BuildChecksExtension { open class AndroidSdk : Check(), RequireParameters { + @Deprecated("not used anymore, remove after 2020.19") var compileSdkVersion: Int = -1 var revision: Int = -1 override fun validate() { - check(compileSdkVersion > 0) { "$extensionName.androidSdk.compileSdkVersion must be set" } check(revision > 0) { "$extensionName.androidSdk.minRevision must be set" } } } diff --git a/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildParamCheckPlugin.kt b/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildParamCheckPlugin.kt index 891c863454..e3958116e1 100644 --- a/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildParamCheckPlugin.kt +++ b/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/BuildParamCheckPlugin.kt @@ -44,7 +44,7 @@ open class BuildParamCheckPlugin : Plugin { registerRequiredTasks(project, checks) if (checks.hasInstance()) { - checkJavaVersion(checks.getInstance()) + checkJavaVersion(checks.getInstance()) } if (checks.hasInstance()) { checkGradleProperties(project) @@ -81,7 +81,6 @@ open class BuildParamCheckPlugin : Plugin { description = "Checks sdk version in docker against local one to prevent build cache misses" - compileSdkVersion.set(check.compileSdkVersion) revision.set(check.revision) // don't run task if it is already compared hashes and it's ok // task will be executed next time if either local jar or docker jar(e.g. inputs) changed diff --git a/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/CheckAndroidSdkVersionTask.kt b/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/CheckAndroidSdkVersionTask.kt index 84021a0302..0eb251943d 100644 --- a/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/CheckAndroidSdkVersionTask.kt +++ b/subprojects/gradle/buildchecks/src/main/kotlin/com/avito/android/plugin/build_param_check/CheckAndroidSdkVersionTask.kt @@ -12,9 +12,6 @@ import java.io.File abstract class CheckAndroidSdkVersionTask : DefaultTask() { - @get:Input - abstract val compileSdkVersion: Property - @get:Input abstract val revision: Property @@ -55,9 +52,8 @@ abstract class CheckAndroidSdkVersionTask : DefaultTask() { } private fun localRevision(): Int { - val sourceProperties: File = project.androidSdk.platformSourceProperties(compileSdkVersion.get()) + val sourceProperties: File = project.androidSdk.platformSourceProperties return requireNotNull(sourceProperties.loadProperties().getProperty("Pkg.Revision", null)) .toInt() } - } diff --git a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/OutputsVerifier.kt b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/OutputsVerifier.kt index 2b1c0093cc..f32d554c82 100644 --- a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/OutputsVerifier.kt +++ b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/OutputsVerifier.kt @@ -56,7 +56,7 @@ class OutputsVerifier( } fun checkPackageName(apkFile: File, expectedPackage: String) { - androidSdk.getPackageName(apkFile).fold( + androidSdk.aapt.getPackageName(apkFile).fold( { actualPackage -> if (actualPackage != expectedPackage) { errors += diff --git a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/VerifyOutputsTask.kt b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/VerifyOutputsTask.kt index ec3449dd97..7d0ca51086 100644 --- a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/VerifyOutputsTask.kt +++ b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/VerifyOutputsTask.kt @@ -7,16 +7,19 @@ import com.avito.plugin.SignVerifier import com.avito.utils.exhaustive import com.avito.utils.logging.ciLogger import org.gradle.api.DefaultTask +import org.gradle.api.model.ObjectFactory import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputDirectory import org.gradle.api.tasks.TaskAction +import org.gradle.kotlin.dsl.property import java.io.File +import javax.inject.Inject /** * Verifies that all declared release artifacts are built and copied to outputs folder * e.g. checks that all artifacts produces and placed correctly */ -abstract class VerifyOutputsTask : DefaultTask() { +abstract class VerifyOutputsTask @Inject constructor(objects: ObjectFactory) : DefaultTask() { /** * hardcoded here, because it's a contract check @@ -27,17 +30,21 @@ abstract class VerifyOutputsTask : DefaultTask() { private val outputsDir = File("${project.rootProject.rootDir.canonicalPath}/outputs") @Input - lateinit var config: ArtifactsConfiguration + val config = objects.property() @Input - var checkSignatures: Boolean = true + val checkSignatures = objects.property() @TaskAction fun doWork() { val signVerifier = SignVerifier.Impl(project.androidSdk) - val outputsVerifier = OutputsVerifier(project.androidSdk, signVerifier, outputsDir) + val outputsVerifier = OutputsVerifier( + androidSdk = project.androidSdk, + signVerifier = signVerifier, + outputsDir = outputsDir + ) - config.outputs.forEach { (_: String, output: Output) -> + config.get().outputs.forEach { (_: String, output: Output) -> // Check file that have already been copied to outputs folder val originalArtifact = File(output.path) .relativeTo(project.rootProject.projectDir) @@ -49,13 +56,13 @@ abstract class VerifyOutputsTask : DefaultTask() { is Output.ProguardMapping, is Output.FileOutput -> outputsVerifier.requireFile(artifactPath) is Output.ApkOutput -> outputsVerifier.requireFile(artifactPath) { - if (checkSignatures && output.signature != null) { + if (checkSignatures.get() && output.signature != null) { outputsVerifier.checkApkSignature(it, output.signature!!) } outputsVerifier.checkPackageName(artifactPath, output.packageName) } is Output.BundleOutput -> outputsVerifier.requireFile(artifactPath) { - if (checkSignatures && output.signature != null) { + if (checkSignatures.get() && output.signature != null) { outputsVerifier.checkBundleSignature(it, output.signature!!) } } diff --git a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/VerifyArtifactsStep.kt b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/VerifyArtifactsStep.kt index 8c7123ed2b..9b6aaaa25a 100644 --- a/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/VerifyArtifactsStep.kt +++ b/subprojects/gradle/cd/src/main/kotlin/com/avito/ci/steps/VerifyArtifactsStep.kt @@ -10,7 +10,6 @@ import com.avito.utils.logging.ciLogger import com.avito.utils.onBuildFailed import org.gradle.api.Project import org.gradle.api.Task -import org.gradle.api.invocation.Gradle import org.gradle.api.tasks.TaskContainer import org.gradle.api.tasks.TaskProvider import org.gradle.kotlin.dsl.register @@ -65,8 +64,8 @@ open class VerifyArtifactsStep( val verifyOutputsTask = project.tasks.register(verifyTaskName(context)) { group = "cd" description = "Checks that all defined release artifacts are present" - config = artifactsConfig - checkSignatures = artifactsConfig.failOnSignatureError + config.set(artifactsConfig) + checkSignatures.set(artifactsConfig.failOnSignatureError) dependsOn(copyTask) } @@ -79,11 +78,3 @@ open class VerifyArtifactsStep( internal fun verifyTaskName(context: String) = "${context}VerifyArtifacts" internal fun TaskContainer.verifyTaskProvider(context: String) = typedNamed(verifyTaskName(context)) - -private fun Gradle.isTaskScheduled(task: Task): Boolean? { - return try { - this.taskGraph.hasTask(task) - } catch (ex: IllegalStateException) { - null - } -} diff --git a/subprojects/gradle/signer/src/main/kotlin/com/avito/plugin/SignVerifier.kt b/subprojects/gradle/signer/src/main/kotlin/com/avito/plugin/SignVerifier.kt index d53a9bbbb6..b755fa781f 100644 --- a/subprojects/gradle/signer/src/main/kotlin/com/avito/plugin/SignVerifier.kt +++ b/subprojects/gradle/signer/src/main/kotlin/com/avito/plugin/SignVerifier.kt @@ -18,7 +18,7 @@ interface SignVerifier { class Impl(private val androidSdk: AndroidSdk) : SignVerifier { override fun verifyApk(apk: ExistingFile, expectedSha1: String): Result { - return androidSdk.getApkSha1(apk) + return androidSdk.keytool.getJarSha1(apk) .fold( { actualSha1 -> if (actualSha1 == expectedSha1) { @@ -31,7 +31,7 @@ interface SignVerifier { } override fun verifyBundle(bundle: ExistingFile, expectedSha1: String): Result { - return androidSdk.getBundleSha1(bundle).fold( + return androidSdk.keytool.getJarSha1(bundle).fold( { actualSha1 -> if (actualSha1 == expectedSha1) { Result.Ok