-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Download and generate Spdx licenses #316
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have not reviewed the whole thing
abstract val generatedSpdx: DirectoryProperty | ||
|
||
init { | ||
inputJson.convention(project.resources.text.fromUri("https://spdx.org/licenses/licenses.json").asString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whaaaaaaat is going on here. Gradle has an entire HTTP client hidden in a property factory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I actually want to revert to continuing the embed the JSON, except we can embed it into the build-logic for this task to use. I'm generally wary of depending on the internet and would prefer updates be explicit rather than it magically getting new values based on whenever you build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whaaaaaaat is going on here. Gradle has an entire HTTP client hidden in a property factory?
Yup, since 2.2.
I think I actually want to revert to continuing the embed the JSON
So we download the json (manually/ automatically) using Gradle but keep the generator to generate Kotlin code from the Json to have typed Spdx Ids? Sounds okay for me too.
Yeah, and it can't fail a release due connections problems/new entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. And that's basically the goal. We don't want it to change out from underneath us. Also means old builds are reproducible, although not sure why that would ever be needed.
gradle/build-logic/src/main/kotlin/app/cash/licensee/GenerateSpdxIdTask.kt
Outdated
Show resolved
Hide resolved
gradle/build-logic/src/main/kotlin/app.cash.licensee.repos.settings.gradle.kts
Outdated
Show resolved
Hide resolved
gradle/build-logic/src/main/kotlin/app/cash/licensee/defaultFallbackUrls.kt
Outdated
Show resolved
Hide resolved
Is sorting needed? I filed a bug upstream a few years ago for them to do stable sorting. The last JSON update for 0.11 seemed to indicate they now sort. |
I don't know because I can only get the latest json but let's remove it and see it next update, and add it if needed. |
@@ -15,7 +15,7 @@ | |||
*/ | |||
package app.cash.licensee | |||
|
|||
internal val fallbackUrls = buildMap { | |||
internal val defaultFallbackUrls: FallbackBuilder.() -> Unit = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
internal val defaultFallbackUrls: FallbackBuilder.() -> Unit = { | |
@Suppress("HttpUrlsUsage") | |
internal val defaultFallbackUrls: FallbackBuilder.() -> Unit = { |
val fileSpec = FileSpec.builder("app.cash.licensee", "licensesSpdx") | ||
|
||
fileSpec.addType(addSpdxIdInterface()) | ||
|
||
return fileSpec.build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val fileSpec = FileSpec.builder("app.cash.licensee", "licensesSpdx") | |
fileSpec.addType(addSpdxIdInterface()) | |
return fileSpec.build() | |
return FileSpec.builder("app.cash.licensee", "licensesSpdx") | |
.addType(addSpdxIdInterface()) | |
.build() |
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package app.cash.licensee |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
package app.cash.licensee | |
@file:Suppress("HttpUrlsUsage") | |
package app.cash.licensee |
build.gradle
Outdated
RegularFile jsonFile = layout.projectDirectory.file("src/main/resources/app/cash/licensee/licenses.json") | ||
|
||
tasks.register("downloadLicensesJson", app.cash.licensee.SortedSpdx) { | ||
json.set(resources.text.fromUri("https://spdx.org/licenses/licenses.json").asString()) | ||
sortedLicencesJson.set(jsonFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RegularFile jsonFile = layout.projectDirectory.file("src/main/resources/app/cash/licensee/licenses.json") | |
tasks.register("downloadLicensesJson", app.cash.licensee.SortedSpdx) { | |
json.set(resources.text.fromUri("https://spdx.org/licenses/licenses.json").asString()) | |
sortedLicencesJson.set(jsonFile) | |
def jsonFile = layout.projectDirectory.file("src/main/resources/app/cash/licensee/licenses.json") | |
tasks.register("downloadLicensesJson", app.cash.licensee.SortedSpdx) { | |
json = resources.text.fromUri("https://spdx.org/licenses/licenses.json").asString() | |
sortedLicencesJson = jsonFile | |
} |
TaskProvider<app.cash.licensee.GenerateSpdxIdTask> generateSpdx = tasks.register('generateSpdx', app.cash.licensee.GenerateSpdxIdTask) { | ||
// Do not wire the output from writeSortedSpdx here to split downloading the remote file and code gen. | ||
inputJson.set(jsonFile) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TaskProvider<app.cash.licensee.GenerateSpdxIdTask> generateSpdx = tasks.register('generateSpdx', app.cash.licensee.GenerateSpdxIdTask) { | |
// Do not wire the output from writeSortedSpdx here to split downloading the remote file and code gen. | |
inputJson.set(jsonFile) | |
} | |
def generateSpdx = tasks.register('generateSpdx', app.cash.licensee.GenerateSpdxIdTask) { | |
// Do not wire the output from writeSortedSpdx here to split downloading the remote file and code gen. | |
inputJson = jsonFile | |
} |
import org.gradle.api.initialization.resolve.RepositoriesMode | ||
|
||
dependencyResolutionManagement { | ||
repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS) | |
repositoriesMode = RepositoriesMode.FAIL_ON_PROJECT_REPOS |
import org.gradle.api.initialization.resolve.RepositoriesMode | ||
|
||
dependencyResolutionManagement { | ||
repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS) | |
repositoriesMode = RepositoriesMode.FAIL_ON_PROJECT_REPOS |
import assertk.assertions.isNull | ||
import kotlin.test.Test | ||
|
||
class SpdxLicensesGeneratorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class SpdxLicensesGeneratorTest { | |
@Suppress("HttpUrlsUsage") | |
class SpdxLicensesGeneratorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will try to review the rest tonight
task.doLast { | ||
def destFile = new File(destDir, 'licenses.json') | ||
def json = new JsonSlurper().parse(destFile) | ||
json.licenses = json.licenses.sort { a,b -> a.licenseId <=> b.licenseId } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I COMPLETELY forgot that I was doing this. Maybe we do need sort... I thought they were finally sorting because they said they would.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverting bef2c7b is easy, but on the other hand, we could test it with the next license update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I lost track of this PR.
Yeah we need to revert. It's definitely not sorted, despite me asking.
Still very much open to merging this if you want to fix the merge conflict and revert that commit. If not, I'll try and find some time to do it here soon. |
I am open for a merge too, will do solve the conflicts. |
No description provided.