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

Don't expose java.net.URL #3192

Closed
wants to merge 3 commits into from

Conversation

boswelja
Copy link

@boswelja boswelja commented Sep 29, 2023

When constructing a URL from Gradle, builds will spew warnings such as

In plugin 'org.jetbrains.dokka' type 'org.jetbrains.dokka.gradle.DokkaTaskPartial' property 'unsuppressedSourceSets.$0.externalDocumentationLinks.$0.packageListUrl' has @Input annotation used on type 'java.net.URL' or a property of this type. Reason: Type 'java.net.URL' is not supported on properties annotated with @Input because Java Serialization can be inconsistent for this type. For more information, please refer to https://docs.gradle.org/8.3/userguide/validation_problems.html#unsupported_value_type in the Gradle documentation.

This pull request is intended to remove problematic usages of java.net.URL, and thus resolve any compile-time warnings related to this.

This is a breaking change.

Resolves #2993 and #3120

@boswelja boswelja marked this pull request as ready for review September 29, 2023 10:20
@IgnatBeresnev IgnatBeresnev self-requested a review October 11, 2023 22:43
@IgnatBeresnev IgnatBeresnev self-assigned this Oct 11, 2023
@pdvrieze
Copy link

Why not store the values as URI (to replace URL) rather than String? There is no problem with URI

@boswelja
Copy link
Author

Is there any special advantage a URI has over a String in this case?

@pdvrieze
Copy link

Is there any special advantage a URI has over a String in this case?

URI's allow various operations, they also restrict invalid inputs. Finally the implementation will still need to use URL (URI.toURL) for the connection so URI's would fit there.

But perhaps mostly, URI is a more specific type.

@whyoleg
Copy link
Contributor

whyoleg commented Jan 8, 2024

Hey!
Thanks for the PR and all discussions here!
And sorry for the delay with review/answer (we are busy now with preparing the next version).

But I have some news now:

  1. Unfortunately, we will not be able to accept this PR because of breaking changes (both binary and source). Breaking build scripts for users, even with good intentions, is not what we want to do now (more info on why in third bullet)
  2. During refactoring happened in Refactor project structure and introduce composite builds #3174 (rather big PR with a lot of changes) we also updated our build to Gradle 8.4 and so came across this issue. We were able to overcome it not breaking API/ABI. You can take a look at the approach here. In the end, in upcoming release (1.9.20) it will be fine to use URL there and no warning will be emitted.
  3. We know that this solution may look like a hack, but we should keep backward compatibility. Though, during the implementation of the new Gradle plugin (Gradle Runner 2.0 #3131) we will make sure that URL is not exposed.

Additional note: to avoid receiving deprecation warnings when running Gradle with JDK 20 better to use URI("...").toURL() instead of URL("...").
Example:

sourceLink {
   remoteUrl.set(URI("https://github.com/kotlin/dokka/tree/master/src").toURL())
}

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.

Replace java.net.URL with alternative, e.g. java.net.URI
5 participants