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

Insert all module transitive module dependencies #SCL-21158 #59

Merged
merged 2 commits into from
Oct 24, 2023

Conversation

azdrojowa123
Copy link
Contributor

@azdrojowa123 azdrojowa123 commented May 9, 2023

No description provided.

@azdrojowa123 azdrojowa123 changed the title Insert all transitive dependant modules #SCL-21158 Insert all module transitive module dependencies #SCL-21158 May 9, 2023
@azdrojowa123 azdrojowa123 force-pushed the insert_all_module_transitive_dependencies branch from 2500b00 to 32257dd Compare May 30, 2023 11:10
@azdrojowa123 azdrojowa123 force-pushed the insert_all_module_transitive_dependencies branch 8 times, most recently from d3849bf to dffb000 Compare October 12, 2023 10:13
val sourceConfigurationsNames = sourceConfigurations.map(_.name)
val transformedConfigurations = mapConfigurations(configurations).map { configuration =>
// additional mapping is required in case of situation where project1 dependsOn project2 in two custom configurations -> the test one
// and the one containing sources. Without this mapping custom test configuration will be transformed to TEST in #mapConfigurations and in the scala plugin, scope
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure that this special case mentioned in the comment is covered with tests.
ATM it might be easier to use sbt import integration tests in Scala Plugin

* It needs to be done because `Classpaths#interSort` generate transitive dependencies by analyzing configurations strings (e.g. "compile->provided")
* So for such project: <br>
* {{{ val root = (project in file(".")).dependsOn(proj1 % "compile->provided") }}}
* `Classpaths#interSort` will return:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure that this special case mentioned in the comment is covered with tests.
ATM it might be easier to use sbt import integration tests in Scala Plugin

@azdrojowa123 azdrojowa123 force-pushed the insert_all_module_transitive_dependencies branch 2 times, most recently from 55aad34 to b4286e1 Compare October 20, 2023 08:23
@azdrojowa123
Copy link
Contributor Author

azdrojowa123 commented Oct 20, 2023

I didn't resolve conversations that relate to the changes that I made in CR commits and may be discussable, or I am not sure that this is what was meant.
In general, I finished the review

val allSourceConfigurationsTask = StructureKeys.allSourceConfigurations.in(projectRef).get(state)
val allTestConfigurationsTask = StructureKeys.allTestConfigurations.in(projectRef).get(state)

val classpathConfigurationTask = sbt.Keys.classpathConfiguration.in(projectRef)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to use only allConfigurationsWithSource setting key for all projects, it is more cleaner

} yield {

val projectToTransitiveDependencies = getTransitiveDependenciesForProject(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of allSourceConfigurationsTask and allTestConfigurationsTask now I only use one setting key.
For now I left that sourceConfigurations and classpathConfigurationTask are always executed, not only when options.insertProjectTransitiveDependencies is true for simplicity. It shouldn't waste a lot of resources.
If I notice an increase in import time during tests on my branch in comparison to idea233.x, I will come back here.

* would be calculated to `TEST` (in [[org.jetbrains.sbt.project.SbtProjectResolver.scopeFor]]) which would not be truth,
* because now the scope for `dummy` dependency should be `COMPILE`.
*
* Check behaviour of this logic when SCL-18284 will be fixed
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested in testCustomConfigurationsWithNestedProjectDependencies in the scala plugin

* [[sbt.Classpaths.interSort]] for `compile` scope in project `root` will return: {{{
* compile -> (proj1, "provided"), (proj2, "compile")
* }}}
* which in practice means that we only have to add `proj2` as a dependency to `root` and `proj1` dependency shouldn't be taken into account.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested in testNonSourceConfigurationsWithNestedProjectDependencies in scala plugin

@azdrojowa123 azdrojowa123 force-pushed the insert_all_module_transitive_dependencies branch from ce411a3 to 7d0c3fe Compare October 24, 2023 09:11
[project transitive dependencies] fix deserializers
@azdrojowa123 azdrojowa123 force-pushed the insert_all_module_transitive_dependencies branch from 7d0c3fe to feaf978 Compare October 24, 2023 11:30
@azdrojowa123 azdrojowa123 force-pushed the insert_all_module_transitive_dependencies branch from feaf978 to 258c788 Compare October 24, 2023 12:19
@azdrojowa123 azdrojowa123 merged commit 8d25f0d into master Oct 24, 2023
1 check passed
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.

2 participants