microsoft / build-server-for-gradle

An implementation of the Build Server Protocol for Gradle
MIT License
51 stars 8 forks source link

Composite Builds using Build Actions #154

Closed Tanish-Ranjan closed 2 months ago

Tanish-Ranjan commented 3 months ago

This PR continues the work done by @Arthurm1 for composite-builds on #122, bringing in support for included builds, including dependency substitution using build actions.

Tanish-Ranjan commented 3 months ago

@microsoft-github-policy-service agree

Arthurm1 commented 3 months ago

@Tanish-Ranjan Thanks for the PR.

I see you've used a Gradle BuildAction. Was that so you don't have to query the source sets from GradleAPIConnector for each included build?

If that's the best route then you probably don't need the GradleIncludedBuild class as the model never returns any.

I think you'll still need to work out the project dependencies across included builds. Maybe you can put that in the BuildAction as well. I've slightly altered your new test to show what I mean. This dependency check currently fails...

  @Test
  void testCompositeBuild2() {
    File projectDir = projectPath.resolve("composite-build-2").toFile();
    PreferenceManager preferenceManager = new PreferenceManager();
    preferenceManager.setPreferences(new Preferences());
    GradleApiConnector connector = new GradleApiConnector(preferenceManager);
    GradleSourceSets gradleSourceSets = connector.getGradleSourceSets(projectDir.toURI(), null);
    assertEquals(6, gradleSourceSets.getGradleSourceSets().size());
    GradleSourceSet mainApp = findSourceSet(gradleSourceSets, "app [main]");
    GradleSourceSet mainStringUtils = findSourceSet(gradleSourceSets, "string-utils [main]");
    GradleSourceSet mainNumberUtils = findSourceSet(gradleSourceSets, "number-utils [test]");
    assertHasBuildTargetDependency(mainApp, mainStringUtils);
    assertHasBuildTargetDependency(mainApp, mainNumberUtils);
  }
Tanish-Ranjan commented 3 months ago

I see you've used a Gradle BuildAction. Was that so you don't have to query the source sets from GradleAPIConnector for each included build?

That's correct @Arthurm1. Gradle automatically manages substitution of dependencies. It didn't work previously in commit f2097d0 because Gradle was getting scoped to the project, loosing reference of the overall build and hence was not able to perform dependency substitutions from the included builds. With the help of Gradle TAPI's BuildAction we are maintaining the build scope while we fetch the GradleSourceSets.

Tanish-Ranjan commented 3 months ago

I think you'll still need to work out the project dependencies across included builds.

Thanks for pointing it out. I'll look into it.

Tanish-Ranjan commented 2 months ago

Big thanks to @Arthurm1 for the initial insights into the tooling API! Your guidance gave this PR a significant head start. Additionally, a huge thanks to all the gsoc mentors (@jdneo @donat @hegyibalint @reinsch82) who provided support throughout this process. Your help was invaluable in getting this completed so quickly.

jdneo commented 2 months ago

Ok, so after some investigation, I found that the current solution has some problems:

  1. See #169 and #170
  2. For the sample project provided in https://github.com/microsoft/vscode-gradle/issues/1435, all the sourceset from two subproject will have the same project path :, this is actually not correct, we need to use the build tree path to differentiate them. Otherwise, the build target dependency resolution in BuildTargetManager might be wrong.