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 issue#22850: the fast fail mechanism replaces the stack overflow #23097

Closed
wants to merge 2 commits into from

Conversation

bodhili
Copy link
Contributor

@bodhili bodhili commented Dec 10, 2022

Fixes #22850

Context

  • To be consistent, Gradle should throw an exception and describe the cycle, similar to when you do a project assemble with cyclically-dependent projects
    FAILURE: Build failed with an exception
    What went wrong:
Circular dependency between the following tasks:
 org.example:registry:1.0-SNAPSHOT
  \--- org.example:discovery:1.0-SNAPSHOT
       \--- org.example:registry:1.0-SNAPSHOT (*)

  (*) - details omitted (listed previously)
  • Clear prompt information can help users solve problems more quickly

Contributor Checklist

  • Review Contribution Guidelines
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
  • Provide unit tests (under <subproject>/src/test) to verify logic
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes
  • Ensure that tests pass sanity check: ./gradlew sanityCheck
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest

Gradle Core Team Checklist

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation

@bodhili bodhili requested a review from a team as a code owner December 10, 2022 12:53
@bot-gradle bot-gradle added the from:contributor PR by an external contributor label Dec 10, 2022
@bodhili bodhili force-pushed the fix_issue_22850 branch 2 times, most recently from e2159a3 to 97abdd0 Compare December 10, 2022 13:11
Signed-off-by: bodhili <present.h7s@gmail.com>

fix issue#22850

Signed-off-by: bodhili <present.h7s@gmail.com>
@ov7a
Copy link
Member

ov7a commented Oct 24, 2023

Sorry for the late reply.

This PR does not fix the original issue with the StackOverflow error. The dependency graph can contain cycles. So it's wrong to assume that any cycle should trigger an error.

Also, there are no tests covering this.

If you want to continue to work on this, the reproducer in the issue should handle the cycle correctly without failing.
If you don't want to work on this, this PR will be closed (or you can do it).

@ov7a ov7a added the pending:feedback Indicates that changes or additional info are required, and the issue will be closed without them label Oct 24, 2023
@gitstream-cm
Copy link

gitstream-cm bot commented Oct 24, 2023

This PR appears to be lacking tests. Consider adding tests to cover the new functionality.

@gitstream-cm
Copy link

gitstream-cm bot commented Oct 24, 2023

Change Summary

This PR is 98.08% new code.
Platform Added Lines % of Total Line Changes Deleted Lines % of Total Line Changes Files Changed % of Total Files Changed
bt_ge_build_cache 0 0% 0 0% 0 0%
build_infrastructure 0 0% 0 0% 0 0%
core_configuration 0 0% 0 0% 0 0%
core_execution 0 0% 0 0% 0 0%
core_runtime 0 0% 0 0% 0 0%
documentation 0 0% 0 0% 0 0%
extensibility 0 0% 0 0% 0 0%
gradle_enterprise 0 0% 0 0% 0 0%
ide 0 0% 0 0% 0 0%
jvm 0 0% 0 0% 0 0%
kotlin_dsl 0 0% 0 0% 0 0%
release_coordination 0 0% 0 0% 0 0%
software 0 0% 0 0% 0 0%

@bodhili bodhili closed this Oct 25, 2023
@bodhili bodhili deleted the fix_issue_22850 branch December 5, 2023 09:07
@ov7a ov7a removed the pending:feedback Indicates that changes or additional info are required, and the issue will be closed without them label Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from:contributor PR by an external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stack overflow when traversing transitive modules with cyclic relationship
3 participants