Skip to content

Make artifacts.json available as an asset #412

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

Merged
merged 10 commits into from
Mar 19, 2025
Merged

Make artifacts.json available as an asset #412

merged 10 commits into from
Mar 19, 2025

Conversation

jgbirk
Copy link
Collaborator

@jgbirk jgbirk commented Mar 13, 2025

By calling bundleAndroidAsset.set(true) the plugin will copy the generated artifacts.json into the corresponding variant assets folder.

Closes #181

@jgbirk jgbirk requested a review from JakeWharton March 13, 2025 01:28
Comment on lines 171 to 177
val capitalizedVariantName = variant.name.replaceFirstChar {
if (it.isLowerCase()) {
it.titlecase(ROOT)
} else {
it.toString()
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val capitalizedVariantName = variant.name.replaceFirstChar {
if (it.isLowerCase()) {
it.titlecase(ROOT)
} else {
it.toString()
}
}
vval capitalizedVariantName = variant.name.replaceFirstChar { it.titlecase(ROOT) }

@hfhbd
Copy link
Contributor

hfhbd commented Mar 16, 2025

While I do see a need, I don't think this is the solution.

  • It is android only
  • It only copies a file => Use Copy Task
  • There is no need to copy the file at all => just wire the output of licensee to the android resources
  • The task does not need this input

=> I am okay to wire the outputs of the task to the android resources, even by default if R8/D8 can remove the file if unused.

But if we do want to make the internal json file public available, we should also implement #410 to read the file. Or what is the real use-case? Should the user see the JSON file as is?
Or do you want to show it with some UI? Then you need parsing at all.

=> Or licensee could generate Kotlin code directly.

@jgbirk
Copy link
Collaborator Author

jgbirk commented Mar 16, 2025

While I do see a need, I don't think this is the solution.

  • It is android only
  • It only copies a file => Use Copy Task
  • There is no need to copy the file at all => just wire the output of licensee to the android resources
  • The task does not need this input

=> I am okay to wire the outputs of the task to the android resources, even by default if R8/D8 can remove the file if unused.

But if we do want to make the internal json file public available, we should also implement #410 to read the file. Or what is the real use-case? Should the user see the JSON file as is? Or do you want to show it with some UI? Then you need parsing at all.

=> Or licensee could generate Kotlin code directly.

What is the problem being android only? we support android specific stuff already.
I don't think so we wanna get into the business of parsing JSON files neither generating Kotlin code. We are providing a way for users to consume the JSON file as they see fit.
What do you think @JakeWharton ?

@hfhbd
Copy link
Contributor

hfhbd commented Mar 16, 2025

@jgbirk Like I said, I am okay to wire the outputs of the task to the android resources. But there is no need for a copy task, just use variant.sources.assets!!.addGeneratedSourceDirectory(task, LicenseeTask::outputDir).

But I still don't understand the real use case. The end user of the app will see the json file as it? That's very uncommon.

@jgbirk
Copy link
Collaborator Author

jgbirk commented Mar 16, 2025

@jgbirk Like I said, I am okay to wire the outputs of the task to the android resources. But there is no need for a copy task, just use variant.sources.assets!!.addGeneratedSourceDirectory(task, LicenseeTask::outputDir).

But I still don't understand the real use case. The end user of the app will see the json file as it? That's very uncommon.

👍 Got it, I can do that.
Correct, I'm just making the file visible to context.assets.
We have our own open source view, where we will parse the JSON using Moshi and display it. Other users might decide to use GSON, etc.

@hfhbd
Copy link
Contributor

hfhbd commented Mar 16, 2025

@jgbirk

We have our own open source view, where we will parse the JSON using Moshi and display it. Other users might decide to use GSON, etc.

Now I got your use-case.
But to prevent loading and parsing the json file at runtime again, it was just an idea to not provide a (unstable) json file, that needs another Json library, we could generate Kotlin code with proper api instead.

@jgbirk
Copy link
Collaborator Author

jgbirk commented Mar 16, 2025

variant.sources.assets!!.addGeneratedSourceDirectory(task, LicenseeTask::outputDir)

Maybe I'm missing something, but just that is not creating the file.

@Goooler
Copy link
Contributor

Goooler commented Mar 17, 2025

The reason for not using variant.sources.assets!!.addGeneratedSourceDirectory(task, LicenseeTask::outputDir) here is that outputDir contains both artifacts.json and validation.txt. Since validation.txt is useless for users, they would still need to exclude it using android.packaging.resources.excludes or a similar mechanism. I added a similar copy task logic at https://github.com/LawnchairLauncher/lawnicons/blob/815f9f21522545449eb1b1b78bf9c8f59f80f7bc/app/build.gradle.kts#L112-L125.

androidComponents.onVariants { variant ->
    val capName = variant.name.replaceFirstChar { it.titlecase(Locale.ROOT) }
    val licenseeTask = tasks.named<LicenseeTask>("licenseeAndroid$capName")
    val copyArtifactsTask = tasks.register<Copy>("copy${capName}Artifacts") {
        dependsOn(licenseeTask)
        from(licenseeTask.map { it.jsonOutput })
        // Copy artifacts.json to a new directory.
        into(layout.buildDirectory.dir("generated/dependencyAssets/${variant.name}"))
    }
    variant.sources.assets?.addGeneratedSourceDirectory(licenseeTask) {
        // Avoid using LicenseeTask::outputDir as it contains extra files that we don't need.
        objects.directoryProperty().fileProvider(copyArtifactsTask.map { it.destinationDir })
    }
}

@jgbirk
Copy link
Collaborator Author

jgbirk commented Mar 17, 2025

The reason for not using variant.sources.assets!!.addGeneratedSourceDirectory(task, LicenseeTask::outputDir) here is that outputDir contains both artifacts.json and validation.txt. Since validation.txt is useless for users, they would still need to exclude it using android.packaging.resources.excludes or a similar mechanism. I added a similar copy task logic at https://github.com/LawnchairLauncher/lawnicons/blob/815f9f21522545449eb1b1b78bf9c8f59f80f7bc/app/build.gradle.kts#L112-L125.

androidComponents.onVariants { variant ->
    val capName = variant.name.replaceFirstChar { it.titlecase(Locale.ROOT) }
    val licenseeTask = tasks.named<LicenseeTask>("licenseeAndroid$capName")
    val copyArtifactsTask = tasks.register<Copy>("copy${capName}Artifacts") {
        dependsOn(licenseeTask)
        from(licenseeTask.map { it.jsonOutput })
        // Copy artifacts.json to a new directory.
        into(layout.buildDirectory.dir("generated/dependencyAssets/${variant.name}"))
    }
    variant.sources.assets?.addGeneratedSourceDirectory(licenseeTask) {
        // Avoid using LicenseeTask::outputDir as it contains extra files that we don't need.
        objects.directoryProperty().fileProvider(copyArtifactsTask.map { it.destinationDir })
    }
}

Yep, that served me as inspiration for this!

@JakeWharton
Copy link
Collaborator

Been thinking about this over the weekend, although I did think about this originally when the tool was built.

My original intent was to parse this in a build plugin specific to our app, and either write out a new different JSON format or to generate Kotlin directly in the shape of our existing screen. The reason I wanted to do this was so that we could massage the data to look nicer while retaining the ability to fail the build if something was completely absent.

One of the ways our Open Source attribution screen looks better than others is that it's far more human readable and using logical groupings of artifacts rather than a simple linear display. For example, we could choose to put com.squareup.* under "Square" and app.cash.* under Cash App, or we could choose to put both plus xyz.block under the bland, plain white bread, boring-ass "Block" name.

Now we could simply embed the raw JSON and do all of this at runtime. There are no real performance or size concerns either way. We would lose the ability to fail the build if someone adds a dependency for which there is missing human-friendly data, though, although we obviously can always fall back to the machine data.

@@ -68,6 +68,9 @@ abstract class LicenseeTask : DefaultTask() {
@get:Input
internal abstract val unusedAction: Property<UnusedAction>

@get:Input
internal abstract val bundleAndroidAsset: Property<Boolean>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK it's not needed. And can you add a test without calling licensee?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah looks unused

Copy link
Collaborator

@JakeWharton JakeWharton Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a test, we probably want to enable the flag, build an APK (without explicitly running the task), and then maybe list its contents and look for the file? I'm not sure how thorough we really want to be here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just run assemble to test the task wiring?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that covers everything except the relative path within assets which is pretty good. Since I think we will already need a dedicated test function, adding a quick path check inside the APK shouldn't be much more work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the test case, let me know if its good.

@jgbirk jgbirk merged commit 769d8d2 into trunk Mar 19, 2025
2 checks passed
@jgbirk jgbirk deleted the asset-copy branch March 19, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Android sample project that embeds JSON as asset
4 participants