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.

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

Update build config to be more modular, using buildSrc convention plugins #2703

Open
aSemy opened this issue Oct 11, 2022 · 7 comments
Open
Labels
awaiting response An issue/PR that cannot be completed without additional information or a discussion enhancement An issue for a feature or an overall improvement infrastructure Everything related to builds tools, CI configurations and project tooling

Comments

@aSemy
Copy link
Contributor

aSemy commented Oct 11, 2022

Is your feature request related to a problem? Please describe

Presently the Dokka Gradle config uses a lot of allprojects {} and subprojects {} blocks to share configuration, but this is discouraged by Gradle

Using inheritance makes configuring the project difficult, as it prevents subprojects from being ordered nicely (for example, it's not easily possible to have 'empty' subprojects, as the subprojects{} block will add the plugins to them). It also means configuration is applied, even when not desired. For example, Dokka always applies the kotlin("jvm") plugin

plugin("org.jetbrains.kotlin.jvm")

But that's not required for the Gradle plugin project, as Gradle will supply the correct Kotlin plugin when the kotlin-dsl plugin is applied.

Describe the solution you'd like

Dokka uses buildSrc convention plugins to configure and apply configuration in a modular way.

Describe alternatives you've considered

Additional context

This would help with implementation of #2700, so the Gradle Plugin subproject would not have unecessary configuration applied to it (like the kotlin("jvm") plugin.

Are you willing to provide a PR?

I have made a start, but there's a lot of work to do!

@aSemy aSemy added the enhancement An issue for a feature or an overall improvement label Oct 11, 2022
@IgnatBeresnev IgnatBeresnev added the runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin label Oct 12, 2022
@aSemy
Copy link
Contributor Author

aSemy commented Oct 13, 2022

Other improvements I think can be made

@IgnatBeresnev IgnatBeresnev added infrastructure Everything related to builds tools, CI configurations and project tooling and removed runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin labels Mar 7, 2023
@IgnatBeresnev
Copy link
Member

@aSemy looks like the issue can be closed as completed? There's a PR for the module catalog, and integration tests I'd like to be refactored/re-written separately, there's a lot to improve..

@IgnatBeresnev IgnatBeresnev added the awaiting response An issue/PR that cannot be completed without additional information or a discussion label Mar 7, 2023
@aSemy
Copy link
Contributor Author

aSemy commented Mar 8, 2023

There's some more work to do to make the build modular, as there's still some cross-project dependencies (e.g. publishing to Maven in integration tests). #2704 really laid down the ground work, so fixing these problems won't take as long to fix. I'm going to wait until #2652 is done though.

Of course I don't need an issue to make PR, so this issue can be closed.

@aSemy
Copy link
Contributor Author

aSemy commented Mar 28, 2023

The next few tasks I see are:

@aSemy
Copy link
Contributor Author

aSemy commented Mar 28, 2023

I'm puzzled about what to do with ValidatePublications. It needs an update (it uses cross-project config so it's not compatible with project isolation, and it uses project at runtime which isn't compatible with Configuration Cache).

I can see the value of a test to make sure that publications are configured correctly, especially since there are multiple repos that need to be enabled/disabled conditionally based on the Dokka version and publication type, but I'm not sure how to implement that using a Gradle task...

@IgnatBeresnev
Copy link
Member

@aSemy I'll explain how it works and what I think it was made for, maybe it'll give you some ideas.

In TeamCity we have the following configuration parameters for publishing:

2023-04-06_22-37-16

Where the dokka_version dropdown contains:

  • snapshot
  • dev
  • publish
  • rc

Based on the chosen dokka_version, it adds a suffix to dokka_version_number, and then passes the resulting variable as the dokka_version Gradle property, which is used for publishing (and which is validated). Note: the publish type adds no suffix.

So if dokka_version == dev && dokka_version_number == 1.8.20, it will run the build with dokka_version = 1.8.20-dev.

The validate publication task asserts that the chosen dokka_publication_channels can accept the dokka_version that was passed.

So that we don't end up publishing 1.8.20-dev to Maven Central, or 1.8.20-SNAPSHOT to Space.

Note: the gradle-plugin-portal is broken atm, it doesn't work. There's a separate checkbox "Gradle plugin portal upload" - if it's set, TeamCity runs the publishPlugins task first, and only then dokkaPublish


Whether that's something that we need is a good question... It's nice to have, but if it's very difficult to implement or support, we can discuss dropping it.

checkProjectDependenciesArePublished() - this check also seems nice, but I wonder if it's needed or if there's a better way to do that? From what I understand, it checks that all of the internal project dependencies are published to a repository. I'm sure it must be a common concern in libraries?

@aSemy
Copy link
Contributor Author

aSemy commented Jun 4, 2023

Thanks for the info @IgnatBeresnev. I've been digging around in the code and seeing the intended usage makes it more clear.

Something else to note is that ValidatePublications was created before the integration tests that use :publishToMavenLocal.

Current goal

My current goal is to refactor the build logic to make Dokka build config one step closer to config cache compatibility, which is required for updating to Gradle 8.

Proposed actions

Here's a list of my proposed actions:


checkProjectDependenciesArePublished() - this check also seems nice, but I wonder if it's needed or if there's a better way to do that? From what I understand, it checks that all of the internal project dependencies are published to a repository. I'm sure it must be a common concern in libraries?

It's a really nice idea, but it would need a big re-write to be compatible with the newer Gradle restrictions.

However, I don't think it's necessary any more. The check was written before Dokka had any integration tests. This is no longer the case, and Dokka verifies publications are published by using MavenLocal. So, I think it can be safely removed!

Here are two ideas I have to re-write the test to be compatible. Both of these would be complicated and redundant, but they're fun to think about.

  • Each published subproject also produces a "I've been published with $group:$artifact:$version" report file (similar to the frontend files are shared).

    Other projects would fetch these files, and confirm each of their Dokka-subproject runtime dependencies has provided a report file, with the correct GAV.

  • Create a 'test-build-infra' subproject that uses Gradle TestKit to load Dokka, and run publishing tasks, and verify the results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response An issue/PR that cannot be completed without additional information or a discussion enhancement An issue for a feature or an overall improvement infrastructure Everything related to builds tools, CI configurations and project tooling
Projects
None yet
Development

No branches or pull requests

2 participants