Skip to content
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

fix - Use project dir instead of project path #171

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Conversation

jdneo
Copy link
Member

@jdneo jdneo commented Jul 10, 2024

  • In composite build scenario, the project path is not based on the build tree root. Besides, the .getBuildTreePath() is not available before Gradle 8.3, we use the project dir to identify a Gradle project instead. The project dir is used in dependency resolution.

Fix the point 2 in #154 (comment)

- In composite build scenario, the project path is not based on the build
  tree root, since the .getBuildTreePath() is not available before GradleApiConnectorTest
  8.3, we use the project dir to identify a Gradle project instead. The
  project dir is used in dependency resolution.
@jdneo jdneo added this to the 0.3.0 milestone Jul 10, 2024
@jdneo jdneo added the bug Something isn't working label Jul 10, 2024
@jdneo
Copy link
Member Author

jdneo commented Jul 10, 2024

// cc @Tanish-Ranjan, @Arthurm1

@Arthurm1
Copy link
Contributor

So project.getPath() is not unique when using included builds? We might need to also look at what goes into gradleSourceSet.setDisplayName. It might be better to do in the action after full retrieval. Maybe set display name to full path and then pare it down to differences in the build action?
I did something similar here. I can look to reproduce in another PR?

@jdneo
Copy link
Member Author

jdneo commented Jul 10, 2024

So project.getPath() is not unique when using included builds?

No they are not. In the sample project that I referenced, both project path is :.

It might be better to do in the action after full retrieval. Maybe set display name to full path and then pare it down to differences in the build action?
I did something similar here. I can look to reproduce in another PR?

Sure, feel free to address it in another PR if you want :)

@Tanish-Ranjan
Copy link
Contributor

So project.getPath() is not unique when using included builds?

It won't. Any information that comes from the project will be stripped down to the rootProject. SourceSetsModelBuilder considers the project it receives in the buildAll parameter, to be the root project. This is why we had to resort to BuildActions in #154, as it contains the complete information of the project hierarchy. It queries for each sub project in the build. This is where the SourceSetsModelBuilder starts building the requested model unaware of the entire project hierarchy and thinks the sub project we are querying is actually the root project.

Now because of the way model builders work we cannot rely on hierarchical information from it, such as project path.

Hope I explained it well. Please ask any questions if you have queries.

@jdneo
Copy link
Member Author

jdneo commented Jul 11, 2024

Another solution is to get the build tree path by casting the Project to DefaultProject, and call getIdentityPath(). But that is an internal API, and since the project dir works fine for now, we can use project dir currently.

Just to share this information in case we need to revisit this problem in the future.

@jdneo jdneo merged commit 5f49a66 into develop Jul 11, 2024
4 checks passed
@jdneo jdneo deleted the cs/project-path-fix branch July 11, 2024 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants