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

Fix build on Linux (and other stuff) #355

Merged
merged 8 commits into from
Jan 21, 2020
Merged

Conversation

jasper-d
Copy link
Contributor

While working on #351, I encountered a problem when building on Linux like this: dotnet build ./src/NATS.sln -c LinuxRelease --no-incremental --force

/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018: The "ResolveTargetingPackAssets" task failed unexpectedly. [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018: System.IO.FileNotFoundException: Could not find file '/usr/share/dotnet/packs/Microsoft.NETCore.App.Ref/3.0.0/data/FrameworkList.xml'. [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018: File name: '/usr/share/dotnet/packs/Microsoft.NETCore.App.Ref/3.0.0/data/FrameworkList.xml' [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018:    at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter) [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018:    at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String path, OpenFlags flags, Int32 mode) [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018:    at System.IO.FileStream.OpenHandle(FileMode mode, FileShare share, FileOptions options) [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options) [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
/usr/share/dotnet/sdk/3.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(264,5): error MSB4018:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize) [/home/jd/sources/nats.net.origin/src/Tests/IntegrationTests/IntegrationTests.csproj]
//and so on...

I took a look at the solution and project configuration (that took some time) and changed the following:

  1. Fix build on Linux
  2. Remove special platforms for Linux (i.e. LinuxRelease & LinuxDebug) so building on Linux works with just dotnet build [-c Release|Debug] as one may expect
  3. Remove IsWindows and IsLinux constants
  4. Exclude the Winforms sample from the build on non-Windows OS by butchering its .csproj (necessary for 2. & 3.). This should isolate the non-standard configuration to the Winforms app.
  5. Remove global.json (it pins the required SDK to a specific patch release, was 3.0.101)
  6. Reduce the number of target frameworks where possible (we had a mix of netcoreapp2.2, netcoreapp3.0, netstandard1.6, net45 and net452). NATS.Client still targets netstand1.6 and net45. We now target netcoreapp3.1 as latests LTS release and net452 with all other projects (net452 is necessary for xunit to work)
  7. Removed assembly signing (strong naming)
  8. Adjusted the Azure pipeline definition to run tests on netcoreapp3.1 instead of 3.0 and 2.2 (the latter has reached end of life)

Tested these changes on Windows and Linux and it works.

Besides fixing the build on Linux, I hope that these changes make it easier to understand the setup for contributors not familiar with the code base (I had to dig through it to understand why the Linux specific platforms where used).

@jasper-d
Copy link
Contributor Author

Close & reopen to trigger CI

@jasper-d jasper-d closed this Jan 13, 2020
@jasper-d jasper-d reopened this Jan 13, 2020
@ColinSullivan1
Copy link
Member

@jasper-d, Thanks! This will take awhile to review and I'd like to test on a few platforms, please be patient...

About signing - we did have a specific request for signed assemblies. Users tend to want both signed and unsigned assemblies, so we figured those that didn't want them signed could build this themselves, but the officially published assemblies would be signed. See: nats-io/nats.net#291.

I'll take a look and test this out.

@jasper-d
Copy link
Contributor Author

Ok, I assumed strong naming was introduced by accident. Reverted it.

Let me know if you have any questions.

@ColinSullivan1
Copy link
Member

Thanks for being patient, I had to setup a linux env to test this. LGTM, and thank you for contribution! Much appreciated!

@ColinSullivan1 ColinSullivan1 merged commit c50d7f6 into nats-io:master Jan 21, 2020
@jasper-d
Copy link
Contributor Author

@ColinSullivan1 Thanks for merging, didn't expect it that quickly :)

We could add Linux and MacOS to the Azure Devops build matrix to add some basic verification for future contributions.

@ColinSullivan1
Copy link
Member

That's a good idea.

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.

3 participants