-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Use live apphost when publishing ilc as singlefile #105004
base: main
Are you sure you want to change the base?
Changes from 26 commits
8e244d6
93c6d88
b00efad
351856c
46b01fd
b90b7fd
97a0686
5d7dba3
d46a6fd
713e9ec
8be0a6a
4684361
e5471c8
5903321
1b81367
ea14512
2da74d3
e096e4a
1b9948c
8d81ab4
a59b66a
fc810a3
e2344a9
d376752
ba71b35
4b0df9f
bd6549a
011280d
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 |
---|---|---|
|
@@ -12,9 +12,11 @@ | |
<GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles> | ||
<EnableDefaultEmbeddedResourceItems>false</EnableDefaultEmbeddedResourceItems> | ||
<Configurations>Debug;Release;Checked</Configurations> | ||
<UseLocalTargetingRuntimePack Condition="'$(StageTwoBuild)' == 'true'">true</UseLocalTargetingRuntimePack> | ||
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. Is this a problem for incremental builds? Should the build with LKG target and the build local targeting pack go into separate directories? 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. This project is not built in stage 1, so incremental build does not come into play. |
||
</PropertyGroup> | ||
|
||
<Import Project="../AotCompilerCommon.props" /> | ||
<Import Project="$(RepositoryEngineeringDir)targetingpacks.targets" Condition="'$(StageTwoBuild)' == 'true'" /> | ||
|
||
<PropertyGroup> | ||
<SelfContained>true</SelfContained> | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -15,9 +15,11 @@ | |||
<EnableDefaultEmbeddedResourceItems>false</EnableDefaultEmbeddedResourceItems> | ||||
<Configurations>Debug;Release;Checked</Configurations> | ||||
<RunAnalyzers>false</RunAnalyzers> | ||||
<UseLocalTargetingRuntimePack Condition="'$(StageTwoBuild)' == 'true'">true</UseLocalTargetingRuntimePack> | ||||
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 this be in 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. We can try :)
Suggested change
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. Unfortunately, it is giving errors like We will need to add tracking. targetingpacks.targets is used in 29 places. I'm leaving it as is for now. 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. A future change should import targetingpacks.targets in the repo root un-conditionally and enable the features in it via feature knobs. I should have done that in the first place back when I introduced the file. |
||||
</PropertyGroup> | ||||
|
||||
<Import Project="../AotCompilerCommon.props" /> | ||||
<Import Project="$(RepositoryEngineeringDir)targetingpacks.targets" Condition="'$(StageTwoBuild)' == 'true'" /> | ||||
|
||||
<ItemGroup Label="Embedded Resources"> | ||||
<EmbeddedResource Include="Properties\Resources.resx"> | ||||
|
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.
@jkoritzinsky
ILCompiler_publish.csproj
is not listed in this file (or anywhere else). Was it an oversight or something for the future?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 think there was an intent to hook it up for the
Microsoft.DotNet.ILCompiler
package, but I think it didn't quite happen. Looks like we still publish from theILCompiler.csproj
project and ship that. I think the build needs to be adjusted to hook this up correctly (useILCompiler_publish.csproj
to publish ilc and remove the publishing logic fromILCompiler.csproj
)