Skip to content

Commit

Permalink
Fix destination overwrite issue
Browse files Browse the repository at this point in the history
A previous change made the destination of the converter be the
parent folder of the recipe folder rather thant the folder itself,
leaving the decision of the folder name to the recipe (via metadata).

This change also impacted detection of existing destination folder, and
while it worked for batch conversion, it did not for single conversion.

This change fixes this, by separating the logic for single vs batch
conversion.

Single conversion expect the recipe folder to be empty, but the
parent (which is passed to the converted) can contain other things.
Overwritting will not impact the parent but the recipe folder only.

Batch conversion expect the recipe folder parent to be empty since
it's going to receive a lot of recipes. Overwritting will delete
the whole content of the parent (--destination) folder.

This also replaces all error handling to use System.exit instead
of exceptions as it's more user friendly (there's an option to
throw for debugging if needed)

Bug: N/A
Test: existing
Change-Id: Ie115ce6b8b13e4c77c0f99371f0d37d89e2f7a7b
  • Loading branch information
ducrohet committed Dec 27, 2023
1 parent 8d54ba8 commit 3a8a8e7
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,6 @@ fun main(args: Array<String>) {
printErrorAndTerminate("Folder does not exist: ${destinationPath.toAbsolutePath()}")
}

if (!destinationPath.isEmptyExceptForHidden()) {
if (!overwrite) {
error("the destination $destinationPath folder is not empty, call converter with --overwrite to overwrite it")
} else {
destinationPath.deleteNonHiddenRecursively()
}
}

if (finalSource != null) {
RecipeConverter(
agpVersion = agpVersion,
Expand All @@ -104,9 +96,20 @@ fun main(args: Array<String>) {
branchRoot = branchRoot,
).convert(
source = Path.of(finalSource),
destination = destinationPath
destination = destinationPath,
overwrite = overwrite
)
} else if (finalSourceAll != null) {
// if we do a convert all then we expect the root folder to be (mostly) empty
// (hidden files, like git files, are kept)
if (!destinationPath.isEmptyExceptForHidden()) {
if (!overwrite) {
printErrorAndTerminate("the destination $destinationPath folder is not empty, call converter with --overwrite to overwrite it")
} else {
destinationPath.deleteNonHiddenRecursively()
}
}

RecursiveConverter(
agpVersion = agpVersion,
repoLocation = repoLocation,
Expand Down Expand Up @@ -136,7 +139,7 @@ fun main(args: Array<String>) {

// check the env var for the SDK exist
if (System.getenv("ANDROID_HOME") == null) {
throw RuntimeException("To run $COMMAND_VALIDATE command, the environment variable ANDROID_HOME must be set and must point to your Android SDK.")
printErrorAndTerminate("To run $COMMAND_VALIDATE command, the environment variable ANDROID_HOME must be set and must point to your Android SDK.")
}

if (mode != null) {
Expand Down Expand Up @@ -233,15 +236,18 @@ private fun validateNullArg(arg: Any?, msg: String) {
}
}

private fun printErrorAndTerminate(msg: String): Nothing {
fun printErrorAndTerminate(msg: String): Nothing {
System.err.println(msg)
if (System.getenv("CONVERT_DEBUG") != null) {
throw RuntimeException("error. See console output")
}
exitProcess(1)
}

private fun Path.isEmptyExceptForHidden(): Boolean = !Files.list(this).anyMatch { !it.name.startsWith('.') }

@OptIn(ExperimentalPathApi::class)
private fun Path.deleteNonHiddenRecursively() {
fun Path.deleteNonHiddenRecursively() {
Files.list(this).filter {
!it.name.startsWith('.')
}.forEach {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.android.gradle_recipe.converter.converters

import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import com.google.android.gradle_recipe.converter.recipe.RecipeData
import com.google.android.gradle_recipe.converter.recipe.toMajorMinor
import java.nio.file.Files
Expand Down Expand Up @@ -79,7 +80,7 @@ abstract class Converter(
fun copyGradleFolder(dest: Path) {
val source = branchRoot.resolve(GRADLE_RESOURCES_FOLDER)
if (!source.isDirectory()) {
throw RuntimeException("Unable to find gradle resources at $source")
printErrorAndTerminate("Unable to find gradle resources at $source")
}

dest.mkdirs()
Expand All @@ -104,11 +105,11 @@ abstract class Converter(
protected fun getMinAgp(): String = recipeData?.minAgpVersion
// this really should not happen
// TODO(b/317888166) improve this
?: error("recipeData not loaded in Converter")
?: printErrorAndTerminate("recipeData not loaded in Converter")

protected fun getVersionInfoFromAgp(agpVersion: String): VersionInfo {
val agp = agpVersion.toMajorMinor()
return getVersionsFromAgp(branchRoot, agp)
?: throw RuntimeException("Unable to fetch VersionInfo for AGP $agp")
?: printErrorAndTerminate("Unable to fetch VersionInfo for AGP $agp")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.android.gradle_recipe.converter.converters

import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import java.nio.file.Path

/**
Expand All @@ -31,7 +32,7 @@ fun Path.mkdirs(): Boolean {
val ret = file.mkdirs()
if (!ret) {
if (!file.isDirectory) {
throw RuntimeException("Unable to create folder: $this")
printErrorAndTerminate("Unable to create folder: $this")
}
}
return ret
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,20 @@
*/
package com.google.android.gradle_recipe.converter.converters

import com.google.android.gradle_recipe.converter.deleteNonHiddenRecursively
import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import com.google.android.gradle_recipe.converter.recipe.RecipeData
import com.google.android.gradle_recipe.converter.recipe.toMajorMinor
import java.io.File
import java.io.IOException
import java.lang.System.err
import java.nio.file.*
import java.nio.file.FileVisitResult
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.SimpleFileVisitor
import java.nio.file.StandardCopyOption
import java.nio.file.attribute.BasicFileAttributes
import kotlin.io.path.exists
import kotlin.io.path.isDirectory
import kotlin.io.path.isRegularFile
import kotlin.io.path.name
import kotlin.io.path.readLines

private const val VERSION_MAPPING = "version_mappings.txt"
Expand Down Expand Up @@ -57,7 +60,7 @@ private fun initAgpToGradleMap(branchRoot: Path) {
if (!::agpToVersionsMap.isInitialized) {
val file = branchRoot.resolve(VERSION_MAPPING)
if (!file.isRegularFile()) {
throw RuntimeException("Missing AGP version mapping file at $file")
printErrorAndTerminate("Missing AGP version mapping file at $file")
}

val lines = file
Expand Down Expand Up @@ -141,7 +144,7 @@ class RecipeConverter(

Mode.RELEASE -> {
ReleaseConverter(
agpVersion = agpVersion ?: error("Must specify the AGP version for release"),
agpVersion = agpVersion ?: printErrorAndTerminate("Must specify the AGP version for release"),
gradleVersion = gradleVersion,
repoLocation = repoLocation,
gradlePath = gradlePath,
Expand All @@ -158,15 +161,27 @@ class RecipeConverter(
* @param destination the destination folder. A new folder will be created inside to contain the recipe
*
*/
fun convert(source: Path, destination: Path): ConversionResult {
fun convert(source: Path, destination: Path, overwrite: Boolean = false): ConversionResult {
if (!source.isDirectory()) {
error("the source $source folder is not a directory")
printErrorAndTerminate("Source $source is not a directory!")
}

val recipeData = RecipeData.loadFrom(source, mode)

val recipeDestination = destination.resolve(recipeData.destinationFolder)

if (recipeDestination.isRegularFile()) {
printErrorAndTerminate("Destination $recipeDestination exist but is not a folder!")
}

if (recipeDestination.isDirectory() && recipeDestination.isNotEmpty()) {
if (!overwrite) {
printErrorAndTerminate("Destination $recipeDestination folder is not empty, call converter with --overwrite to overwrite it")
} else {
recipeDestination.deleteNonHiddenRecursively()
}
}

val success = if (converter.isConversionCompliant(recipeData)) {
converter.recipeData = recipeData

Expand Down Expand Up @@ -232,4 +247,6 @@ class RecipeConverter(

return ConversionResult(recipeData, success)
}
}
}

private fun Path.isNotEmpty(): Boolean = Files.list(this).findAny().isPresent
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.android.gradle_recipe.converter.converters

import com.google.android.gradle_recipe.converter.converters.RecipeConverter.Mode
import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import com.google.android.gradle_recipe.converter.recipe.visitRecipes
import java.io.File
import java.io.IOException
Expand Down Expand Up @@ -45,10 +46,9 @@ class RecursiveConverter(
val link: String
)

@Throws(IOException::class)
fun convertAllRecipes(sourceAll: Path, destination: Path) {
if (!sourceAll.exists()) {
error("the source $sourceAll folder doesn't exist")
printErrorAndTerminate("the source $sourceAll folder doesn't exist")
}

val recipeConverter = RecipeConverter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.android.gradle_recipe.converter.converters

import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import com.google.android.gradle_recipe.converter.recipe.RecipeData
import java.nio.file.Files
import java.nio.file.Path
Expand Down Expand Up @@ -48,8 +49,8 @@ class ReleaseConverter(

} else {
// internal CI release
pathToAgpRepo = repoLocation ?: error("must specify path to repo")
pathToGradle = gradlePath ?: error("must specify path to Gradle")
pathToAgpRepo = repoLocation ?: printErrorAndTerminate("must specify path to repo")
pathToGradle = gradlePath ?: printErrorAndTerminate("must specify path to Gradle")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.google.android.gradle_recipe.converter.recipe

import com.github.rising3.semver.SemVer
import com.google.android.gradle_recipe.converter.converters.RecipeConverter
import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import org.tomlj.Toml
import org.tomlj.TomlParseResult
import java.io.File
Expand Down Expand Up @@ -59,7 +60,7 @@ class RecipeData private constructor(
if (parseResult.hasErrors()) {
System.err.println("TOML Parsing error(s) for $toml:")
parseResult.errors().forEach { error -> System.err.println(error.toString()) }
throw IllegalArgumentException("Unable to read $toml")
printErrorAndTerminate("Unable to read $toml")
}

val indexName = if (mode == RecipeConverter.Mode.RELEASE) {
Expand All @@ -80,7 +81,7 @@ class RecipeData private constructor(
} else {
// check there's no path separator in there
if (entry.contains('/')) {
error("destinationFolder value ('$entry') cannot contain / character ($recipeFolder)")
printErrorAndTerminate("destinationFolder value ('$entry') cannot contain / character ($recipeFolder)")
}
entry
}
Expand All @@ -92,7 +93,7 @@ class RecipeData private constructor(
indexName = indexName,
destinationFolder = destinationFolder,
minAgpVersion = parseResult.getString("agpVersion.min")
?: error("Did not find mandatory 'agpVersion.min' in $toml"),
?: printErrorAndTerminate("Did not find mandatory 'agpVersion.min' in $toml"),
maxAgpVersion = parseResult.getString("agpVersion.max"),
tasks = parseResult.getArray("gradleTasks.tasks")?.toList()?.map { it as String } ?: emptyList(),
keywords = parseResult.getArray("indexMetadata.index")?.toList()?.map { it as String } ?: emptyList()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.android.gradle_recipe.converter.recipe

import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import java.nio.file.Files
import java.nio.file.Path
import kotlin.io.path.exists
Expand All @@ -26,7 +27,7 @@ import kotlin.io.path.isHidden
*/
fun visitRecipes(rootFolder: Path, recipeCallback: (Path) -> Unit) {
if (!rootFolder.exists()) {
error("the source $rootFolder folder doesn't exist")
printErrorAndTerminate("the source $rootFolder folder doesn't exist")
}

Files.walk(rootFolder).filter { currentPath: Path ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class InternalCIValidator(
private val gradlePath: String,
private val branchRoot: Path,
) {
@Throws(IOException::class)
fun validate(sourceAll: Path, tmpFolder: Path?) {

val converter = RecipeConverter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.google.android.gradle_recipe.converter.converters.RecipeConverter
import com.google.android.gradle_recipe.converter.converters.RecipeConverter.Mode
import com.google.android.gradle_recipe.converter.converters.getMaxAgp
import com.google.android.gradle_recipe.converter.converters.getVersionsFromAgp
import com.google.android.gradle_recipe.converter.printErrorAndTerminate
import com.google.android.gradle_recipe.converter.recipe.RecipeData
import com.google.android.gradle_recipe.converter.recipe.toMajorMinor
import java.nio.file.Path
Expand Down Expand Up @@ -50,7 +51,7 @@ class MinMaxCurrentAgpValidator(
agpVersion: String,
) {
val gradleVersion = getVersionsFromAgp(branchRoot, agpVersion.toMajorMinor())?.gradle
?: throw RuntimeException("Unable to find Gradle version for AGP version $agpVersion - Make sure it's present in version_mappings.txt")
?: printErrorAndTerminate("Unable to find Gradle version for AGP version $agpVersion - Make sure it's present in version_mappings.txt")

val recipeConverter = RecipeConverter(
agpVersion = agpVersion,
Expand Down

0 comments on commit 3a8a8e7

Please sign in to comment.