Skip to content

Add Automatic-Module-Names to MANIFEST.MF files of projects with suffix_3 #12903

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

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

makingthematrix
Copy link
Contributor

The issue is described in #12899

In this PR I decided to add Automatic-Module-Name values only to projects with the suffix _3, that is:

  1. scala3-compiler_3
  2. scala3-library_3
  3. scala3-library-sjs1_3
  4. scala3-language-server_3
  5. scala3-staging_3
  6. scala3-tasty-inspector_3
  7. scaladoc_3
  8. tasty-core_3
    and for consistency also tasty-core-scala2. The values are derived from dottyOrganization + the project name.

…ix _3

The issue is described in scala#12899

In this PR I decided to add `Automatic-Module-Name` values only to projects with the suffix _3, that is:
1. scala3-compiler_3
2. scala3-library_3
3. scala3-library-sjs1_3
4. scala3-language-server_3
5. scala3-staging_3
6. scala3-tasty-inspector_3
7. scaladoc_3
8. tasty-core_3
and for consistency also tasty-core-scala2. The values are derived from dottyOrganization + the project name.
@smarter
Copy link
Member

smarter commented Jun 22, 2021

Can we just do it for all projects to reduce the lines of code added to the build?

@makingthematrix
Copy link
Contributor Author

@smarter : Sure, but I don't know how :) This is my first PR here.

@smarter
Copy link
Member

smarter commented Jun 22, 2021

I think you could add this setting in commonSettings

asDottyCompiler(NonBootstrapped).
settings(
Compile / packageBin / packageOptions +=
Package.ManifestAttributes("Automatic-Module-Name" -> s"$dottyOrganization-${name.value}")
Copy link
Member

Choose a reason for hiding this comment

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

According to https://docs.oracle.com/javase/specs/jls/se16/html/jls-7.html#jls-7.7:

A module name consists of one or more Java identifiers (§3.8) separated by "." tokens.

A java identifier can't have a - in its name, so I don't think this is a valid name, use a . instead.

Copy link
Member

Choose a reason for hiding this comment

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

Similarly, you probably need to replaceAll("-", ".") to handle the "-" in the project names.

Copy link
Member

Choose a reason for hiding this comment

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

Also instead of name I think we should use moduleName, that's what shows up in the pom in the end.

@smarter
Copy link
Member

smarter commented Jun 24, 2021

Thanks, this looks fine to me but can you confirm that this solves the issue you saw with maven in #12899 ? (You can publish scala3 using publishLocal from sbt)

@makingthematrix
Copy link
Contributor Author

Thanks, this looks fine to me but can you confirm that this solves the issue you saw with maven in #12899 ? (You can publish scala3 using publishLocal from sbt)

Sorry. Yesterday I only had time to make the update. Yes, I tested it with the project I mentioned in the issue and the warning is gone now. By the way, I have never had any problems with the '-' character in the automatic module name, but okay, replacing it with '.' shouldn't hurt.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Thank you!

@smarter smarter merged commit 359881d into scala:master Jun 24, 2021
@SethTisue
Copy link
Member

There is some discussion about split packages at https://users.scala-lang.org/t/gradle-7-6-jdk-19-and-scala-3-java-lang-classnotfoundexception-scala-predef-even-though-scala-library-2-13-10-jar-is-in-classpath/8972

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