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

Package NEON as .net global tool #166

Merged
merged 4 commits into from
Dec 13, 2019
Merged

Package NEON as .net global tool #166

merged 4 commits into from
Dec 13, 2019

Conversation

devhawk
Copy link
Contributor

@devhawk devhawk commented Dec 11, 2019

Fixes #164, #159, #82 , #163

NEON was updated to .NET core 3.1 as part of #154. This PR adds additional changes in csproj files to enable building NEON as a .NET Core Global Tool. This will allow developers to install NEON from the command line instead of having to clone this repo and build manually

$ dotnet tool install -g Neo.Neon

Currently, the Neo.Compiler.MSIL builds as both .NET Standard 2.0 and .NET Core 3.1 to support scenarios where someone wants to use the compiler as a library. Unfortunately, you can't build as a global tool when targeting .net standard. So I refactored the build script to allow building as .net standard and .net core global tool separately. dotnet pack will build Neo.Compiler.MSIL as a .NET Standard 2.0 library package while dotnet pack /p:GlobalTool=true builds Neo.Compiler.MSIL as a .NET Core 3.1 global tool. To differentiate the packages, I gave the global tool package the name Neo.Neon instead of Neo.Compiler.MSIL.

Additionally, this PR bumps the version numbers.

  • The current version of Neo.SmartContract.Framework is v2.9.3.1. This PR bumps the version to v2.10 as per Semantic Versioning guidelines
  • The current version of Neo.Compiler.MSIL is v2.4.1.1. This PR bumps the version to v2.6. Semantic Versioning guidelines would suggest a new version number of v2.5, but the temporary Neo.neon-de fork already shipped v2.5.x releases. Bumping Neo.Neon to v2.6 makes it clear that this is the more up-to-date package.

<PropertyGroup Condition="'$(GlobalTool)' == true">
<PackAsTool>true</PackAsTool>
<TargetFramework>netcoreapp3.1</TargetFramework>
<PackageId>Neo.Neon</PackageId>
Copy link
Member

Choose a reason for hiding this comment

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

neon?

Copy link
Member

@superboyiii superboyiii Dec 12, 2019

Choose a reason for hiding this comment

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

I think it makes sense as the name of . netcoreapp since it's always neon.exe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you're asking @erikzhang. The exe is named neon, but the package is named neo.neon. All neo packages need to have the "neo" prefix in order to get associated with the neo-project nuget org

Copy link
Member

Choose a reason for hiding this comment

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

If the PackageId doesn't match the exe name, we have to install it by a name and use it by another name. It is too confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neo express already works this way. Exe name is neo-express while package name is neo.express. Besides, developers aren't typically going to execute neon.exe manually, it is automatically invoked by the contract build script created by the template.

Additionally, there are already a bunch of neon packages on nuget.org that we don't want to get confused with

Copy link
Member

Choose a reason for hiding this comment

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

What is the benefit of a PackageId and exe name mismatch?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's used to avoid conflict of the same name for packages on nuget.

Copy link
Member

Choose a reason for hiding this comment

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

I searched on nuget, there is no conflict name.

Copy link
Member

Choose a reason for hiding this comment

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

Can we call it Neo2.XXX ?

Copy link
Member

Choose a reason for hiding this comment

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

I prefer neon with version 2.x.

@superboyiii
Copy link
Member

@devhawk Nice feature! Harry, you missed neo-logo-72.png at the same path of neo-devpack-dotnet.sln. And I just use a png of the same name to test, it works on both .NET Standard 2.0 and .NET Core 3.1. But some issue appear when installing, not sure how to solve.
image
image

@devhawk
Copy link
Contributor Author

devhawk commented Dec 12, 2019

Thanks for testing this @superboyiii! I'm not sure what you mean by

you missed neo-logo-72.png at the same path of neo-devpack-dotnet.sln.

that file was added in commit e7d56b8. I added it at the root so it could be accessed for packages built out of ./Neo.Compiler.MSIL and ./Neo.SmartContract.Framework

@devhawk
Copy link
Contributor Author

devhawk commented Dec 12, 2019

@superboyiii I'm sorry, I provided instructions for building the neo.neon package but I forgot to include instructions for installing it!

I see you've successfully built the tool package using /p:GlobalTool=true. In order to install the tool, you need to provide the package location via --add-source. Otherwise, dotnet tool install looks in nuget.org for the tool package. Eventually that will work, but the package has not yet been published on nuget.org yet. You should be able to install the package with this command:

> dotnet tool install neo.neon -g --add-source .\bin\Debug\

@superboyiii
Copy link
Member

That's cool! Thanks for your clear instruction.

@shargon
Copy link
Member

shargon commented Dec 12, 2019

I will test it tomorrow!

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

Tested, and works perfect! but i think that we should rename it to something related to neo2.
neo2.neon or something like that.

@shargon
Copy link
Member

shargon commented Dec 13, 2019

@devhawk I was not able to uninstall it, what i am doing wrong?
image

@devhawk
Copy link
Contributor Author

devhawk commented Dec 13, 2019

@shargon you misspelled uninstall

@devhawk
Copy link
Contributor Author

devhawk commented Dec 13, 2019

i think that we should rename it to something related to neo2.
neo2.neon or something like that.

I think there should be one version of NEON that works with both neo 2 & 3. I filed #83 to this effect a while ago.

@devhawk devhawk merged commit 362b634 into neo-project:master-2.x Dec 13, 2019
@devhawk devhawk deleted the devhawk/neonGlobalTool branch December 13, 2019 20:44
@shargon shargon mentioned this pull request Dec 17, 2019
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.

None yet

4 participants