Run ILC after ComputeResolvedFilesToPublishList#111514
Run ILC after ComputeResolvedFilesToPublishList#111514sbomer wants to merge 12 commits intodotnet:mainfrom
Conversation
ghost
added
the
|
Did you consider using the scheme that ILLink/R2R/SingleFile use right now? It feels like it would make the sequencing most "standard". We'd probably need to wait for the new SDK to flow to this repo but we just did an upgrade and we could do another one. |
|
This moves us in that direction - now this target will be sequenced appropriately - and we can change from AfterTargets to DependsOnTargets in a later change. I'm looking at the native aot targets all-up to see if there's other logic that needs cleanup, but wanted to start with this piece. |
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Show resolved
Hide resolved
|
(The CI failures look related) |
|
/azp run runtime-ioslike |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@ivanpovazan is there going to be any work required in the iOS SDK as a result of this change? |
No, because we opt-out from NativeAOT Publish targets and handle them differently: |
|
/azp run runtime-androidemulator |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…sification Content items that happen to be managed DLLs (e.g. TestAssembly.dll referenced with OutputItemType=Content) were incorrectly classified as managed assemblies to compile and removed from publish output. Use the SDK's PostprocessAssembly metadata to only classify files the SDK has identified as runtime assemblies, and add ReferenceOutputAssembly=false to the FileVersionInfo test so the SDK doesn't treat it as one.
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-android |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
I think this is ready to go - runtime-extra-platforms failures look unrelated (fixing some of them in #124582). PTAL @ivanpovazan @MichalStrehovsky @agocke |
| --> | ||
| <Target Name="ComputeLinkedFilesToPublish" | ||
| BeforeTargets="ComputeResolvedFilesToPublishList" | ||
| AfterTargets="ComputeResolvedFilesToPublishList" |
There was a problem hiding this comment.
I think this makes sense in the existing targets, but eventually we want to move into the new CompileNative target that's been provided. Do you know if this is going to work there? Or are we going to need significant surgery for that change?
There was a problem hiding this comment.
Yup, this moves us in the right direction - this logic should run after ComputeLinkedFilesToPublish which is also where the new CompileNative target is. This change simplifies the work that will be needed to use the new target as a hook.
|
Can you also just run this locally and eyeball the target flow in the MSBuild binlog? Not to say that I don't trust CI... but I don't trust CI. Maybe building the runtime with this change is good enough? |
|
Target ordering matches my expectations: Before: |
6 participants
Fixes #108909
Needs more testing, but seems to work with a small repro so far.
Fixes #118298 (removes CopyNativeBinary logic)