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

[CSharp] #2021 fixes nuget packaging options to avoid missing dll exceptions #2024

Merged
merged 2 commits into from
Nov 25, 2017
Merged

[CSharp] #2021 fixes nuget packaging options to avoid missing dll exceptions #2024

merged 2 commits into from
Nov 25, 2017

Conversation

listerenko
Copy link
Contributor

@listerenko listerenko commented Sep 25, 2017

@ericvergnaud Hi,
I modified csproj options a bit, now I can get a working nuget package locally without the issue we described in #2021.
I added .net 3.5 as a target to "main" csproj along with netstandard, since it's easier to keep track of requirements for both sets of api's when editing code and, ideally, both targets can be packed into a nuget package with a single command. Right now it's possible only on Windows via msbuild /t:pack or Visual Studio; unfortunately, due to dotnet/msbuild#1333, right now dotnet build pack does not work for .net 3.5 target the way it should, so I adjusted the existing script to create packages from .nuspec and different solutions for different targets.

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Sep 25, 2017

Hey!
thanks for the help, much appreciated.
Up to now I've always released from my mac using Mono.
Not sure if build-nuget-package.sh is of much use on a wintel box..
Would it be too much to ask if you can provide a powershell script for releasing from Windows?

@listerenko
Copy link
Contributor Author

Nah, it's a single msbuild call, so that's ok, I added the script, although the old one should continue to work with mono, it's just less convenient. It's cumbersome to have two ways to make a package, however.

@ericvergnaud
Copy link
Contributor

Thanks
From your previous comment; I thought that due to dotnet/msbuild#1333 packaging was only possible on windows. Hence my ask for the script.
What am I missing?

@listerenko
Copy link
Contributor Author

listerenko commented Sep 27, 2017

Sorry, didn't make myself clear enough. It's not about windows, it's an issue with dotnet cli . You can't use it both on windows and Linux to build projects targeting .net 3.5, you need full msbuild for it, or, in your case, xbuild from mono. So I edited your existing script to make dotnet build only standard target (explicitly setting it in build command parameters), the rest is still done by xbuild and nuget. The downside is that you have two ways to make a package and three sln/csproj files, so it gets a bit confusing, but the only way to get rid of them are either dropping .net 3.5 support (in that case you just use single csproj and dotnet build pack on all platforms), or dropping requirement to be able to build package with both targets on non-windows machine. Anyone will still be able to compile netstandard target on any platform, but windows will be required to make nuget package with both standard and net3.5 dll's.

@listerenko
Copy link
Contributor Author

Or you leave everything as is, this way you can build everything everywhere, but have to cope with multiple csproj's and duplicating package metadata in nuspec and csproj.

@ericvergnaud
Copy link
Contributor

Ok I guess we will publish from windows until MS fixes #1333 .
Then we'll get back to a single unified way that works on all platforms.
Again, many thanks for your help with this.
@parrt are you comfortable with this?

@parrt
Copy link
Member

parrt commented Sep 27, 2017

@ericvergnaud can you explain the changes for me? Is it just packaging? new name?

@listerenko
Copy link
Contributor Author

if you are comfortable with publishing from windows, excess csproj/sln should probably be removed to reduce clutter. It will still be possible to compile .net standard version on any os supported by .net core.

@parrt hi. It's about more simple packaging process (all metadata in single file, both target frameworks are defined in single csproj file, single msbuild call for all restore/build/pack operation); defining both target frameworks (.net 3.5 and .net standard) in single csproj allows IntelliSense to perform analysis against both targets, thus preventing you from introducing changes incompatible with any of the platforms.
During build both targets will produce dll with the same name, yes, since it's more correct behaviour when targeting multiple runtimes and working with .net standard api's (or you'll get weird dll not found errors depending on runtime you use to build an app transitively dependent on this library, that's what #2021 was initially about).

@parrt
Copy link
Member

parrt commented Sep 28, 2017

Hmm...well,I really need to rely on you guys for correct C# behavior and packaging. Am I still able to build from Mac? How does this affect the nuget packages we have now?

@listerenko
Copy link
Contributor Author

You can build from any platform supported by .net core tools (including Mac) using command dotnet build -c Release -f netstandard1.3 Antlr4.dotnet.sln. You'll get working dll targeting .net standard, which you can consume from both mono and .net core projects. I'm not sure about building for .net 3.5 using mono, I don't know if xbuild supports new csproj format, but this should not be an issue anyway since libraries built for .net standard are supported by current mono version.
As for nuget package, supported platforms will remain the same (basically everything .net related after 3.5), but the issue my co-worker described in #2021 will be fixed.

@parrt
Copy link
Member

parrt commented Sep 29, 2017

Ok, i'm not sure exactly what @ericvergnaud wants me to make a decision on. I don't know much about the C# ecosystem.

@ericvergnaud
Copy link
Contributor

@parrt our current CI is not affected, but our release process to Nuget is.
Due to bugs/limitations, we need to publish from Windows, and I want to make sure you are comfortable with this.

@parrt
Copy link
Member

parrt commented Sep 30, 2017

Ah! Hmm...I think I can get a Windows VM running on my mac. would it require installation of all M$ dev tools and such or just nuget stuff?

@ericvergnaud
Copy link
Contributor

I believe the community edition suffices:
https://www.visualstudio.com/fr/thank-you-downloading-visual-studio/?sku=Community&rel=15
@kaedvann can you confirm?

@listerenko
Copy link
Contributor Author

Yes, community edition will work, you really require only msbuild anyway. Oh, and to enable building for .net 3.5 on modern windows you will need to explicitly enable it in "Windows Features", like described here, and add .net 3.5 sdk during visual studio install (they improved installer in 2017, so you'll only need to check relevant sdk's in 'individual components' tab)

@parrt
Copy link
Member

parrt commented Oct 2, 2017

Ok, So there is no name change or anything like that on the visible nuget product?

@parrt
Copy link
Member

parrt commented Oct 2, 2017

Ok, i have windows 10 x64 in vmware with Visual Studio Comm 2015. Is that sufficient?

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Oct 2, 2017 via email

@listerenko
Copy link
Contributor Author

@parrt just to make sure, is it visual studio 2015 or visual studio 15 (which is in fact visual studio 2017, they need some consistency on naming)? If really is vs 2015 (it will have version id like 14.0.25420.10 in "about" window), it will not work. If the version shown is something like 15.3.3 or 15.3.4, than it's fine. You'll need to add msbuild to PATH (it'll be something like C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin) and then execute the script I added (runtime/CSharp/runtime/CSharp/Build-NugetPackage.ps1) from Powershell.

@parrt
Copy link
Member

parrt commented Oct 3, 2017

Pretty sure it's 2015 not 15. Heh, will the mac version of VS work?

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Oct 3, 2017 via email

@parrt
Copy link
Member

parrt commented Oct 3, 2017

ok, i'm away from work machine today so will try later.

@listerenko
Copy link
Contributor Author

@parrt hi, any luck with setting up renewed build?

@parrt
Copy link
Member

parrt commented Oct 9, 2017

Okay it looks like I have Visual Studio Community 2015

screen shot 2017-10-09 at 4 42 03 pm

I am downloading 2017 now.

@parrt
Copy link
Member

parrt commented Oct 9, 2017

Ok, now says:
screen shot 2017-10-09 at 4 48 59 pm

Do I need any other installed products?

@listerenko
Copy link
Contributor Author

VS is fine, we just need to ensure you have necessary sdk's and stuff

  1. ensure msbuild is added to PATH, is should be accessible from command line (it is installed with visual studio, just not added to PATH automatically)
  2. ensure .net 3.5 is enabled in Programs & Features (I described it a bit earlier)
  3. .net core sdk should be installed (dotnet --version works in command line and shows something meaningful)

Hm, that should be it.

@parrt
Copy link
Member

parrt commented Oct 10, 2017

Ok, enabled .net 3.5. and msbuild appears to work. I had to reboot after setting the path. hahaha. just like old times.

screen shot 2017-10-10 at 12 24 03 pm

.net core sdk shows 2.0.0 from prompt. Sound good?

@listerenko
Copy link
Contributor Author

Yup, now the script runtime/CSharp/runtime/CSharp/Build-NugetPackage.ps1 should create you a nuget package with both targets and all relevant metadata filled.

@listerenko
Copy link
Contributor Author

@parrt Hi, sorry to bother you, but have you had any lack with packaging?

@parrt
Copy link
Member

parrt commented Oct 18, 2017

hiya. just finished school semester and today off to 3 or 4 day vacation. i hope to start antlr stuff again on Monday

@listerenko
Copy link
Contributor Author

listerenko commented Nov 13, 2017

@xied75 that was helpful, thank you.
@parrt Hi, I got appveyor to produce nuget packages(https://ci.appveyor.com/project/parrt/antlr4/build/4.7.1-SNAPSHOT+AppVeyor.1350/artifacts)
I also updated the doc about releasing and removed unnecessary package.nuspec to avoid confusion. So it looks like you can publish the package from any platform as long as appveyor works and you have nuget.exe.
I tried to remove other excess .csproj files, but they are somehow used in testing, which I couldn't completely figure out and rewrite to new csproj, so I'll guess they'll live till next improvement attempt.

@parrt
Copy link
Member

parrt commented Nov 13, 2017

great! so if @ericvergnaud signs off on this I'll merge. I can see the artifact :)

@listerenko listerenko changed the title #2021 fixes nuget packaging options to avoid missing dll exceptions [CSharp] #2021 fixes nuget packaging options to avoid missing dll exceptions Nov 16, 2017
@listerenko
Copy link
Contributor Author

Has been a while. @ericvergnaud ?

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Nov 23, 2017 via email

@ericvergnaud
Copy link
Contributor

@parrt if it works for you, it works. blessed

@parrt
Copy link
Member

parrt commented Nov 25, 2017

Seems to work and I downloaded an artifact, though I didn't test the artifact as I didn't know immediately how to test it. @kaedvann can you verify that the artifact at appveyor is correct?

@parrt
Copy link
Member

parrt commented Nov 25, 2017

@kaedvann Hmm...something is wrong with the C# build on travis: https://travis-ci.org/antlr/antlr4/jobs/307183842 can you take a look?

@ewanmellor
Copy link
Contributor

@parrt That failure is #2078 and there is already a PR to retry around it: #2124 .

@parrt parrt added this to the 4.7.1 milestone Nov 25, 2017
@parrt parrt merged commit 963d44f into antlr:master Nov 25, 2017
@parrt
Copy link
Member

parrt commented Nov 25, 2017

Cool. i just merged that one and this one. thanks VERY much for pushing this through!

@listerenko
Copy link
Contributor Author

Great! But I still would like to answer @parrt . Yes, I checked the produced artifact, it seemed to resolve the initial issue this PR was about. About testing - NuGet has a command to install package from local file, something like Install-Package SomePackage -Source C:\PathToThePackageDir\. If you have a test project or something, you could add this nuget to the project and run some checks. Since the packages contains two dll's, you'll need to run the tests both on full framework (Mono or Windows, shouldn't matter) or .NET Core runtime. Oh, and as the package is in fact a zip-archive, you could unpack it and check .nuspec inside to ensure all the metadata about the package is correctly filled (I checked that too, but still, anything can happen).
Look forward to seeing this updated package on nuget. :)

@xied75
Copy link
Contributor

xied75 commented Nov 27, 2017

@parrt @ericvergnaud I wonder should we version the new nuget package to e.g. 4.7.2? So that it won't replace the current 4.7.1 to avoid breaking already running builds.

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Nov 27, 2017 via email

@parrt
Copy link
Member

parrt commented Nov 27, 2017

Oh right we did some weird thing with numbering to get out a release. can we do 4.7.1.1?

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Nov 27, 2017 via email

@xied75
Copy link
Contributor

xied75 commented Nov 27, 2017

Not that good if considering SemVer is not using the 4th number.

@parrt
Copy link
Member

parrt commented Nov 27, 2017

Yeah but better than 4.7.2 which would mean 4.7.1 ;)

@parrt
Copy link
Member

parrt commented Nov 29, 2017

@ericvergnaud @kaedvann can you alter the build config for 4.7.1.1? I'd like to push out 4.7.1 soon. thanks!

@listerenko
Copy link
Contributor Author

listerenko commented Nov 30, 2017

@parrt it is only the <Version> node in the Antlr4.Runtime.dotnet.csproj file. No other adjustments required

@parrt
Copy link
Member

parrt commented Dec 4, 2017

Ok, done: b9adef5

ewanmellor added a commit to ewanmellor/antlr4 that referenced this pull request Nov 15, 2018
Add -f netstandard1.3 to the dotnet build command used on Travis.

This has been necessary since PR antlr#2024, and the build has been silently failing
all this time.
ewanmellor added a commit to ewanmellor/antlr4 that referenced this pull request Nov 15, 2018
Add -f netstandard1.3 to the dotnet build command used on Travis.

This has been necessary since PR antlr#2024, and the build has been silently failing
all this time.
ewanmellor added a commit to ewanmellor/antlr4 that referenced this pull request Nov 15, 2018
Add -f netstandard1.3 to the dotnet build command used on Travis.

This has been necessary since PR antlr#2024, and the build has been silently failing
all this time.
ewanmellor added a commit to ewanmellor/antlr4 that referenced this pull request Nov 15, 2018
Add -f netstandard1.3 to the dotnet build command used on Travis.

This has been necessary since PR antlr#2024, and the build has been silently failing
all this time.
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.

5 participants