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

bugfix: Don't use aggregate in Scala Native #4823

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Feb 16, 2025

This causes the nativeLink task to be executed for all the modules and it doesn't make sense to do that for non app modules.

build.sbt Outdated
@@ -229,7 +229,7 @@ lazy val cli = crossProject(JVMPlatform, NativePlatform, JSPlatform)
// TODO: enable NPM publishing
.jsSettings(scalaJsSettings, scalaJSUseMainModuleInitializer := true)
.jvmEnablePlugins(NativeImagePlugin)
.jvmConfigure(_.dependsOn(dynamic.jvm).aggregate(dynamic.jvm)).aggregate(core)
.jvmConfigure(_.dependsOn(dynamic.jvm).aggregate(dynamic.jvm).aggregate(core.jvm))
Copy link
Collaborator

Choose a reason for hiding this comment

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

the purpose of this aggregation was only to be able to run cliXxx/test and get tests from dependencies (in reality, only sysops).

so, in this case, we should remove this aggregation for all platforms and either:

  • add coreXxx/test to ci-test-xxx commands
  • or move .aggregate(core) to the tests project.

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 added it to tests instead.

This causes the nativeLink task to be executed for all the modules and it doesn't make sense to do that for non app modules.
@kitbellew kitbellew merged commit b822e23 into scalameta:main Feb 17, 2025
32 checks 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