Skip to content
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.

Sign up for GitHub

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

Jump to bottom

improve copying base-frontend files between subprojects #2970

Merged
merged 4 commits into from
May 25, 2023
Merged

improve copying base-frontend files between subprojects #2970

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
62eb776
improve copying base-frontend files between subprojects
aSemy Apr 18, 2023
588c6f2
Merge branch 'master' into fix/base_frontend_files
aSemy May 19, 2023
dc94d2c
fix versions in libs.versions.toml
aSemy May 20, 2023
f571ae6
rename dokka-base-frontend-files to dokka-html-frontend-files
aSemy May 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions build-logic/src/main/kotlin/org/jetbrains/conventions/dokka-base-frontend-files.gradle.kts
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This convention plugin adds shared config based on Variant-aware sharing of artifacts between projects docs

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.jetbrains.conventions

import org.gradle.api.attributes.Usage.USAGE_ATTRIBUTE

/**
* Utility for sharing the Dokka Base Plugin frontend files between subprojects in a safe, cacheable way.
*/

plugins {
id("org.jetbrains.conventions.base")
}

/** Apply a distinct attribute to the incoming/outgoing configuration */
fun AttributeContainer.dokkaBaseFrontendFilesAttribute() =
attribute(USAGE_ATTRIBUTE, objects.named("org.jetbrains.dokka.base-frontend-files"))
Copy link
Member

Choose a reason for hiding this comment

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

I would propose to rename all mentions of base-frontend-files to html-frontend-files while we're here.

The HTML plugin (and thus the format itself) will one day be extracted out of base and into it's own module, it's already been attempted in #2752, but we switched to more urgent matters

This convention plugin also has great potential for reusability, and in theory the frontend files could reside in multiple other modules that have nothing to do with base, so to me it makes sense to rename it to something broader

Copy link
Contributor Author

@aSemy aSemy May 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I would propose to rename all mentions of base-frontend-files to html-frontend-files while we're here.

Good idea, done!

By the way, now that the Gradle config of Dokka is more Gradle-idiomatic (e.g. there's no allprojects {} Gradle configuration, subprojects are mostly isolated), it might help to think about restructuring the subprojects while you're doing this.

Personally I think the subproject nesting and terse subproject names can be confusing, so I prefer a more flat layout so that all of the subprojects are visible at once, while the subproject names are expanded so they are ordered. So something like this:

.
├── dokka-core/
│   ├── dokka-generator
│   ├── dokka-generator-configuration
│   ├── dokka-content-matcher-test-utils
│   └── dokka-test-api
└── dokka-plugins/
    ├── dokka-html
    ├── dokka-html-frontend
    ├── dokka-gfm
    ├── dokka-gfm-template-processing
    └── ...

(this example includes some theoretical subprojects, and note that it won't necessarily affect the Maven coordinates, which can be set independently)

The Kotest and Gradle projects are examples of this layout that I like.

Just sharing an idea :)

Copy link
Member

@IgnatBeresnev IgnatBeresnev May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Good suggestion for sure, there's definitely lots of room for improvement!

Although there's a bit too much refactoring going on at the moment with K2 and the extraction of the html format out of dokka-base, it'll introduce a lot of nasty merge conflicts for several long-lived branches... Should be easier to do it closer to ~1.9.20 when as much as possible gets merged into master though


// incoming configuration
val dokkaBaseFrontendFiles by configurations.registering {
description =
"Retrieve Dokka Base Plugin frontend files from other subprojects... but the subproject must :plugins:base:frontend."
isCanBeConsumed = false
isCanBeResolved = true
attributes { dokkaBaseFrontendFilesAttribute() }
}

// outgoing configuration
val dokkaBaseFrontendFilesElements by configurations.registering {
description = "Provide Dokka Base Plugin frontend files to other subprojects"
isCanBeConsumed = true
isCanBeResolved = false
attributes { dokkaBaseFrontendFilesAttribute() }
}
2 changes: 0 additions & 2 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import org.jetbrains.ValidatePublications
import org.jetbrains.kotlin.gradle.tasks.*
import org.jetbrains.publicationChannels

@Suppress("DSL_SCOPE_VIOLATION") // fixed in Gradle 8.1 https://github.com/gradle/gradle/pull/23639
Expand Down Expand Up @@ -40,7 +39,6 @@ apiValidation {
// note that subprojects are ignored by their name, not their path https://github.com/Kotlin/binary-compatibility-validator/issues/16
ignoredProjects += setOf(
// NAME PATH
"search-component", // :plugins:search-component
"frontend", // :plugins:base:frontend

"kotlin-analysis", // :kotlin-analysis
Expand Down
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ gradlePlugin-nexusPublish = "1.1.0"
gradlePlugin-dokka = "1.7.10"
gradlePlugin-gradlePluginPublish = "0.20.0"
gradlePlugin-gradle = "4.0.1"
gradlePlugin-gradleNode = "3.5.1"

## NPM ##
node = "16.13.0"
Expand Down Expand Up @@ -113,3 +114,4 @@ kotlinx-binaryCompatibilityValidator = { id = "org.jetbrains.kotlinx.binary-comp
shadow = { id = "com.github.johnrengelman.shadow", version.ref = "gradlePlugin-shadow" }
gradle-pluginPublish = { id = "com.gradle.plugin-publish", version.ref = "gradlePlugin-gradlePluginPublish" }
nexusPublish = { id = "io.github.gradle-nexus.publish-plugin", version.ref = "gradlePlugin-nexusPublish" }
gradleNode = { id = "com.github.node-gradle.node", version.ref = "gradlePlugin-gradleNode" }
7 changes: 0 additions & 7 deletions plugins/base/.gitignore
View file
Open in desktop

This file was deleted.

50 changes: 23 additions & 27 deletions plugins/base/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import org.jetbrains.registerDokkaArtifactPublication
plugins {
id("org.jetbrains.conventions.kotlin-jvm")
id("org.jetbrains.conventions.maven-publish")
id("org.jetbrains.conventions.dokka-base-frontend-files")
}

dependencies {
Expand Down Expand Up @@ -33,45 +34,40 @@ dependencies {
testImplementation(projects.core.testApi)
testImplementation(platform(libs.junit.bom))
testImplementation(libs.junit.jupiter)

dokkaBaseFrontendFiles(projects.plugins.base.frontend) {
because("fetch frontend files from subproject :plugins:base:frontend")
}
}

val projectDistDir = project(":plugins:base:frontend").file("dist")
val generateFrontendFiles = tasks.getByPath(":plugins:base:frontend:generateFrontendFiles")
// access the frontend files via the dependency on :plugins:base:frontend
val dokkaBaseFrontendFiles: Provider<FileCollection> =
configurations.dokkaBaseFrontendFiles.map { frontendFiles ->
frontendFiles.incoming.artifacts.artifactFiles
}

val prepareDokkaBaseFrontendFiles by tasks.registering(Sync::class) {
description = "copy Dokka Base frontend files into the resources directory"

val copyJsFiles by tasks.registering(Copy::class) {
from(projectDistDir) {
from(dokkaBaseFrontendFiles) {
include("*.js")
into("dokka/scripts")
}
dependsOn(generateFrontendFiles)
destinationDir =
File(sourceSets.main.get().resources.sourceDirectories.singleFile, "dokka/scripts")
}

val copyCssFiles by tasks.registering(Copy::class) {
from(projectDistDir) {
from(dokkaBaseFrontendFiles) {
include("*.css")
into("dokka/styles")
}
dependsOn(generateFrontendFiles)
destinationDir =
File(sourceSets.main.get().resources.sourceDirectories.singleFile, "dokka/styles")
}

val copyFrontend by tasks.registering {
dependsOn(copyJsFiles, copyCssFiles)
into(layout.buildDirectory.dir("generated/src/main/resources"))
}
Copy link
Contributor Author

@aSemy aSemy Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Currently the frontend files are copied directly into src/main/resources. This works fine, but it might cause false-positives if there's an issue where the frontend files aren't generated - any previous files will remain and continue to work, until you run clean.

Sync tasks are better for mirroring files. However, they will delete any other files in the target directory...

So instead I synced the files into a new directory, build/generated/src/main/... and converted the task to be a file provider, and added that file to the resources dir.

Which IntelliJ also recognises

image

And there's no need for setting up task dependencies - since the task was converted to a file-provider, Gradle will be able to keep track of the task and it will understand that the task must run for the resources to be generated.


tasks {
processResources {
dependsOn(copyFrontend)
}

sourcesJar {
dependsOn(processResources)
}
sourceSets.main {
resources.srcDir(prepareDokkaBaseFrontendFiles.map { it.destinationDir })
}

test {
maxHeapSize = "4G"
}
tasks.test {
maxHeapSize = "4G"
}

registerDokkaArtifactPublication("dokkaBase") {
Expand Down
43 changes: 31 additions & 12 deletions plugins/base/frontend/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import com.github.gradle.node.npm.task.NpmTask
import org.jetbrains.kotlin.util.parseSpaceSeparatedArgs

@Suppress("DSL_SCOPE_VIOLATION") // fixed in Gradle 8.1 https://github.com/gradle/gradle/pull/23639
plugins {
base
id("com.github.node-gradle.node") version "3.2.1"
id("org.jetbrains.conventions.dokka-base-frontend-files")
alias(libs.plugins.gradleNode)
}

node {
Expand All @@ -11,19 +15,34 @@ node {
distBaseUrl.set(null as String?) // Strange cast to avoid overload ambiguity
}

val npmRunBuild = tasks.getByName("npm_run_build") {
inputs.dir(file("src/main"))
inputs.files(file("package.json"), file("webpack.config.js"))
outputs.dir(file("dist/"))
val distributionDirectory = layout.projectDirectory.dir("dist")

val npmRunBuild by tasks.registering(NpmTask::class) {
dependsOn(tasks.npmInstall)

npmCommand.set(parseSpaceSeparatedArgs("run build"))

inputs.dir("src/main")
inputs.files(
"package.json",
"webpack.config.js",
)

outputs.dir(distributionDirectory)
outputs.cacheIf { true }
}
Comment on lines +20 to 33
Copy link
Contributor Author

Choose a reason for hiding this comment

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

some minor tidying here, which hopefully makes the task more understandable.

I removed the task-rule based creator because I think it's a little less 'magic'.


task("generateFrontendFiles") {
dependsOn(npmRunBuild)
configurations.dokkaBaseFrontendFilesElements.configure {
outgoing {
artifact(distributionDirectory) {
builtBy(npmRunBuild)
}
}
}

tasks {
clean {
delete(file("node_modules"), file("dist"))
}
tasks.clean {
delete(
file("node_modules"),
file("dist"),
)
}
1 change: 0 additions & 1 deletion settings.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ include(

":plugins:base",
":plugins:base:frontend",
":plugins:base:search-component",
aSemy marked this conversation as resolved.
Show resolved Hide resolved
":plugins:base:base-test-utils",
":plugins:all-modules-page",
":plugins:templating",
Expand Down