gradle
gradle copied to clipboard
Allow overriding the default Gradle library repository through a new Gradle property
Allow overriding the default Gradle library repository through a new Gradle property called org.gradle.internal.gradle.libs.repo.override (previously overriding was only possible through the GRADLE_LIBS_REPO_OVERRIDE environment variable).
The Gradle property can be more useful because it can be defined in the build itself (either through gradle.properties or programmatically)
Context
This can be useful for every project which needs to override the default Gradle library repository (typical in corporate settings, behind a firm firewall). While the environment variable works it is not convenient: each developer has to define the environment variable manually in his/her dev environment. The Gradle property is more practical because it can be defined inside the build and kept together with the rest of the build configuration in the source repository.
Contributor Checklist
- [X] Review Contribution Guidelines
- [X] Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
- [X] 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.
- [X] Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
- [x] Provide integration tests (under
<subproject>/src/integTest) to verify changes from a user perspective - [x] Provide unit tests (under
<subproject>/src/test) to verify logic - [X] Update User Guide, DSL Reference, and Javadoc for public-facing changes
- [ ] Ensure that tests pass sanity check:
./gradlew sanityCheck - [x] Ensure that tests pass locally:
./gradlew <changed-subproject>:quickTest
Reviewing cheatsheet
Before merging the PR, comments starting with
- ❌ ❓must be fixed
- 🤔 💅 should be fixed
- 💭 may be fixed
- 🎉 celebrate happy things
Thank you for your interest in Gradle.
The implementation does not match the proposed solution on the related issue.
- https://github.com/gradle/gradle/issues/25840
Please rework your implementation to match the design.
The solution should use a Gradle property instead of a system property and provide tests to verify the functionality.
@cobexer thanks for your comment.
I have updated the PR as requested. It is now looking for a Gradle property and I have copied 3 of the existing tests (that were using the env var for override) and modified them to use the new property.
Please check.
Thank you for your contribution!
This PR has a valid DCO and tests. The relevant team for this area will confirm the design of the implementation choices.
Change Summary
This PR is 89.82% 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 | 2 | 0.88% | 1 | 0.44% | 1 | 16.67% |
| extensibility | 0 | 0% | 0 | 0% | 0 | 0% |
| gradle_enterprise | 0 | 0% | 0 | 0% | 0 | 0% |
| ide | 201 | 88.94% | 22 | 9.73% | 5 | 83.33% |
| 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% |
This PR in its current state would already be really helpful for people sitting behind corporate firewalls and proxies.
However I agree with @SidB3's comment on #25840: in some cases users need full control over how that maven repository (used by DefaultGradleApiSourcesResolver) gets configured (for example to setup the authentication correctly).
So the support for that use case could be added (either as part of this PR or in a followup PR) with some DSL additions that would allow users to provide a custom Action<? super MavenArtifactRepository> which then could be used during DefaultGradleApiSourcesResolver creation.
That's why I did some refactoring on DefaultGradleApiSourcesResolver to allow that kind of customization to happen.
But I didn't include any DSL changes.
My questions to the reviewer(s) from the Gradle Team are: do you agree with this train of thought? Would you be supportive to have the custom-maven-config-action-DSL?
My only question would be in regards to portability. Will this only work if all users and CI use the same OS type? Does it support ~ or other placeholders for user.home?
Would it make sense to make ok aware properties org.gradle.internal.gradle.libs.repo.override.windows/org.gradle.internal.gradle.libs.repo.override.linux/... ?
What takes precedence, env or the new property?
@leonard84 thanks for taking a look at this. Let me try to answer your questions out of order:
-
Currently the new gradle property takes precedence over the environment variable. But I can reverse it if needed.
-
Not sure how dependent this is on OS type. The aim of this PR is to make it possible to override the repo URL more easily (so it can be done as part of the project setup instead of asking every developer to manually set an environment variable). So I would assume the URL wouldn't be OS dependent. At least in our organization it would be the same regardless of the client OS. However, because this is normal gradle property, it can be redefined/overdefined in various places (for example in command line or in a gradle.properties file in the GRADLE_USER_HOME dir). So I personally don't think OS dependent properties would add much.
-
Regarding the placeholders. No explicit support was added for anything like that. Note that the existing env variable doesn't support those either. Also my assumption was that these would still mostly be used with remote URL-s (http addresses) not with file url-s.
Thanks @akiraly for some reason I was assuming that it was referring to a file system location, if it is mainly intended for web resources, then I agree that my points are mostly irrelevant.
@akiraly thanks for your efforts with this PR. We are currently not able to to merge this due to ongoing discussions on how to address the underlying issue.
We've picked up the ball and resumed the conversation on this pull request. We still have a few things to sort out, but my understanding is that we actually want to get rid of the property altogether. The Gradle libs URLs was a workaround because the localGroovy dependency is handled in Gradle in a special way. We aim to simplify Gradle and making localGroovy resolvable/configurable via dependency management.
@donat thanks, that would be the cleanest. I assume that way the global repository configuration (url, authentication, etc...) would apply to these resolutions as well?
I assume that way the global repository configuration (url, authentication, etc...) would apply to these resolutions as well?
Exactly.
To be explicit, we're not planning to accept this PR. Instead, we'll aim for the improving the dependency resolution for localGroovy(). Let's continue the conversation at https://github.com/gradle/gradle/issues/29049.