Skip to content

Handle composite builds #122

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

Closed
wants to merge 1 commit into from

Conversation

Arthurm1
Copy link
Contributor

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
Copy link
Member

jdneo commented Dec 21, 2023

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
Copy link
Contributor Author

👍. 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
Copy link
Contributor Author

rebased

@Arthurm1
Copy link
Contributor Author

Arthurm1 commented Apr 1, 2024

rebased

@jdneo
Copy link
Member

jdneo commented Apr 2, 2024

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
Copy link
Contributor Author

Arthurm1 commented Apr 2, 2024

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.

@jdneo jdneo marked this pull request as draft April 24, 2024 04:06
@Tanish-Ranjan
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor

Okay, I'll start from there.

@Arthurm1
Copy link
Contributor Author

Arthurm1 commented Jun 7, 2024

@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
Copy link
Member

jdneo commented Jun 11, 2024

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

@Arthurm1
Copy link
Contributor Author

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
Copy link
Member

jdneo commented Jun 11, 2024

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
Copy link
Contributor

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
Copy link
Member

jdneo commented Jun 19, 2024

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 #122 (comment)? Since we still need to get the normal dependencies.

@Arthurm1
Copy link
Contributor Author

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.

@jdneo
Copy link
Member

jdneo commented Jul 5, 2024

It's resolved by #154.

Thank you @Arthurm1 for all the effort and help you provided for this issue.

@jdneo jdneo closed this Jul 5, 2024
@Arthurm1 Arthurm1 deleted the composite_build branch July 9, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants