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

Setup the 3.4.1 development #19321

Closed
wants to merge 2 commits into from
Closed

Setup the 3.4.1 development #19321

wants to merge 2 commits into from

Conversation

Kordyjan
Copy link
Contributor

@Kordyjan Kordyjan commented Dec 21, 2023

The tasty version is set to one minor higher than it should be, so it differs from 3.4.0-RC1. The same technique has been used since 3.0.0.

No changes were needed in MiMa filters. They, however, will need to be adjusted after the stable 3.4.0 release.

@Kordyjan Kordyjan mentioned this pull request Dec 21, 2023
18 tasks
@bishabosha
Copy link
Member

bishabosha commented Dec 21, 2023

I think we could consider 28.4-2 meaning it was released before the stable 3.4 release, but incompatible with 28.4-1,

Edit: although given 3.4.1-RC1 is supposed to be after 3.4.0 maybe what I say is better for 3.4.0-RC2 - so probably ignore it all to avoid hassle

@nicolasstucki
Copy link
Contributor

W have a hacky way to work around the TASTy MiMa version issue in https://github.com/lampepfl/dotty/blob/main/project/scripts/scala2-library-tasty-mima.sh. We might use the same trick here while we find a proper solution.

That script changes the TASTy format version to the last version that is supported by TASTy MiMa. To do this it modifies TastyFormat.scala, cleans everything, and recompiles with the this fake version.

also remove obsolete syntax from test
@Kordyjan
Copy link
Contributor Author

Kordyjan commented Jan 11, 2024

@nicolasstucki I'm not sure if I understand. Do you mean solving the

Backward incompatible TASTy file has version 28.4-experimental-1, produced by Scala 3.4.0-RC1,
  expected stable TASTy from 28.0 to 28.4, or exactly 28.5-experimental-1.

problem from tests, or something else?

@nicolasstucki
Copy link
Contributor

I'm not sure if I understand. Do you mean solving the ... problem from tests, or something else?

I thought you might get a similar issue on TASTy MiMa. But it seems you did not. Therefore my previous comment seems irrelevant.

@nicolasstucki
Copy link
Contributor

@bishabosha is this error message correct in this situation?

Backward incompatible TASTy file has version 28.4-experimental-1, produced by Scala 3.4.0-RC1,
  expected stable TASTy from 28.0 to 28.4, or exactly 28.5-experimental-1.

@bishabosha
Copy link
Member

bishabosha commented Jan 13, 2024

is this error message correct in this situation?

its illegal to read any experimental tasty unless its the exact same version, this is why I suggested to use 28.3-experimental-2, at least then the error message would say it expects to see "stable TASTy from 28.0 to 28.3"

@nicolasstucki
Copy link
Contributor

nicolasstucki commented Jan 16, 2024

What seems to have happened is that we forgot to update the TASTy version before releasing 3.4.0-RC1. This implies that 3.4.0-RC1 produces TASTy with the same version as 3.3.X (minor version 4). We should make sure that 3.4.0-RC2 has the TASTy minor version 5.

Note that we accidentally skipped one TASTy minor version in 3.3.1 (#16768) passing from 2 to 4, instead of minor version 3. This implies that now we are following a scheme where Scala 3.N.X produces TASTy with version N+1. As shown by this problem we have now, this scheme is not ideal. We could realign them in the future by releasing a new minor Scala version that does not have binary changes (a glorified patch release).

I was wrong, the TASTy minor version are fine. The base version was wrong, but fixed afterward.


val baseVersion = "3.4.0-RC1"
val baseVersion = "3.4.1-RC1"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add some tests that check that the minor version N (3.N.X) is equal to the minor TASTy version minus one.

@@ -83,9 +83,9 @@ object DottyJSPlugin extends AutoPlugin {
object Build {
import ScaladocConfigs._

val referenceVersion = "3.3.1"
val referenceVersion = "3.4.0-RC1"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should either wait to have 3.4.0-RC2 that has the correct minor TASTy version or keep using 3.3.1

Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be 3.4.0? why would we start 3.4.1 before 3.4.0 was released?

@nicolasstucki
Copy link
Contributor

nicolasstucki commented Jan 16, 2024

@Kordyjan what I said yesterday during the meeting was wrong. We should not need to disable the non-bootstrapped tests if the TASTy version are correctly updated.

Also see #19321 (comment)

@@ -12,9 +12,66 @@ object MiMaFilters {
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#TypeReprMethods.dealiasKeepOpaques"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.paramVariance"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#TypeLambdaMethods.paramVariances"),
ProblemFilters.exclude[MissingClassProblem]("scala.annotation.internal.AssignedNonLocally"),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check MiMa against 3.4.0-RC1 and not 3.3.1. See #19469.

@@ -318,7 +318,7 @@ object TastyFormat {
* compatibility, but remains backwards compatible, with all
* preceeding `MinorVersion`.
*/
final val MinorVersion: Int = 4
final val MinorVersion: Int = 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final val MinorVersion: Int = 5
final val MinorVersion: Int = 4

See #13447

Copy link
Contributor Author

@Kordyjan Kordyjan Jan 18, 2024

Choose a reason for hiding this comment

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

I'm even more confused now. We are now setting up a tasty version for the nightly releases. According to this diagram and this table, when we have a nightly version in form 3.x.y, where y > 0, tasty should have version 28.(x+1)-exp1. That means it should be 28.5-exp1 now, as we will release nightlies for 3.4.1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, based on those diagrams the Nighlies should be on 28.5. I do not know why nightlies should have a different version. It seems it would make more sense for them to have the same version. I guess that the current check does not take this into account. @bishabosha could you clarify this?

Copy link
Contributor

@nicolasstucki nicolasstucki Jan 19, 2024

Choose a reason for hiding this comment

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

We released Scala 3.4.0-RC1 with a 28.4-1 TASTy version. Based on the diagram it should have been 28.4-2, or maybe 28.4-0 (this might be a typo in the diagram). The error we got is consistent with this problem

Backward incompatible TASTy file has version 28.4-experimental-1, produced by Scala 3.4.0-RC1,
  expected stable TASTy from 28.0 to 28.4, or exactly 28.5-experimental-1.

However I am not sure if the check would take the -2 in 28.4-2 into account
https://github.com/lampepfl/dotty/blob/e64a5a5f08563af4790c808a541ed55454489dcc/tasty/src/dotty/tools/tasty/TastyFormat.scala#L339-L385

Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to update the TASTy minor version using 3.3.1 for now. See #19483. We can later update the reference version to 3.4.0-RC2. We just need to make sure we have the correct version for that one.

Copy link
Member

@bishabosha bishabosha Jan 19, 2024

Choose a reason for hiding this comment

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

Right, based on those diagrams the Nighlies should be on 28.5. I do not know why nightlies should have a different version. It seems it would make more sense for them to have the same version. I guess that the current check does not take this into account. @bishabosha could you clarify this?

Nightlies should be 1 minor version ahead because experimental is treated as before the next stable version - so 28.4-exp-1 can only read 3.3.x compiled code, therefore to read the 3.4.0 std lib, the 3.4.1-RC1-NIGHTLY must have 28.5-exp-1, however the 3.4.1-RC1 final must also have 28.4-0 tasty version

Copy link
Contributor

@nicolasstucki nicolasstucki Jan 19, 2024

Choose a reason for hiding this comment

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

however the 3.4.1-RC1 final must also have 28.4-0 tasty version

Should it be 28.4-0 or 28.4-2 as in the diagram?

Copy link
Member

@bishabosha bishabosha Jan 19, 2024

Choose a reason for hiding this comment

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

the 28.(x+1)-2 is only for RC when patch version is 0 (for minor version x), as that is considered still a preview before the minor is frozen. for RC of some patch release >0, it should be 28.x-0 because RC in the same minor should be forward compatible

nicolasstucki added a commit that referenced this pull request Jan 18, 2024
Part of #19321

We are currently releasing nightlies with the wrong version. We should
update this now. We do not need to wait until we can update the
reference version.

Now we can check MiMa against 3.4.0-RC1 to ensure that the API has not
changed.
However, that version has a couple of mistakes that will be fixed in
3.4.0-RC2.
Kordyjan added a commit that referenced this pull request Jan 18, 2024
@nicolasstucki nicolasstucki mentioned this pull request Jan 19, 2024
15 tasks
@WojciechMazur WojciechMazur deleted the start-3.4.1 branch October 9, 2024 13:34
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.

4 participants