Give VerifyMigration task its own output directory #1380

[lfielke]

Currently the Gradle task that generates the source code and the VerifyMigration task both share the build/sqldelight directory. One of the first things that the VerifyMigration task does is to delete this directory. If a generate interfaces task has run, then that deleted directory contained the generated source code needed to compile the project!

This change avoids clobbering the generated source code generated by changing the VerifyMigration task to use a directory named build/sqldelight-verifyMigration instead.

It was certainly unexpected for me to discover that a check task deletes source code generated by an earlier task, so I don't think this was by design. Changing them to use separate directories fixes the racy behavior where the build will succeed or fail determined by the relative ordering of VerifyMigration and compile tasks. See #1380 for an example of this.

[JakeWharton]

I'm not sure this is enough. The Android and/or Kotlin multiplatform support likely means there are multiple of these tasks per module.

We probably need to move responsibility of creating this directory to the person who is setting up this task.

Additionally, I would prefer if we put everything under the 'sqldelight' folder instead of having multiple.

[lfielke]

Good point. I'd be happy to try and change the plugin to use directories something like this:

• generateInterface tasks: build/sqldelight/generated/{variant}
• verifyMigration tasks: build/sqldelight/verifymigration/{variant}

Are there any compatibility concerns with changing the current build/sqldelight output directory?

Also, feel free to suggest better names. I haven't done much with Gradle, so don't know the preferred conventions.

Note that variants aren't really working too well generally anyway, because the generateInterface tasks all delete and write to build/sqldelight and clobber each other. So for example in an Android project generateDebugDatabaseInterface and generateReleaseInterface will race with each other.

[JakeWharton]

No concerns changing directory structure. No one should be relying on it, and if they are we need to expose API for it, not just having it be relied on implicitly. Your names seem fine, though.

[AlecKazakova]

i think we can pretty safely make the change to have the current generate tasks output to build/sqldelight/generated and then have verification tasks output to build/sqldelight/verifyMigration

we currently dont do output to any variant specific folders. It's not the most lovely setup if you peak under the hood but it works, I don't think we need to change that in this PR but can investigate further later.

[JakeWharton]

We also need to have the database name in the path.

I think we want to do something like build/generated/sqldelight/REASON/DATABASE_NAME/VARIANT/ where REASON is "code" or "migration_verification", DATABASE_NAME is the DB name (obviously), and VARIANT is the android and/or kotlin MPP variant.

[lfielke]

I haven't had time to work on this, but I'll outline my thought process and where I'm blocked.

Two gradle tasks (GenerateSchemaTask, VerifyMigrationTask) need to have their output directory set as a property, like SqlDelightTask does already. Then change these tasks to write to that directory. That part should be straight forward.

The tricky part is getting the plugin to correctly set up these tasks. I'm a bit uncertain about "variants" and how these work. I see there is some code in SourceRoots.kt, but that looks a bit hairy. I can't find anything specific in the gradle.org docs about a general way variants are handled -- are they just inventions of the Android plugin and Kotlin MPP?

[lfielke]

I've force pushed new commits for the directory changes, basically as discussed but without any variant handling.

Generated source code is now written to a subdirectory in build/sqldelight/code (instead of directly in build/sqldelight).

VerifyMigrationsTask creates a subdirectory in build/sqldelight/migration_verification based on the task name that the plugin generates. This means that each VerifyMigrationsTask created by the plugin has its own working directory under migration_verification.

This avoids the races for deleting the directories in #1380.

[AlecKazakova]

dope - thanks