-
Notifications
You must be signed in to change notification settings - Fork 1.1k
chore: add the scala-library
projects (non-bootstrapped and bootstrapped)
#23510
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
Conversation
cc869c3
to
78060b3
Compare
project/Build.scala
Outdated
* - The only commands you should run on this project are `publish` and `publishLocal` | ||
* - This project produces no artifacts and has no sources | ||
*/ | ||
lazy val `scala-library` = project.in(file("scala-library")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: publishSettings
are missing here, but I prefer to add them when we are ready to release
78060b3
to
b1ddc81
Compare
b1ddc81
to
618ad57
Compare
library/src/scala/Conversion.scala
Outdated
@experimental | ||
opaque type into[+T] >: T = T | ||
//@experimental | ||
//opaque type into[+T] >: T = T | ||
|
||
/** Unwrap an `into` */ | ||
extension [T](x: into[T]) | ||
@experimental def underlying: T = x | ||
//extension [T](x: into[T]) | ||
// @experimental def underlying: T = x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously this must be fixed before merging the PR. This triggers a stale symbol only in the new stdlib...
618ad57
to
f0967f9
Compare
266a732
to
6e3e584
Compare
project/Build.scala
Outdated
version := dottyVersion, | ||
versionScheme := Some("semver-spec"), | ||
crossPaths := false, // org.scala-lang:scala-library doesn't have a crosspath | ||
// NOTE: The only difference here is that we drop `-Werror` and semanticDB for now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
semanticDB fails because the definitions are not present in Predef, which will be fixed in another PR and -Werror
is self explanatory. Too many warnings for now that will have to be fixed later (some even in scala 2)
// Add the source directories for the stdlib (non-boostrapped) | ||
Compile / unmanagedSourceDirectories := Seq(baseDirectory.value / "src"), | ||
Compile / unmanagedSourceDirectories += baseDirectory.value / "src-non-bootstrapped", | ||
// NOTE: The only difference here is that we drop `-Werror` and semanticDB for now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://github.com/scala/scala3/pull/23510/files#r2215995517 for details
val externalLibraryDeps = (`scala3-library` / Compile / externalDependencyClasspath).value.map(_.data).toSet | ||
val externalCompilerDeps = (`scala3-compiler` / Compile / externalDependencyClasspath).value.map(_.data).toSet | ||
|
||
// IMPORTANT: We need to use actual jars to form the ScalaInstance and not | ||
// just directories containing classfiles because sbt maintains a cache of | ||
// compiler instances. This cache is invalidated based on timestamps | ||
// however this is only implemented on jars, directories are never | ||
// invalidated. | ||
val tastyCore = (`tasty-core` / Compile / packageBin).value | ||
val scala3Library = (`scala3-library` / Compile / packageBin).value | ||
val scala3Interfaces = (`scala3-interfaces` / Compile / packageBin).value | ||
val scala3Compiler = (`scala3-compiler` / Compile / packageBin).value | ||
|
||
val libraryJars = Array(scala3Library) ++ externalLibraryDeps | ||
val compilerJars = Seq(tastyCore, scala3Interfaces, scala3Compiler) ++ (externalCompilerDeps -- externalLibraryDeps) | ||
|
||
Defaults.makeScalaInstance( | ||
scalaVersion.value, | ||
libraryJars = libraryJars, | ||
allCompilerJars = compilerJars, | ||
allDocJars = Seq.empty, | ||
state.value, | ||
scalaInstanceTopLoader.value | ||
) | ||
}, | ||
scalaCompilerBridgeBinaryJar := { | ||
Some((`scala3-sbt-bridge` / Compile / packageBin).value) | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still uses the old compiler that uses the old stdlib, in another PR, when I add the versions of the compiler with the new stdlib as a dependency, we will proceed to the swap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that many other settings are required to be added before moving to prod. This is just one step to move forward in the build for now.
6e3e584
to
a19c2b0
Compare
scala-library
projectscala-library
projects (non-bootstrapped and bootstrapped)
Also, I'm explicitly splitting and duplicating the settings in each project as want to know exactly the configuration and not to start follow methods to see what gets configured and what doesn't. This is a choice, not a bad implementation. |
uses: actions/setup-java@v4 | ||
with: | ||
distribution: 'temurin' | ||
java-version: 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are already compiling the migration of stdlib with 17, ahead of the switch to Java 17 in 3.8.0
permissions: | ||
contents: read | ||
|
||
jobs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests CI directives e.g. [skip_ci]
or [compile_nonboostrapped]
are missing, currently we have already multiple jobs that would execute even though they're told to not be executed. If possible we should gradually try to unify all of GHA jobs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer not to have them for multiple reasons:
- This is a temporary files
- CI will have to be redesigned anyways (subsequent PR)
- I prefer always checking this during the transition period (until we actually proceed to the swap). I don't want to have any surprises down the road.
Compile / unmanagedSourceDirectories := Seq(baseDirectory.value / "src"), | ||
Compile / unmanagedSourceDirectories += baseDirectory.value / "src-bootstrapped", | ||
// NOTE: The only difference here is that we drop `-Werror` and semanticDB for now | ||
Compile / scalacOptions := Seq("-deprecation", "-feature", "-unchecked", "-encoding", "UTF8", "-language:implicitConversions"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: we're using here 2 ways of providing settings: using seperate strings ("-encoding", "UTF8"
), and using a single argument with separator (-language:implicitConversions
).
Maybe let's pick a single style and use it?
Personally I'm in favour of replacing -encoding:UTF8
change as it's easier to ready when all data is kept in single token.
Compile / scalacOptions := Seq("-deprecation", "-feature", "-unchecked", "-encoding", "UTF8", "-language:implicitConversions"), | ||
(Compile / scalacOptions) ++= Seq( | ||
// Needed so that the library sources are visible when `dotty.tools.dotc.core.Definitions#init` is called | ||
"-sourcepath", (Compile / sourceDirectories).value.map(_.getCanonicalPath).distinct.mkString(File.pathSeparator), | ||
), | ||
// Only publish compilation artifacts, no test artifacts | ||
Compile / publishArtifact := true, | ||
Test / publishArtifact := false, | ||
// Do not allow to publish this project for now | ||
publish / skip := true, | ||
// Project specific target folder. sbt doesn't like having two projects using the same target folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the argument of explicit, duplicated, configuration, but maybe let's create a Def.settings
directly above these project definitions with the common parts that are known to be always shared between the two?
Otherwise, sooner or later these 2 would diverge.
So stuff like the marked settings, crossPaths
, versionScheme
, moduleName
can be defined in a single place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a branch with these kind of changes but it's coming later. For now, I just need the base project to unblock some rebases like explicitly null checking the stdlib and stuff
Add the configuration for the
scala-library
project in sbt.For now, this configuration is disabled and doesn't publish anything until we enable it in 3.8.0