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.

Sign up for GitHub

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

Jump to bottom

C# runtime upgrade #2987

Merged
merged 16 commits into from
Dec 31, 2020
Merged

C# runtime upgrade #2987

merged 16 commits into from
Dec 31, 2020

Conversation

KvanTTT
Copy link
Member

@KvanTTT KvanTTT commented Dec 2, 2020 •
edited
Loading

  • Drop support of outdated runtimes (net35) and standards (netstandard1.3)
  • Add support of netstandard2.0 and netstandard2.1 (see the table)
  • Simplify the structure of solutions and projects
  • Remove not used code under preprocessor directives

fix #2984

@KvanTTT KvanTTT changed the title C # runtime upgrade C# runtime upgrade Dec 2, 2020
@KvanTTT KvanTTT changed the title C# runtime upgrade [WIP] C# runtime upgrade Dec 2, 2020
@ericvergnaud
Copy link
Contributor

Not sure if you can make this work without updating the Java files for testing the C# target

@KvanTTT
Copy link
Member Author

KvanTTT commented Dec 3, 2020

Yes, that's why I put [WIP] suffix. I'm working on it.

@KvanTTT KvanTTT marked this pull request as draft December 26, 2020 18:43
@KvanTTT KvanTTT force-pushed the CSharpRuntimeUpgrade branch 15 times, most recently from 7cb8c6d to 995dea6 Compare December 28, 2020 13:06
@KvanTTT KvanTTT marked this pull request as ready for review December 28, 2020 14:23
@KvanTTT KvanTTT changed the title [WIP] C# runtime upgrade C# runtime upgrade Dec 28, 2020
@KvanTTT
Copy link
Member Author

KvanTTT commented Dec 28, 2020

@ericvergnaud please review.

ericvergnaud
ericvergnaud reviewed Dec 29, 2020
View reviewed changes
.travis.yml
@@ -162,21 +164,18 @@ matrix:
stage: main-test
- os: linux
jdk: openjdk8
dist: trusty
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove this in the context of this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Travis does not work with dotnet if this dist is used, see this build for example https://travis-ci.org/github/antlr/antlr4/builds/751735906

Does it matter?

@ericvergnaud
Copy link
Contributor

Can you help clarify how this change impacts the release process?
There is an imminent critical 4.9.1 bug fix release and I would not want this less critical change to be in the way.

@KvanTTT
Copy link
Member Author

KvanTTT commented Dec 30, 2020 •
edited
Loading

Install dotnet on any platform (see https://dotnet.microsoft.com/download) and run the following command on any OS (Win, Linux, macOS):

  • building: dotnet build runtime/CSharp/Antlr4.csproj -c Release.
    Output .dll will be in runtime/CSharp/bin/Release/netstandard2.0 or in runtime/CSharp/bin/Release/netstandard2.1
  • packing: dotnet pack runtime/CSharp/Antlr4.csproj -c Release.
    Output .nupkg will be in runtime/CSharp/bin/Release/Antlr4.Runtime.Standard.4.9.0.nupkg.

You can try to do it locally at frist.

Mono and .NET Framework are not required at all for building.

With netstandard2.0 it's possible to target NET Framework >= 4.6.1 (2015), NET Core >= 2.0, Mono >= 5.4 and other platforms (Xamarin, Unity, UWP).

@ericvergnaud ericvergnaud merged commit e613eae into antlr:master Dec 31, 2020
@parrt
Copy link
Member

parrt commented Jan 3, 2021

@KvanTTT heh, that build sequence work. Adding to the documentation.

@parrt parrt added this to the 4.9.1 milestone Jan 3, 2021
jnyrup
jnyrup reviewed Jan 25, 2021
View reviewed changes
runtime/CSharp/runtime/CSharp/Antlr4.Runtime/Properties/AssemblyInfo.cs
// You can specify all the values or you can default the Build and Revision Numbers
// by using the '*' as shown below:
// [assembly: AssemblyVersion("1.0.*")]
[assembly: AssemblyVersion("4.9")]
Copy link

Choose a reason for hiding this comment

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

I'm not sure if all these things are still used, but I noticed that AssemblyVersion changed from 4.9.0.0 in 4.9.0 to 0.0.0.0 in 4.9.1
Here are screenshots from [Package Explorer]
(https://github.com/NuGetPackageExplorer/NuGetPackageExplorer)

4.9.0
image

4.9.1
image

Copy link
Member

Choose a reason for hiding this comment

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

@KvanTTT and @ericvergnaud. oh no! did I miss updating version number? Can you tell me if this is catastrophic or whether we can leave things as they are until the next release?

@ericvergnaud
Copy link
Contributor

Looks like the issue was introduced before you released, when merging this PR #2987

KvanTTT added a commit to KvanTTT/antlr4 that referenced this pull request Jan 25, 2021
@KvanTTT
Fix missed version, antlr#2987 (review)
cf16ec7
@KvanTTT KvanTTT deleted the CSharpRuntimeUpgrade branch January 25, 2021 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parrt parrt parrt left review comments

@jnyrup jnyrup jnyrup left review comments

@ericvergnaud ericvergnaud ericvergnaud left review comments

Assignees
No one assigned
Labels
comp:build target:csharp
Projects
None yet
Milestone
4.9.1
Development

Successfully merging this pull request may close these issues.

netstandard 2.0 build
4 participants
@KvanTTT @ericvergnaud @parrt @jnyrup