Support dependencies resolved outside of a Project context #24
Conversation
bigdaz
force-pushed
the
dd/settings-dependencies
branch
2 times, most recently
from
b6eae8b
to
d6093ad
Compare
| static String getTestGradleVersion() { | ||
| System.getProperty("testGradleVersion", GradleVersion.current().version) | ||
| } | ||
|
|
||
| static settingsPluginsAreSupported() { | ||
| return GradleVersion.version(testGradleVersion) >= GradleVersion.version("6.0") |
There was a problem hiding this comment.
I recommend consistent usage of return types and the return keyword. Either use them or don't.
There was a problem hiding this comment.
Roger that. Good feedback.
| manifestHasPlugin2("project :included-plugin:a") | ||
| } | ||
|
|
||
| private void createMultiProjectBuildWithPlugins(TestFile rootDir) { |
There was a problem hiding this comment.
The formatting for these multiline strings is very strange. 🤔
| if (expected.package_url != null) { | ||
| assert actual.package_url == expected.package_url | ||
| } |
There was a problem hiding this comment.
Might you want to assert that it isn't present, rather than not asserting anything at all?
There was a problem hiding this comment.
No, the package_url will be present, but this test isn't asserting anything about it.
There was a problem hiding this comment.
Might be worth continuing to assert that it is set to a non-empty value, beginning with pkg:, even if you're not asserting anything else about it?
| // TODO: See the Gradle Enterprise Build Scan Plugin: `ConfigurationResolutionCapturer_5_0` | ||
|
|
||
| if (projectIdentityPath == null && rootComponent.dependencies.isEmpty()) { | ||
| // Not a project configuration, and no dependencies to extract: can safely ignore |
There was a problem hiding this comment.
Why is a ResolveConfigurationDependenciesBuildOperationType event issued in this case then?
There was a problem hiding this comment.
This happens when a configuration is resolved that doesn't have any dependencies configured. I'm pretty sure the main case this happens is when resolving a classpath configuration when no plugins (or other classpath dependencies) have been declared.
| private fun manifestName(config: ResolvedConfiguration): String { | ||
| if (config.identityPath != null) { | ||
| return "project ${config.identityPath}" | ||
| } | ||
| return "build ${config.buildPath}" | ||
| } |
There was a problem hiding this comment.
This is an interesting way of differentiating names. Given these names may be rendered in the GitHub UI, might it be good to capitalize the first letters of Project and Build?
Settings script dependencies are not currently captured, whether added by a `buildscript` block or via `plugins`.
Previously, we were ignoring any dependency resolution that occurred outside the context of a Gradle Project. This meant that any dependencies resolved in the context of a Settings script were not being captured. By tracking the 'buildPath' and the 'projectPath' separately, we can now collect all dependency resolution. Due to a limitation in the events being processed, all non-project resolution will be allocated to a single manifest for the entire build process. It may be possible later to make this more precise, and assign resolution to a specific build process.
- Only test Settings plugins on Gradle >= 6.0. Earlier versions of Gradle did not support the `plugins` block within a Settings script. - Do not specify a version for subproject plugin.
- Avoid using implicit return in Groovy - Reformat Groovy code
bigdaz
force-pushed
the
dd/settings-dependencies
branch
from
d6093ad
to
79aa996
Compare
Previously, we were ignoring any dependency resolution that occurred outside the
context of a Gradle Project. This meant that any dependencies resolved in the
context of a Settings script were not being captured.
By tracking the 'buildPath' and the 'projectPath' separately, we can now collect
all dependency resolution.
Due to a limitation in the events being processed, all non-project resolution will
be allocated to a single manifest for the entire build process. It may be possible
later to make this more precise, and assign resolution to a specific build process.