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

WIP: Migrate to new csproj VS2017+ #66

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pluskal
Copy link

@pluskal pluskal commented Nov 12, 2019

Changing csproj to new version VS2017+ as a prerequisite for Adding async support

AssemblyInfo is no longer required, info moved to csproj.
Csproj do not require listing files so template is also removed.
Adding copy phase do csharp_builder.rb so copmpiled sources are copied with CI to test project destination.

Note that project cannot be compiled, same as the master, see #644 for details. After manual resolution of compile errors it seems ok.

I hope to follow with multi-targeting and .net core tests after kaitai_struct_csharp_runtime builds for .net core.

AssemblyInfo is no longer required, info moved to csproj. Csproj do not require listing files so template is also removed.
builder/csharp_builder.rb Outdated Show resolved Hide resolved
@GreyCat
Copy link
Member

GreyCat commented Nov 13, 2019

I also suspect that this won't work with ancient mono's xbuild?

@pluskal
Copy link
Author

pluskal commented Nov 13, 2019

I also suspect that this won't work with ancient mono's xbuild?

I am not sure about this. I noticed, that xbuild is deprecated according to logs in CI.

�[0K$ ./ci-$TARGET$SUBTARGET
#### CSharpBuilder: initialized
XBuild Engine Version 14.0
Mono, Version 5.10.1.20
Copyright (C) 2005-2013 Various Mono authors
Mono JIT compiler version 5.10.1.20 (tarball Thu Mar 29 10:48:35 UTC 2018)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
	TLS:           __thread
	SIGSEGV:       altstack
	Notifications: epoll
	Architecture:  amd64
	Disabled:      none
	Misc:          softdebug 
	Interpreter:   yes
	LLVM:          supported, not enabled.
	GC:            sgen (concurrent by default)
#### CSharpBuilder: creating project with 339/339 files
#### CSharpBuilder: project file created: "spec/csharp/kaitai_struct_csharp_tests/kaitai_struct_csharp_tests.csproj"
#### CSharpBuilder: build attempt 1 (log: test_out/csharp/build-1.log)
#### CSharpBuilder: running command: ["xbuild", "spec/csharp/kaitai_struct_csharp_tests/kaitai_struct_csharp_tests.csproj"], log: "test_out/csharp/build-1.log"

>>>> xbuild tool is deprecated and will be removed in future updates, use msbuild instead <<<<

What about to switch to msbuild as the devs suggest?

@GreyCat
Copy link
Member

GreyCat commented Nov 15, 2019

There is no msbuild in mono packages, so I would assume that it means it needs to be download and installed separately. I'm 95% sure that it's not just a matter of swapping xbuild => msbuild — likely we'll need to configure msbuild somehow to teach it that we have this installation of mono here and this is what we want to use to compile these .cs files.

builder/csharp_builder.rb Outdated Show resolved Hide resolved
builder/csharp_builder.rb Outdated Show resolved Hide resolved
spec/csharp/.gitignore Outdated Show resolved Hide resolved
@pluskal
Copy link
Author

pluskal commented Nov 15, 2019

There is no msbuild in mono packages, so I would assume that it means it needs to be download and installed separately. I'm 95% sure that it's not just a matter of swapping xbuild => msbuild — likely we'll need to configure msbuild somehow to teach it that we have this installation of mono here and this is what we want to use to compile these .cs files.

I'll try to look at this more closely. It seems that some distributions contain mono/msbuild package, according to xamarin.github.io/bugzilla-archives.

@GreyCat
Copy link
Member

GreyCat commented Nov 17, 2019

Actually, we've already broken mono/xbuild builds with kaitai-io/kaitai_struct_csharp_runtime#15:

Errors:
/home/travis/build/kaitai-io/ci_targets/tests/spec/csharp/kaitai_struct_csharp_tests/kaitai_struct_csharp_tests.csproj (default targets) ->
/usr/lib/mono/xbuild/14.0/bin/Microsoft.Common.targets (ResolveProjectReferences target) ->
	/home/travis/build/kaitai-io/ci_targets/runtime/csharp/kaitai_struct_runtime_csharp.csproj: error : /home/travis/build/kaitai-io/ci_targets/runtime/csharp/kaitai_struct_runtime_csharp.csproj: The default XML namespace of the project must be the MSBuild XML namespace. If the project is authored in the MSBuild 2003 format, please add xmlns="http://schemas.microsoft.com/developer/msbuild/2003" to the <Project> element. If the project has been authored in the old 1.0 or 1.2 format, please convert it to MSBuild 2003 format.

@pluskal pluskal requested a review from GreyCat January 2, 2020 08:52
@pluskal pluskal changed the title Migrate to new csproj VS2017+ WIP: Migrate to new csproj VS2017+ Jan 2, 2020
@pluskal
Copy link
Author

pluskal commented Jan 2, 2020

I tried to rebase this PR to the current master and I am not sure about updating the current spec.

What do you think (@GreyCat) about keeping the current csharp spec for mono/.net framework and creating a new for .NET core?

I tried to rebase this PR to current master and test it with msbuild, xbuild and windows with netcli. It requires a lot of if/else path exists checks because of new csproj format and packages.config stores packages in different locations and with different file structure (sln/packages/NUnit.ConsoleRunner.3.4.1/tools/nunit3-console.exe) vs (user profile/.nuget/packages/NUnit.ConsoleRunner/3.4.1). And to make the new csproj work with MSBuild, it requires to run msbuild -t:restore before the actual MSBuild run.

With my aim to add async support, kaitai-io/kaitai_struct#640 I would need to increase TargetFramework to netstandard2.0 in Kaitai Struct Csharp Runtime. Netstandard2.0 is not compatible with netframework4.5 but only with netframework4.6.1+.

I would propose to add a new project to Kaitai Struct Csharp Runtime containing async runtime.

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.

2 participants