Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix building the host-targeting components and packing ILC #111552

Merged
merged 5 commits into from
Jan 17, 2025
Merged

Fix building the host-targeting components and packing ILC #111552

merged 5 commits into from
Jan 17, 2025

Conversation

jkoritzinsky
Copy link
Member

NETCoreSdkRuntimeIdentifier isn't set until after the Microsoft.NET.Sdk's Sdk.props file has been imported fully. Use the "after Sdk props" hook to set the "should build host tools" property so project files and item groups in Subsets.props can see the correct property value.

Also change how we publish ILCompiler to better follow the model of crossgen2. This restores the expected behavior of publishing an AOT or single-file ilc.

Fixes failures in dotnet/sdk#45990

@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

@jkoritzinsky jkoritzinsky mentioned this pull request Jan 17, 2025
Merged
ViktorHofer
ViktorHofer reviewed Jan 17, 2025
View reviewed changes
Directory.Build.props Outdated Show resolved Hide resolved
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this pull request Jan 17, 2025
@jkoritzinsky
Restructure based on dotnet#111552
8f72b07
@am11
Copy link
Member

am11 commented Jan 17, 2025 •
edited
Loading

We need these files on linux-x64:

image

With this PR (and main), we have 201 files in tools/ including the whole sfx.
image

@jkoritzinsky
Copy link
Member Author

@am11 where are you seeing those files being added? I'm checking the CI builds in this PR and it looks like I fixed that in 256edb9.

If this was checked locally, did you clean your build? The artifacts would have been left there from before the fix and caused issues.

@jkoritzinsky
Copy link
Member Author

Yeah I've checked all of the binlogs and they're doing the right thing (no sfx in the tools dir). I'm going to merge this in.

@jkoritzinsky jkoritzinsky merged commit 6c58f79 into dotnet:main Jan 17, 2025
145 of 147 checks passed
@jkoritzinsky jkoritzinsky deleted the import-ordering branch January 17, 2025 22:39
@build-analysis build-analysis bot mentioned this pull request Jan 17, 2025
Open
3 tasks
@am11
Copy link
Member

am11 commented Jan 17, 2025

I was building it in GHA but it was using main branch https://github.com/am11/CrossRepoCITesting/releases/tag/linux_x64_12836857446. The PR branch did removed the extra files https://github.com/am11/CrossRepoCITesting/releases/tag/linux_x64_12837954845. Sorry for the confusion. Good fix! 👍

@am11
Copy link
Member

am11 commented Jan 18, 2025

@jkoritzinsky, on community platforms, we get libs.native artifacts in tools/ dir of ILCompiler package (which is better than not having the whole sfx there, but not same plan as linux-x64): https://github.com/am11/CrossRepoCITesting/releases/tag/freebsd_12839104748

image

Is it because of linux conditions

runtime/src/installer/pkg/projects/Microsoft.DotNet.ILCompiler/Microsoft.DotNet.ILCompiler.pkgproj

Line 41 in 3998f4a

<ItemGroup Condition="'$(TargetOS)' == 'linux'">
or something which happens earlier? I also see them in ilc-published/ dir.

@jkoritzinsky
Copy link
Member Author

It's from whatever drops them in ilc-published. The packaging logic just takes everything from that folder and puts it in tools. I'm not sure what's dropping them there in the community platforms.

Maybe something due to the staged build logic?

@jkoritzinsky jkoritzinsky mentioned this pull request Jan 18, 2025
Closed
grendello added a commit to grendello/runtime that referenced this pull request Jan 20, 2025
@grendello
Merge branch 'main' into dev/grendel/android-build-with-ndk
41ce72b 
* main: (89 commits)
  Add Dispose for X509Chain instance (dotnet#110740)
  Fix XML comment on regex split enumerator (dotnet#111572)
  JIT: tolerate missing InitClass map in SPMI (dotnet#111555)
  Build ilasm/ildasm packages for the host machine (dotnet#111512)
  Unicode 16.0 Support (dotnet#111469)
  Improve performance of interface method resolution in ILC (dotnet#103066)
  Fix building the host-targeting components and packing ILC (dotnet#111552)
  Improve JSON validation perf (dotnet#111332)
  Update github-merge-flow.jsonc to autoflow 9.0 to 9.0-staging (dotnet#111549)
  Include GPL-3 licence text in the notice (dotnet#111528)
  Remove explicit __compact_unwind entries from x64 assembler (dotnet#111530)
  Add MemoryExtensions overloads with comparer (dotnet#110197)
  Avoid capturing the ExecutionContext for the whole HTTP connection lifetime (dotnet#111475)
  Forward DefaultArtifactVisibility down from the VMR orchestrator (dotnet#111513)
  Fix relocs errors on riscv64 (dotnet#111317)
  Added JITDUMP_USE_ARCH_TIMESTAMP support. (dotnet#111359)
  add rcl/rcr tp and latency info (dotnet#111442)
  Fix stack overflow in compiler-generated state (dotnet#109207)
  Produce a package with the host-running ILC for repos in the VMR (dotnet#111443)
  Delete dead code in ilasm PE writer (dotnet#111218)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@am11 am11 am11 left review comments

@ViktorHofer ViktorHofer ViktorHofer approved these changes

@agocke agocke Awaiting requested review from agocke

Assignees

@jkoritzinsky jkoritzinsky

Labels
area-Infrastructure
Projects
Runtime Infra
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jkoritzinsky @am11 @ViktorHofer