Skip to content

MessagePack.Generator RollForward to Major#1445

Merged
AArnott merged 1 commit intomasterfrom
generator-rollforward-major
Jun 14, 2022
Merged

MessagePack.Generator RollForward to Major#1445
AArnott merged 1 commit intomasterfrom
generator-rollforward-major

Conversation

@neuecc
Copy link
Copy Markdown
Member

@neuecc neuecc commented Jun 10, 2022

Currently, there are reports that mpc stops working after each .NET major version upgrade.
How about setting the RollForward in the CLI tool to Major to prevent this?

@neuecc neuecc requested a review from AArnott June 10, 2022 01:44
@AArnott
Copy link
Copy Markdown
Collaborator

AArnott commented Jun 10, 2022

This comes because our last stable release still targets .NET 5.0, which is no longer supported by Microsoft. We really should ship a stable release that targets a supported runtime (.NET 6.0). The develop branch already adds net6.0 support. How do you feel about us releasing 2.4 as a stable release?

We could also merge this, but I get a bit anxious about such a setting because .NET major versions include breaking changes. I'd kinda like to ship on a runtime we've tested, and have things like this prompt us to upgrade. .NET 6 will be around till Nov 2024, so adding .NET 6 support will take care of this problem for a long time.

@AArnott
Copy link
Copy Markdown
Collaborator

AArnott commented Jun 10, 2022

Also, I wonder if your change actually does anything. I just tried packing the tool with your change, and I don't see that setting anywhere in the package.

@neuecc
Copy link
Copy Markdown
Member Author

neuecc commented Jun 10, 2022

Yes, I agree with making 2.4 STABLE.

However, these problems are repeated with the release of .NET 7 and with the release of .NET 8.
In fact, we often hear reports of mpc not working in .NET 6.

It seems that there are not a few instances of encountering such problems and setting Major.
fixie/fixie#246
microsoft/playwright-dotnet#1791
(and other many... https://github.com/search?l=C%23&q=rollforward&state=closed&type=Issues )
I understand that this is different from Microsoft's "ideal" as the default is not, but I find it very inconvenient in practice.

I want that when .NET 7 is released, mpc will be in a state where it will work without updates in an environment where only .NET 7 is installed.
I thought this csproj specification was all I needed to specify, but do I need to specify anything else?

@mayuki
Copy link
Copy Markdown
Contributor

mayuki commented Jun 10, 2022

For example, after the release of .NET 7, if a developer sets up a development environment on a new machine and installs only .NET 7 SDK, MessagePack.Generator (mpc) cannot be used unless it is targeting net7.0.
It is slightly inconvenient to require .NET 6 SDK/Runtime to be installed just for the tool.

Of course, it is preferable to keep the targets updated, but I expect that it will be difficult to release projects in time for the release of .NET.

While I can understand the compatibility concerns, but as a user it would be nice to have a way to reduce the inconvenience caused by the time lag between release .NET and this project.

@AArnott
Copy link
Copy Markdown
Collaborator

AArnott commented Jun 14, 2022

Fair enough. I'll prepare to release 2.4 as stable.
As for this PR, I don't mean to block it. But we should find out how to actually do it properly. The content of the msbuild project is only important insofar as it propagates to the tool package somehow. My validation suggests that this change doesn't propagate, so we may need to research this more.

@AArnott AArnott added this to the v2.4 milestone Jun 14, 2022
Copy link
Copy Markdown
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Ok, I found where this project property actually gets embedded in the tool package (the mpc.runtimeconfig.json file). So I think this will work as advertised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants