Use 7999 as Bitbucket DC default port - #4414
Closed
pstreef wants to merge 1 commit into
Closed
Conversation
| ssh://scm.company.com:22/stash/org/repo, scm.company.com:22/stash, Bitbucket, org/repo, org, repo | ||
| ssh://scm.company.com:7999/stash/org/repo, scm.company.com/stash, Bitbucket, org/repo, org, repo | ||
| ssh://scm.company.com:7999/stash/org/repo, scm.company.com:7999/stash, Bitbucket, org/repo, org, repo | ||
|
|
Contributor
There was a problem hiding this comment.
aren't these the same inputs? expected different results?
Contributor
Author
There was a problem hiding this comment.
no, the origin is both input and expected result
3 tasks
Contributor
Author
timtebeek
added a commit
that referenced
this pull request
Aug 10, 2026
…ildTool` (#8433) * Fall back to `distributionUrl` when the wrapper recipes find no `BuildTool` `UpdateGradleWrapper` and `UpdateMavenWrapper` gated on a `BuildTool` marker of the matching type being present on the wrapper properties file. Only the OpenRewrite build-tool plugins attach that marker, so any other parser pipeline produced a silent no-op with no way to tell "up to date" apart from "provenance missing". It has broken three times in the Moderne CLI alone (moderneinc/moderne-cli#1005, #4414, #4472). The marker is also a worse source of truth than the file itself: the plugins set it from the tool that happened to run the build, while `distributionUrl` is the wrapper's version by definition, and both recipes already parse it. Both scanners now prefer the marker and fall back to the version in `distributionUrl`, bailing out as before when neither yields one. The marker is only updated when one was present to begin with. The `getVisitor()` guard that skipped any file carrying a non-matching `BuildTool` now only rejects non-wrapper paths, so a `gradlew`/`mvnw` stamped with another build tool is still updated once its properties file was accepted. * Make `AddDevelocityGradlePlugin` fall back to the wrapper's `distributionUrl` Both `visitCompilationUnit` overloads bailed when the script they visited carried no `BuildTool` marker of type `Gradle`, the same invisible provenance contract the wrapper recipes just shed. The marker feeds a single decision — whether the build is Gradle 6 or newer, and so whether the plugin belongs in the settings script or the root build script — which `gradle-wrapper.properties` answers by definition. The recipe is now a `ScanningRecipe`. The scanner records each wrapper's version keyed by the directory that wrapper governs, and scripts resolve against the nearest enclosing wrapper, so a nested build with its own wrapper is never judged by the root's version. The marker still wins when present; with neither the recipe bails as before. The duplicated guard is now one helper so the Groovy and Kotlin paths cannot drift. `versionFromDistributionUrl` moves out of `UpdateGradleWrapper` into `org.openrewrite.gradle.internal.GradleWrapperProperties` so both recipes share one copy. This does not relax the `GradleSettings`/`GradleProject` requirement that follows, so it helps LSTs carrying Gradle project provenance but no `BuildTool`, not ones missing both. * Prefer the strict distribution URL parser in `GradleWrapper.create(URI)` The `wrapperUri` option's version was read with `GRADLE_VERSION_PATTERN`, whose `gradle-([0-9.]+)` stops at the first hyphen, so a pre-release distribution such as `gradle-8.5-rc-1-bin.zip` yielded `8.5`. That version is both written into the `gradleVersion` DSL, where it disagreed with the `distributionUrl` beside it, and used to pick the `gradlew` script template. Try `GradleWrapperProperties.versionFromDistributionUrl` first and fall back to the loose pattern only when it finds nothing, so URLs it does not recognize behave exactly as before. `versions.csv` carries script templates for pre-release versions, so `findNearest` now resolves an RC to that RC's own scripts rather than the release's. * Address review feedback on the wrapper `distributionUrl` fallback - Move `versionFromDistributionUrl` from the new `internal.GradleWrapperProperties` onto `util.GradleWrapper`, beside the `GRADLE_VERSION_PATTERN` it complements, and drop the internal class. This makes the method public API rather than internal, so the two distribution URL parsers now sit together where the next reader will find them. - Revert the `getVisitor()` guards in both wrapper recipes to their original form and delete `isWrapperPath`. The guard only ever relaxed a case where the properties file carries a matching `BuildTool` while a sibling wrapper file carries a different one, an LST shape that does not occur, so the tests covering it go too. - Express `AddDevelocityGradlePlugin`'s version resolution as `filter`/`map`/`orElseGet`, matching the scanners in the wrapper recipes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's changed?
Assume 7999 as a default SSH port for Bitbucket and strip it off. This way we can match the origin to the https origin (without port)
What's your motivation?
Currently when using the default port, we need to register 2 origins to make them both work. This can cause conflicts when switching between protocols.
Anyone you would like to review specifically?
@bryceatmoderne @kmccarp
Have you considered any alternatives or workarounds?
Not doing this
Checklist