Skip to content

Conversation

@akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Jun 16, 2020

Essentially reverts #31850 and fixes the underlying issue, which was that alpine-arm64 was missing from the RID graph.

This allows us to switch to PackageReference for SqlClient as the old approach doesn't work for mobile targets where we do self-contained publishing since we don't have the SqlClient dll in the in-tree runtime pack (and we don't want to put it there).
While working on this I also removed some unnecessary duplication between Mono and CoreCLR in runtime.depproj.

Also bumps Microsoft.DotNet.ProjectModel version in Microsoft.Extensions.DependencyModel.Tests.csproj

The older version references a non-existing assembly "System.Runtime.InteropServices.Pinvoke".

@ghost
Copy link

ghost commented Jun 16, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@akoeplinger akoeplinger force-pushed the sqlclient-packagereference branch from 6690aca to c89bf70 Compare June 16, 2020 15:00
@akoeplinger akoeplinger marked this pull request as ready for review June 16, 2020 17:24
It was missing and caused a wrong RID graph to be generated.
The old approach doesn't work for mobile targets where we do self-contained publishing since we don't have the SqlClient dll in the in-tree runtime pack (and we don't want to put it there).
While working on this I also removed some unnecessary duplication between Mono and CoreCLR in runtime.depproj.
…pendencyModel.Tests.csproj

The older version references a non-existing assembly "System.Runtime.InteropServices.Pinvoke".
@akoeplinger akoeplinger force-pushed the sqlclient-packagereference branch from eaa2874 to 4b402d9 Compare June 16, 2020 21:42
@akoeplinger akoeplinger changed the title Use PackageReference for System.Data.SqlClient in tests again Add alpine-arm64 to RID graph and use PackageReference for System.Data.SqlClient in tests again Jun 16, 2020
@ViktorHofer
Copy link
Member

@ericstj @wfurt can you please take a look at the rid change. Shouldn't prevent us from merging meanwhile though.

@akoeplinger
Copy link
Member Author

I talked to @ericstj about this yesterday and he helped me track down the issue :)

The test failures are due to #37946 and an issue with the Ubuntu.1804.Amd64.Open queue that I reported on Teams.

The important parts are the Alpine arm64 jobs which are green, merging.

@akoeplinger akoeplinger merged commit 0132592 into dotnet:master Jun 17, 2020
@akoeplinger akoeplinger deleted the sqlclient-packagereference branch June 17, 2020 13:16
<RuntimeGroup Include="alpine">
<Parent>linux-musl</Parent>
<Architectures>x64</Architectures>
<Architectures>x64;arm64</Architectures>
Copy link
Member

Choose a reason for hiding this comment

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

Do all these versions support arm64? If not we could create a separate group for arm64.

Copy link
Member Author

@akoeplinger akoeplinger Jun 17, 2020

Choose a reason for hiding this comment

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

Yes, all the version back to our minimum 3.6 support arm64.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants