Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[release/3.0] Fix up the set of Windows compatibility libraries exposed in WindowsDesktop - #7372

Closed
dagood wants to merge 7 commits into
dotnet:release/3.0from
dagood:fix-wd-exposure-3.0
Closed

[release/3.0] Fix up the set of Windows compatibility libraries exposed in WindowsDesktop#7372
dagood wants to merge 7 commits into
dotnet:release/3.0from
dagood:fix-wd-exposure-3.0

Conversation

@dagood

@dagood dagood commented Jul 24, 2019

Copy link
Copy Markdown
Member

Port of #7324 to release/3.0. For https://github.com/dotnet/core-setup/issues/7290.

The WindowsBase version issue is not a problem in release/3.0. WindowsDesktop has:

WindowsBase.dll|Microsoft.WindowsDesktop.App.Runtime.win-x64|4.0.0.0|4.800.19.37306

and NETCoreApp has:

WindowsBase.dll|Microsoft.NETCore.App.Runtime.win-x64|4.0.0.0|4.700.19.37208

After the port, a new error exposed an issue where netcoreapp.depproj was building for win-x86, which doesn't match the set platform ,x64. In master, this issue didn't result in an error, maybe because the Arcade SDK version is different. I think it probably caused undesired behavior that just wasn't as easy to see. To fix this, I changed how PlatformManifestProjectReference works a bit:

  • Before: when windowsdesktop.depproj builds netcoreapp.depproj inside the RID-specific MSBuild call, the forced RuntimeIdentifier prop causes the netcoreapp.depproj build to fail.
  • Fix: I avoid doing this by instead using OrderProjectReference, which builds before the RID-specific package build, so the build runs with the default RuntimeIdentifier and it hits the MSBuild cache.

@dagood
dagood requested review from MichaelSimons and ericstj July 24, 2019 19:33
@dagood dagood self-assigned this Jul 24, 2019
@ericstj

ericstj commented Jul 24, 2019

Copy link
Copy Markdown
Member

Looks to me like something is still going wrong with fetching the platform manifest. Builds are failing because x86 isn't finding x64 manifest, and vice-versa.

Comment thread src/pkg/packaging-tools/packaging-tools.targets Outdated
Includes making depproj respect OrderProjectReferences, so that the windowsdesktop.depproj can cleanly depend on the platform manifest from netcoreapp.depproj.
@dagood

dagood commented Jul 25, 2019

Copy link
Copy Markdown
Member Author

List diffs: https://gist.github.com/dagood/c13bf4309bab2a76da0d066f04840738

Same as last time, but:

  • WindowsBase.dll isn't removed. 🎉

  • System.Drawing.dll isn't removed.

    • I think also 🎉 for the same reason, but it didn't come up before.
      System.Drawing.dll|Microsoft.NETCore.App.Runtime.win-x86|4.0.0.0|4.700.19.37208
      System.Drawing.dll|Microsoft.WindowsDesktop.App.Runtime.win-x86|4.0.0.0|4.800.19.37302
      

@dagood

dagood commented Jul 26, 2019

Copy link
Copy Markdown
Member Author

Reference for the WinForms version of System.Drawing.dll being intended to override CoreFX's, indicating that the above change (lack of removal) is good: dotnet/winforms#407.

dagood added a commit that referenced this pull request Jul 26, 2019
…ed in WindowsDesktop (#7372)

* Fix up compat libraries exposed in WindowsDesktop

* Exclude System.IO.Pipes.AccessControl runtime

* Keep System.Security.Permissions

* Use PackageConflictPlatformManifests for exclusion

* Update new versions to match current state

* Fix assumption that other archs' manifests built

* Better fix: one platform manifest for all

Includes making depproj respect OrderProjectReferences, so that the windowsdesktop.depproj can cleanly depend on the platform manifest from netcoreapp.depproj.

* Fix pre-merge TODO: sort dependencies and version props into main lists.
@dagood

dagood commented Jul 26, 2019

Copy link
Copy Markdown
Member Author

Merged by 5ebe419. (I resolved the TODOs in that commit.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants