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: get lenient config if only some deps fail #242

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

magdziarek
Copy link

  • Tests written and linted
  • Documentation written
  • Commit history is tidy

What this does

When a configuration cannot be fully resolved we don't resolve any of its dependencies, often returning an empty graph. It's better to return a partial result than none, like Gradle would. For this, instead of only using fully Resolved Configuration, in case of an error, we can log the failed dependencies and use Lenient Configuration instead

@@ -336,22 +336,28 @@ allprojects { Project currProj ->
def resolvableConfigs = findProjectConfigs(proj, confNameFilter, confAttrSpec)
def resolvedConfigs = []
resolvableConfigs.each({ config ->
def resConf = config.resolvedConfiguration
ResolvedConfiguration resConf = config.getResolvedConfiguration()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to just use lenient configuration everywhere?

Copy link
Author

Choose a reason for hiding this comment

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

Good point - double checking now but in the end it results in the same so I don't see why not

Copy link
Author

Choose a reason for hiding this comment

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

Tests fail for Kotlin if it's only lenient config...We can look into next? Can we merge the way it is since it's a fix for a bug?

Copy link
Contributor

Choose a reason for hiding this comment

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

From my point of view sure, but maybe leave a comment for the next person trying to simplify this? :)

Copy link
Author

Choose a reason for hiding this comment

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

OK, will try Stepsize for this! :)

lib/init.gradle Outdated Show resolved Hide resolved
@@ -455,20 +461,26 @@ allprojects { Project currProj ->
def resolvableConfigs = findProjectConfigs(proj, confNameFilter, confAttrSpec)
def resolvedConfigs = []
resolvableConfigs.each({ config ->
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be extracted into a common function that returns an array of resolved configurations?

@magdziarek magdziarek force-pushed the fix/get-lenient-config-if-some-deps-fail branch from 36b9d4e to 921c3c1 Compare November 18, 2022 16:56
@magdziarek magdziarek merged commit 1011de9 into master Nov 18, 2022
@magdziarek magdziarek deleted the fix/get-lenient-config-if-some-deps-fail branch November 18, 2022 17:28
@snyksec
Copy link

snyksec commented Nov 18, 2022

🎉 This PR is included in version 3.24.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@stepsize
Copy link

stepsize bot commented Nov 18, 2022

This PR modifies files linked to issues tracked in Stepsize. You might want to review their status, priority, and scope.

Use Lenient Configuration by default
  • lib/init.gradle
extract to a common function
  • lib/init.gradle

 Mention [stepsize] in a comment if you'd like to report some technical debt. See examples here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants