Skip to content

[2.x] Avoid the use of URL + Replace tuple with proper record type for licenses#7927

Merged
eed3si9n merged 6 commits into
sbt:developfrom
eed3si9n:wip/uri
Dec 6, 2024
Merged

[2.x] Avoid the use of URL + Replace tuple with proper record type for licenses#7927
eed3si9n merged 6 commits into
sbt:developfrom
eed3si9n:wip/uri

Conversation

@eed3si9n
Copy link
Copy Markdown
Member

@eed3si9n eed3si9n commented Dec 5, 2024

Fixes #7736
See also #7753

Problem

java.net.URL calls out to the network to perform equals,
so we should remove that from anywhere that can be involved in caching etc.

Solution

  1. This changes java.net.URL to java.net.URI in Keys.
  2. This also cherry picks [2.x] Replace tuple with proper record type for licenses librarymanagement#382

eed3si9n and others added 3 commits December 4, 2024 22:12
**Problem**
`java.net.URL` calls out to the network to perform `equals`,
so we should remove that from anywhere that can be involved in caching etc.

**Solution**
This changes java.net.URL to java.net.URI in Keys.
@eed3si9n
Copy link
Copy Markdown
Member Author

eed3si9n commented Dec 5, 2024

I've signed Scala CLA a while back, so I'm not sure what the checker is saying.

Copy link
Copy Markdown
Contributor

@jtjeferreira jtjeferreira left a comment

Choose a reason for hiding this comment

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

LGTM, just 2 comments

def uri(s: String): URI = new URI(s)
def file(s: String): File = new File(s)
def url(s: String): URL = new URI(s).toURL
def url(s: String): URI = new URI(s)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about keeping def url(s: String): URL, but add a Conversion[(String, URL), License], with a deprecated annotation to nudge people into moving to `URI?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is also for homepage, apiURL settings etc. Returning URI maintains the source compatibility, acting as a compat function.

Comment thread lm-core/src/main/scala/sbt/librarymanagement/LibraryManagementSyntax.scala Outdated
@eed3si9n eed3si9n merged commit fdc03b7 into sbt:develop Dec 6, 2024
@eed3si9n eed3si9n deleted the wip/uri branch December 6, 2024 04:37
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.

3 participants