microsoft / build-server-for-gradle

An implementation of the Build Server Protocol for Gradle
MIT License
45 stars 7 forks source link

Handle composite builds #122

Open Arthurm1 opened 6 months ago

Arthurm1 commented 6 months ago

This PR extracts source sets for composite builds - previously, included builds would be ignored.

I've had to get rid of the unique display name code and go with project [sourceset] for all target display names. It just gets far too complicated for little reward when composite builds are added into the mix.

Composite builds have to be handled in the GradleAPIConnector as each included build requires a new ProjectConnection

jdneo commented 6 months ago

I've had to get rid of the unique display name code and go with project [sourceset] for all target display names. It just gets far too complicated for little reward when composite builds are added into the mix.

👍. If it's possible, would you mind separate the display name handling change to another PR?

Arthurm1 commented 6 months ago

👍. If it's possible, would you mind separate the display name handling change to another PR?

I've submitted another PR. When that's merged I'll rebase this PR onto it.

Arthurm1 commented 6 months ago

rebased

Arthurm1 commented 3 months ago

rebased

jdneo commented 3 months ago

I tried this change with a sample in Gradle document site: https://docs.gradle.org/current/samples/sample_composite_builds_basics.html, but it's not working:

Caused by: org.gradle.internal.resolve.ModuleVersionNotFoundException: Could not find org.sample:number-utils:1.0.

It happens when calling sourceSet.getCompileClasspath().getFiles() for the project :app

Maybe it's another issue about how we fetch the classpath, not related with how we handling the include build.

Arthurm1 commented 3 months ago

I may have to rethink how this works. I hadn't taken into account dependency substitutions.

Even though it looks like a maven dependency, implementation 'org.sample:number-utils:1.0' in project app actually refers to the project number-utils in the other included build.

Tanish-Ranjan commented 1 month ago

Hi @Arthurm1 I'd be happy to take a look at this PR and potentially contribute. Let me know if there's anything I can do to help move this forward.

Arthurm1 commented 1 month ago

@Tanish-Ranjan You're welcome to take over. The main thing is that dependencies into included builds aren't handled as stated here

Tanish-Ranjan commented 1 month ago

Okay, I'll start from there.

Arthurm1 commented 3 weeks ago

@Tanish-Ranjan Hi. Are you working on this? No rush - I just noticed that it's a bit tricky to create the project dependencies

jdneo commented 3 weeks ago

Hi @Arthurm1, what kind of issue are you facing?

Arthurm1 commented 3 weeks ago

what kind of issue are you facing?

No issue really - I just noticed that we'll have to move the project dependency collection out of SourceSetsModelBuilder and into GradleAPIConnector. Because the included builds have to be retrieved separately on different ProjectConnection.

The archive output jars will also have to be returned by the SourceSetsModelBuilder.

I was going to have a go but wasn't sure if @Tanish-Ranjan was working on it.

jdneo commented 3 weeks ago

I see. Just to heads up, @Tanish-Ranjan is a participant for the event GSoC (Google Summer of Code). He will work on this open source project during the summer. The composite build is one of the tasks he would like to take.

@Tanish-Ranjan, please let us know if you would like to keep doing this task. If yes, @Arthurm1 is a very experienced and active contributor to this project, and I believe he can definitely give you some guidance for this issue.

To me, I hope we can address this issue ASAP since this will increase the Gradle project import success rate in our VS Code extensions. My plan is to fix it in June.

Tanish-Ranjan commented 3 weeks ago

Yes @jdneo, I'm happy to take on this task for fixing composite builds. I've been preparing myself by researching composite builds and familiarizing myself with the relevant code. I'd also like to reach out to @Arthurm1 for his guidance. Given his experience with the project, I am confident we would be able to resolve this issue soon.

jdneo commented 2 weeks ago

No issue really - I just noticed that we'll have to move the project dependency collection out of SourceSetsModelBuilder and into GradleAPIConnector. Because the included builds have to be retrieved separately on different ProjectConnection.

The archive output jars will also have to be returned by the SourceSetsModelBuilder.

May I ask how to handle the problem mentioned in https://github.com/microsoft/build-server-for-gradle/pull/122#issuecomment-2031099285? Since we still need to get the normal dependencies.

Arthurm1 commented 2 weeks ago

May I ask how to handle the problem mentioned in #122 (comment)? Since we still need to get the normal dependencies.

If the archive dirs are returned in the GradleSourceSet (currently they are not) then in GradleAPIConnector we can check whether they feature in any project's compile classpath. If they do then we can add a project dependency between those 2 projects. The includedBuilds also need to be returned in GradleSourceSets for each projectdir so GradleAPIConnector can retrieve source sets from all the included builds before the project dependencies can be worked out. I described this to Tanish on Slack and do have a PR showing it works but I was leaving it to Tanish.