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 StackOverflowError in case of circular import deps #4284

Merged

Conversation

ZhyliaievD
Copy link
Contributor

@ZhyliaievD ZhyliaievD commented Jun 25, 2024

Check if import dependency was already resolved before

What's changed?

Add a check if import dependency was already resolved before

What's your motivation?

To avoid StackOverflowError in case of circular import deps

Fixes #4093

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

Check if import dependency was already resolved before
@timtebeek
Copy link
Contributor

Thanks for getting this started @ZhyliaievD ! I've updated your PR a bit to add additional assertions to your test that we indeed get merged. Would you mind double checking that you indeed expect junit present on both the parent and child as the only managed dependency versions there?

@ZhyliaievD
Copy link
Contributor Author

ZhyliaievD commented Jul 1, 2024

Hi @timtebeek
Thans for the adjustments(and sorry for long response - just got back home)
Regarding expected managed dependencies - correct, I only expect junit in test case (Reference from the doc about import scope: It indicates the dependency is to be replaced with the effective list of dependencies in the specified POM's)

@timtebeek timtebeek merged commit ff02bf5 into openrewrite:main Jul 1, 2024
2 checks passed
@ZhyliaievD ZhyliaievD deleted the bug/4093-StackOverflow-import-deps-fix branch July 1, 2024 11:31
adastraperangusta pushed a commit to adastraperangusta/rewrite that referenced this pull request Jul 2, 2024
)

* Fix StackOverflowError in case of circular import deps

Check if import dependency was already resolved before

* Format test, remove unnecessary paths and add issue link

* Replace Streams with for loop as per convention

To limit object allocations

* Swap argument order for consistency

* Verify merged managed dependency version

---------

Co-authored-by: Daniil Zhyliaiev <dzhyliaiev@playtika.com>
Co-authored-by: Tim te Beek <tim@moderne.io>
timtebeek added a commit that referenced this pull request Jul 2, 2024
…ile (#4297)

* avoid adding non-existent query as key

in case the jsonpath is a query which yelds no result,
avoid merging the yaml at end of file with the query as a key

* Fix StackOverflowError in case of circular import deps (#4284)

* Fix StackOverflowError in case of circular import deps

Check if import dependency was already resolved before

* Format test, remove unnecessary paths and add issue link

* Replace Streams with for loop as per convention

To limit object allocations

* Swap argument order for consistency

* Verify merged managed dependency version

---------

Co-authored-by: Daniil Zhyliaiev <dzhyliaiev@playtika.com>
Co-authored-by: Tim te Beek <tim@moderne.io>

* Give EffectiveManagedDependencies a unique display name

* Fix model updating to properly work with transitive dependencies.

* Fix test expectation to not break when new versions of flyway-core are released

* Deterministic ordering of new constraints

* Further enhancements to UpdateGradleWrapper's ability to operate in contexts where services.gradle.org is unavailable

* Also enforce that version numbers are literal, rather than dynamic selectors, when services.gradle.org cannot be reached

* Fix typo in comment and apply formatter

---------

Co-authored-by: Stef <stef.dev.49@gmail.com>
Co-authored-by: Daniil Zhyliaiev <yangroang@gmail.com>
Co-authored-by: Daniil Zhyliaiev <dzhyliaiev@playtika.com>
Co-authored-by: Tim te Beek <tim@moderne.io>
Co-authored-by: Sam Snyder <sam@moderne.io>
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
Archived in project
Development

Successfully merging this pull request may close these issues.

StackOverflowError on attempt to run against import dependencies
2 participants