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

Fix rs-api packaging.type workaround #116

Merged

Conversation

mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Jul 26, 2023

When trying to add sbt-license-report to this repo I hit this issue

[warn] 	::::::::::::::::::::::::::::::::::::::::::::::
[warn] 	:: javax.ws.rs#javax.ws.rs-api;2.1.1: Resolution failed several times for dependency: javax.ws.rs#javax.ws.rs-api;2.1.1 {compile=[default(compile)]}:: 
[warn] 	java.net.URISyntaxException: Illegal character in path at index 88: https://repo1.maven.org/maven2/javax/ws/rs/javax.ws.rs-api/2.1.1/javax.ws.rs-api-2.1.1.${packaging.type}
[warn] 	java.net.URISyntaxException: Illegal character in path at index 88: https://repo1.maven.org/maven2/javax/ws/rs/javax.ws.rs-api/2.1.1/javax.ws.rs-api-2.1.1.${packaging.type}
[warn] 	::::::::::::::::::::::::::::::::::::::::::::::

sbt-license-report happens to use Ivy directly (see https://github.com/sbt/sbt-license-report/blob/261b48e290ad8613db21bc4a7026f9529783346a/src/main/scala/sbtlicensereport/license/LicenseReport.scala#L213-L229) to resolve these artifacts and I haven't found a way to get around this so instead I opted to fix the core problem by following jakartaee/rest#572 (comment)

Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm

@mdedetrich
Copy link
Contributor Author

This is a bit unfortunate that I only found this out during release because technically speaking since this PR is updating dependencies its going to create differing jars (more specifically, there won't be any change in JVM bytecode but the pom.xml will have a different entry in its test dependencies).

The reason why this is annoying is that I want to have the sbt-license-report for pekko-connectors-kafka 1.0.0 and its not going to be correct have a report that contains a test dependency which is different from whats in the source dist package and/or the current 1.0.0-RC1 binaries (the binaries is less of a problem because I suppose we can make an exception when I re-generate the artifacts for 1.0.0 but the source dist part is still an issue).

@pjfanning wdyt?

@pjfanning
Copy link
Contributor

I don't see an issue in small increments in jar dependencies, especially test ones

@mdedetrich
Copy link
Contributor Author

I don't see an issue in small increments in jar dependencies, especially test ones

Okay in that case I will merge this PR, it will mean that the 1.0.0 artifacts will be contain an additional change in pom.xml test dependencies.

@mdedetrich mdedetrich merged commit 1b4f010 into apache:main Jul 26, 2023
@mdedetrich mdedetrich deleted the fix-rs-api-packaging-type-workaround branch July 26, 2023 14:22
@pjfanning
Copy link
Contributor

pjfanning commented Jul 26, 2023

I don't see an issue in small increments in jar dependencies, especially test ones

Okay in that case I will merge this PR, it will mean that the 1.0.0 artifacts will be contain an additional change in pom.xml test dependencies.

I would prefer that 1.0.0 jars are built using the git commit we are voting on. I would even think this is a requirement. If you use the correct commit then this merge will not affect the 1.0.0 jar release.

I use the voted on git commit in all my releases.

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.

2 participants