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

UpdateGradleWrapper fails for a custom wrapperUri with placeholders #4816

Open
bmuschko opened this issue Dec 25, 2024 · 3 comments
Open

UpdateGradleWrapper fails for a custom wrapperUri with placeholders #4816

bmuschko opened this issue Dec 25, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@bmuschko
Copy link
Contributor

What version of OpenRewrite are you using?

  • OpenRewrite v8.41.0

How are you running OpenRewrite?

Moderne.io

Is your project a single module or a multi-module project? Single

Can you share your configuration so that we can rule out any configuration issues?

type: specs.openrewrite.org/v1beta/recipe
...
recipeList:
  - org.openrewrite.gradle.UpdateGradleWrapper:
       version: 8.9
       distribution: bin
       wrapperUri: https://services.gradle.org/distributions/gradle-${version}-${distribution}.zip

What is the smallest, simplest way to reproduce the problem?

Run the org.openrewrite.gradle.UpdateGradleWrapper recipe with the declarative YAML format.

What did you see instead?

An error with a stack trace when executing the recipe.

What is the full stack trace of any errors you encountered?

java.lang.IllegalArgumentException: Illegal character in path at index 50: https://services.gradle.org/distributions/gradle-${version}-${distribution}.zip
  java.base/java.net.URI.create(URI.java:906)
  org.openrewrite.gradle.UpdateGradleWrapper.getGradleWrapper(UpdateGradleWrapper.java:139)
  ...

Are you interested in [contributing a fix to OpenRewrite]

Yes

@bmuschko bmuschko added the bug Something isn't working label Dec 25, 2024
@bmuschko bmuschko changed the title UpdateGradleWrapper fails for a wrapperUri with placeholders UpdateGradleWrapper fails for a custom wrapperUri with placeholders Dec 25, 2024
@shanman190
Copy link
Contributor

shanman190 commented Dec 25, 2024

Welcome @bmuschko!

I think this issue may be one of the documentation being out of sync with expectations, but open to feedback as well.

During the work on #4491, I had mentioned there that interpolating the exact version and distribution into the wrapperUri felt kinda weird given the only way to define the wrapperUri is directly and in the most general case, it's being used for corporate environments to swap to a privately built Gradle wrapper. So in that PR, the parameter replacements were removed. It seems like the @Option values though didn't get updated which then transitively impact the documentation and Moderne SaaS.

I think for this, the best course may be to update the documentation to remove the no longer valid information. What do you think?

@bmuschko
Copy link
Contributor Author

Thanks for clarifying!

There's another use case for this I'd like to mention. A company hosts a Gradle Wrapper binary on an internal repository so that consumers don't need to reach out to the public Wrapper service endpoint (as it would be blocked due to security measures).

For now, I think the best course of action would be to remove the version and distribution options to remove the confusion for users. You'd also have to rectify the description of the wrapperUri option.

@shanman190
Copy link
Contributor

shanman190 commented Dec 25, 2024

Yep! And that's another perfectly fair use case.

I think with the current implementation, we'd want to just rectify the description on wrapperUri only as when you provide just the version and distribution, we first try to make an API call to services.gradle.org and use the Node SemVer (see https://docs.openrewrite.org/reference/dependency-version-selectors) semantics for dynamically selecting sersions, but if this fails and the version is exact then we'll try to perform a replacement inline if possible.

With this default implementation, this then allows the recipe to have a default of latest.release (and bin) or for the user to specify any of the supported version ranges to automatically discover the current official version when the API call is allowed to be performed.

@timtebeek timtebeek moved this to Backlog in OpenRewrite Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

2 participants