Propagate native references to merged project wrapper outputs#65592
Propagate native references to merged project wrapper outputs#65592trylek merged 2 commits intodotnet:mainfrom
Conversation
When building tests in merged mode, we need to copy the native references along with the component test dlls to the output folder to make them discoverable by the OS loader. Thanks Tomas
|
Tagging subscribers to this area: @hoyosjs Issue DetailsWhen building tests in merged mode, we need to copy the native Thanks Tomas
|
jkoritzinsky
left a comment
There was a problem hiding this comment.
I think I'd prefer that we'd rather hook into the GetCopyToOutputDirectoryItems infrastructure instead of this design, but I'm okay with this design for now as it's simple and will get us off the ground.
src/tests/Directory.Build.targets
Outdated
| <PropertyGroup> | ||
| <CopyMergedWrapperReferencesHasRun>true</CopyMergedWrapperReferencesHasRun> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
It doesn't look like this property is used. Do we need to set it?
| <PropertyGroup> | |
| <CopyMergedWrapperReferencesHasRun>true</CopyMergedWrapperReferencesHasRun> | |
| </PropertyGroup> |
There was a problem hiding this comment.
Thanks Jeremy for spotting, I have removed the superfluous property I originally used as local instrumentation when debugging the change in 2nd commit on this PR.
There was a problem hiding this comment.
If you prefer triggering this action off another build target or rolling in into an existing one, I have no problems with it whatsoever, I have actually only started testing this change on a larger scale and I have mostly randomly put the code next to the existing logic for publishing native product items.
There was a problem hiding this comment.
(Just for context, this accounts for the two remaining failures apart from the six ones caused by the ilproj rename logic.)
There was a problem hiding this comment.
I think leaving it as-is for this change just to get us off the ground is reasonable.
We have a lot of complexity around how we support copying the native binaries around (due to the fact that we can do the copy as a separate step from the build), so I'm fine holding off doing a more invasive change and just getting in something simple to unblock.
There was a problem hiding this comment.
Sounds good to me, thanks for clarifying. I originally tried to add this logic to CopyNativeProjectBinaries but that's also a Pandora's box including the special casing for the VS. We can / should tackle this later but hopefully it's not detrimental towards our current efforts.
When building tests in merged mode, we need to copy the native
references along with the component test dlls to the output folder
to make them discoverable by the OS loader.
Thanks
Tomas