Skip to content

Make Runtime RID-agnostic and simplify infra#165

Merged
jkoritzinsky merged 2 commits into
SharpGenTools:masterfrom
andrew-boyarshin:the_phantom_menace
Apr 29, 2020
Merged

Make Runtime RID-agnostic and simplify infra#165
jkoritzinsky merged 2 commits into
SharpGenTools:masterfrom
andrew-boyarshin:the_phantom_menace

Conversation

@andrew-boyarshin

@andrew-boyarshin andrew-boyarshin commented Apr 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Reimplement RID-specific Runtime parts to be RID-independent
  • Use proper stack trace method on .NET Framework, fallback is only for .NET Standard 1.1
  • Use nightly Roslyn compiler for nint and nuint, used in new NativeLong/NativeULong implementation
  • Fix wrong data model for Native LONG types (SharpGen was correct, Runtime was not, IntPtr is on Unix, int is on Windows, not the other way around)
  • Fix bug in SharpGen MSBuild targets (it was impossible to override SharpGenSdkAssembly path)
  • Improve MSBuild scripts
  • Update dependencies
  • Use centralized dependencies version management
  • Make CI builds deterministic (still affected by current bug in .NET SDK)
  • Improved test and coverage scripts
  • Migrate to net472;netcoreapp2.1 (MSBuild requires net472;netcoreapp2.0, .NET Core 2.0 is EOL, 2.1 is the closest LTS)
  • Drop Refs since no RID-specific code is left in Runtime
  • Fine-tune Runtime dependency graph
  • Workaround MSBuild Core not loading Microsoft.Win32.Registry implementation dll (I suspect bug in MSBuild assembly load context override) by building RuntimeIdentifier-specific assemblies (win;unix)
  • Drop SharpGen.Interactive

Note, that I'm not sure nint and nuint work on all .NET platforms supported by Runtime, it would be great to have this confirmed by someone more knowledgeable than me. Roslyn specs don't detail on that either. Theoretically it should work, but I can't test it at the moment (still downloading Windows 8 RTM image 😄). Works on .NET Framework 4.5 inbox on Windows 8 RTM x64.

Code Coverage is still here by default (test.ps1), but the results are empty on CI.

@andrew-boyarshin
Make Runtime TFM/RID-agnostic and simplify infra
67c6b4a 
* Reimplement RID-specific Runtime parts to be RID-independent
* Fix wrong data model for Native LONG types
* Fix bug in SharpGen MSBuild targets (it was impossible to override `SharpGenSdkAssembly` path)
* Improve MSBuild scripts
* Update dependencies
* Use centralized dependencies version management
* Make CI builds deterministic
* Improved test and coverage scripts
* Migrate to `net472;netcoreapp2.1`
* Drop Refs since no RID-specific code is left in Runtime
* Fine-tune Runtime dependency graph
* Workaround MSBuild Core not loading `Microsoft.Win32.Registry` implementation dll by building RuntimeIdentifier-specific assemblies (`win;unix`).
@andrew-boyarshin andrew-boyarshin mentioned this pull request Apr 24, 2020
Closed
@andrew-boyarshin andrew-boyarshin changed the title Make Runtime TFM/RID-agnostic and simplify infra Make Runtime RID-agnostic and simplify infra Apr 24, 2020
jkoritzinsky
jkoritzinsky approved these changes Apr 24, 2020
View reviewed changes

@jkoritzinsky jkoritzinsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other than my one comment, looks good to me!

Comment thread SharpGen.Runtime/PlatformDetection.cs Outdated
@andrew-boyarshin

andrew-boyarshin commented Apr 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@jkoritzinsky I've confirmed that new nint/nuint-based NativeU?Long implementation works correctly on Windows 8 RTM (6.2.9200 x64, .NET Framework 4.5, 0 installed updates from Windows Update).

Found issues with many SdkTests not working on x86 (environment doesn't matter). Since the issue is present even before this PR, it's not related to changes here.

@andrew-boyarshin

andrew-boyarshin commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

@jkoritzinsky is there anything else you want done before merging this PR?

@jkoritzinsky

jkoritzinsky commented Apr 29, 2020

Copy link
Copy Markdown
Member

Nope! I just have been really busy and haven’t had a chance to merge this in.

@jkoritzinsky

jkoritzinsky commented Apr 29, 2020

Copy link
Copy Markdown
Member

Actually, can you fix the CodeFactor check?

@andrew-boyarshin

andrew-boyarshin commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

@jkoritzinsky I'm not proficient with PowerShell (I don't know correct Write-Host replacement). The issues are from the code I've moved, but not modified.

@jkoritzinsky

jkoritzinsky commented Apr 29, 2020

Copy link
Copy Markdown
Member

Ok those are fine. I’ll merge this in tomorrow when I’m at my computer.

@jkoritzinsky jkoritzinsky merged commit 001de3e into SharpGenTools:master Apr 29, 2020
@andrew-boyarshin andrew-boyarshin added this to the 2.0 milestone Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkoritzinsky jkoritzinsky jkoritzinsky approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@andrew-boyarshin @jkoritzinsky