Add Accessibility.dll to WinForms #593
Conversation
|
CI is failing because of this HelixPublish.proj ends up invoking the Build target on all the test projects and that rebuilds the product binaries. It appears to be rebuilding because configuration is not passed to the msbuild invocation: it ends up building debug in the release leg. It's also missing a number of other properties that are normally passed by build.cmd that could result in different results / failures. Here this is specifically failing because the binary build restored packages to a repo local .packages folder, but the HelixPublish.proj tries to use the user-profile packages folder (I believe because its missing the ContinuousIntegration property, but there could be others that are important) this leads to missing files and the failure. @zsd4yr do you think you can take a look at fixing or disabling the HelixPublish.proj leg? |
For now I will revert to running tests on build machines to unblock your work here. However, I am a little confused about why this is happening. I will start a thread with you, me, and @alexperovich to investigate |
|
@ericstj you should be unblocked now |
This is a primary interop assembly for OLEACC.dll. It's needed because both Winforms and WPF expose public types that depend on the COM interop types in OLEACC so they cannot embed the types.
Calling this target directly can cause it to run out of order. This was happening when the pkg project built before source, and was causing CreateManifestResourceNames to remove all EmbeddedResources here https://github.com/Microsoft/msbuild/blob/d42d3504057ef2b88dd4f68c4bfc5591371bd6fe/src/Tasks/Microsoft.CSharp.CurrentVersion.targets#L121 because we hadn't run the targets that set %(EmbeddedResource.Type). Instead of doing this depend on the BuildOutputGroup. Output groups are meant to be safe to call in isolation, so they should always fully specify their target dependency chain to produce the correct items without modifying the project state.
And remove workaround for ILProj loading in VS.
|
Build is failing because we don't (can't) have a PDB. Need to suppress that check... |
|
... and previous build succeeded because of MSBuild bug with aftertargets. dotnet/arcade#2267 |
Missing PDB was resulting in a failure during builds when attempting to push symbols.
Codecov Report
@@ Coverage Diff @@
## master #593 +/- ##
=========================================
Coverage ? 27.97%
=========================================
Files ? 903
Lines ? 243518
Branches ? 32091
=========================================
Hits ? 68136
Misses ? 171562
Partials ? 3820
Continue to review full report at Codecov.
|
|
Looks like this is passing now. Can get review / signoff / merge? Please let me know if you'd like any changes. |
|
It would be good to merge this before any of the darc PRs so that it doesn't hit a conflict on global.json/version files. /cc @zsd4yr @AdamYoblick @Shyam-Gupta |
|
FYI folks, when we pick this up downstream we'll need to delete the old Accessibility package and use this instead. Feel free to ping me to help out on any of this. /cc @rladuca @vatsan-madhavan @stevenbrix @dagood |
|
Is this going into the Winforms transport package, instead of a separate Accessiblity transport package? Having a separate transport package would be better- we can then reference it from PresentationFramework, UIAutomationTypes and WindowsBase. Having to reference the full WinForms transport package from those projects just means that either we'll have to deal with potential side-effects due to namespace collisions, or have to filter out other references in a target prior to consumption. I can work with it either way, but would be nice to preserve existing design (separate trasnport package). /cc @dotnet/wpf-developers |
|
You don’t need a target. Reference with ExcludeAssets=All GeneratePathProperty=true then you can manually pull Accessibility from package using the property nuget writes to the obj/*props file. We don’t want Accessibility to have it’s own package since it isn’t shipping standalone. The individual packages was just a side effect of my initial prototype. |
|
Here's a sample of what I am talking about: <ItemGroup>
<PackageReference Include="Microsoft.Private.Winforms" Version="4.8.0-preview4.19171.1" ExcludeAssets="All" GeneratePathProperty="True" />
<Reference Include="$(PkgMicrosoft_Private_Winforms)\lib\netcoreapp3.0\Accessibility.dll" />
</ItemGroup>This worked fine for me, let me know if it gives you any issues. |
|
@ericstj Thanks - that should work! |
ghost
locked as resolved and limited conversation to collaborators
This is a primary interop assembly for OLEACC.dll.
It's needed because both Winforms and WPF expose public types that depend on the
COM interop types in OLEACC so they cannot embed the types.