-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Copy published crossgen2 in artifacts/tests #80154
Changes from 14 commits
948f3d8
472a2e4
2a98bc1
2687e51
7b8631e
2dec281
b9d3c25
98df15d
0458af4
f01dac6
2488e46
00689c8
f07d980
7c45ab8
7ccd4d8
0e059b3
1391809
31f6693
7f14b61
6563640
3683999
aece0d5
d4c5e6f
4ef28bc
f3dbd75
ea688c8
734199f
2a41ca0
f443875
a0bee39
39ea8f8
def8aa2
463213e
aa47096
9dc7214
84e1abe
0d2a34a
76d1b9e
f1331dc
feb1eaf
06637f5
93e78e9
804406f
a9e539b
9c64604
c1a6883
dd8d9bc
2065108
592865b
6481ee4
fcc3e9f
2324602
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
<CopyCoreDisToolsToCoreRoot Condition="$(GCStressDependsOnCoreDisTools) And '$(DotNetBuildSourceOnly)' != 'true'">true</CopyCoreDisToolsToCoreRoot> | ||
<!-- Non-desktop OS's use a custom dotnet host, instead of corerun --> | ||
<IsDesktopOS Condition="'$(TargetsBrowser)' != 'true' and '$(TargetsAndroid)' != 'true' and '$(TargetstvOS)' != 'true' and '$(TargetsiOS)' != 'true' and '$(TargetsMacCatalyst)' != 'true'">true</IsDesktopOS> | ||
<Crossgen2Supported Condition="'$(RuntimeFlavor)' == 'coreclr' and '$(TestBuildMode)' != 'nativeaot'">true</Crossgen2Supported> | ||
</PropertyGroup> | ||
|
||
<Import Project="$(RepositoryEngineeringDir)coredistools.targets" Condition="$(CopyCoreDisToolsToCoreRoot)" /> | ||
|
@@ -36,6 +37,14 @@ | |
<Target Name="CopyDependencyToCoreRoot" | ||
DependsOnTargets="ResolveAssemblyReferences;ResolveRuntimeFilesFromLocalBuild"> | ||
|
||
<!-- Publish crossgen2 on supported platforms. --> | ||
|
||
<MSBuild Condition="'$(Crossgen2Supported)' == 'true'" | ||
Targets="Restore;PublishToDisk" | ||
BuildInParallel="true" | ||
Properties="NativeAotSupported=$(NativeAotSupported);OutputPath=$(CORE_ROOT)\crossgen2;MSBuildRestoreSessionId=$([System.Guid]::NewGuid());" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you split this into two There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, as it will force a re-evaluation of the MSBuild project for the |
||
Projects="$(InstallerProjectRoot)pkg\sfx\Microsoft.NETCore.App\Microsoft.NETCore.App.Crossgen2.sfxproj" /> | ||
|
||
<ItemGroup> | ||
<RunTimeDependencyCopyLocal Include="@(RuntimeCopyLocalItems)" /> | ||
<RunTimeDependencyCopyLocal Include="@(NativeCopyLocalItems)" /> | ||
|
@@ -80,11 +89,6 @@ | |
<!-- Used by the Crossgen comparison job --> | ||
<RunTimeArtifactsIncludeFolders Include="IL/" TargetDir="IL/" /> | ||
|
||
<!-- Used for Crossgen2 R2R tests --> | ||
<RunTimeArtifactsIncludeFolders Include="crossgen2/" TargetDir="crossgen2/"> | ||
<IncludeSubFolders>True</IncludeSubFolders> | ||
</RunTimeArtifactsIncludeFolders> | ||
|
||
<!-- Used for NativeAOT tests --> | ||
<RunTimeArtifactsIncludeFolders Include="ilc-published/" TargetDir="ilc-published/"> | ||
<IncludeSubFolders>True</IncludeSubFolders> | ||
|
@@ -135,7 +139,7 @@ | |
|
||
<RunTimeDependencyCopyLocal | ||
Condition="'%(RuntimeArtifactsIncludeFolders.IncludeSubFolders)' == 'True'" | ||
Include="$(CoreCLRArtifactsPath)%(RunTimeArtifactsIncludeFolders.Identity)**/*" | ||
Include="$(CoreCLRArtifactsPath)%(RunTimeArtifactsIncludeFolders.Identity)**\*" | ||
Exclude="@(RunTimeArtifactsExcludeFiles -> '$(CoreCLRArtifactsPath)%(Identity)')" | ||
TargetDir="%(RunTimeArtifactsIncludeFolders.TargetDir)" /> | ||
</ItemGroup> | ||
|
@@ -223,7 +227,7 @@ | |
|
||
<Copy | ||
SourceFiles="@(RunTimeDependencyCopyLocal)" | ||
DestinationFiles="@(RunTimeDependencyCopyLocal -> '$(CORE_ROOT)/%(TargetDir)%(RecursiveDir)%(Filename)%(Extension)')" | ||
DestinationFiles="@(RunTimeDependencyCopyLocal -> '$(CORE_ROOT)\%(TargetDir)%(RecursiveDir)%(Filename)%(Extension)')" | ||
SkipUnchangedFiles="$(SkipCopyUnchangedFiles)" | ||
OverwriteReadOnlyFiles="$(OverwriteReadOnlyFiles)" | ||
Retries="$(CopyRetryCount)" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this line back up to the unconditional property group? I think this is what's causing the failures (as the local apphost pack can't be found).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reverting, next place where it looks for apphost is:
(from
Build coreclr Pri0 Runtime Tests Run linux x64 checked failed
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is crossgen2_publish NativeAOT'd here?
We just updated the repo to Preview 3 SDK that has this fix: dotnet/sdk#38644 - SDK should not even be looking for an apphost when doing NAOT publish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a few things but this looks like a deadlock.
error NETSDK1102: Optimizing assemblies for size is not supported for the selected publish configuration. Please ensure that you are publishing a self-contained app
error NETSDK1067: Self-contained applications are required to use the application host. Either set SelfContained to false or set UseAppHost to true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These problems look a lot like the kind of problems one gets when running the Publish target, but not setting the
_IsPublishing
property._IsPublishing
is set to true andPublishAot
orPublishTrimmed
orPublishSingleFile
is true._IsPublishing
andPublishTrimmed
/PublishAot
/etc.Running the Publish target without
dotnet publish
has a bunch of bad UX tracked in dotnet/sdk#26324.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this manual setup is likely there so crossgen2 is published with live build of ILCompiler when NativeAotSupported==true. ILCompiler itself is published using nuget package / sdk integration. crossgen2_publish.csproj line 4 is setting
_IsPublishing
. InvokingPublish
/PublishToDisk
directory gives the same missing apphost error. -v:diag dump suggests one of the linker/trimmer target is trying to copy apphost where it fails.