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.

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

Include hostcommon sources in hostfxr static library #75858

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

rseanhall
Copy link
Contributor

The hostfxr static library can't be used without this.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Sep 19, 2022
@ghost
Copy link

ghost commented Sep 19, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

The hostfxr static library can't be used without this.

Author: rseanhall
Assignees: -
Labels:

area-Host

Milestone: -

@rseanhall
Copy link
Contributor Author

I don't know if there's anything that can be done with the build process, but it was harder than I would like to build this change. All I wanted was to build the Microsoft.NETCore.DotNetAppHost packages so I tried to run build -subset host. This failed with:

  InjectResource -> C:\src\dotnet-runtime\artifacts\bin\coreclr\windows.x64.Debug\InjectResource\InjectResource.dll
C:\src\dotnet-runtime\src\native\corehost\corehost.proj(48,5): error MSB3030: Could not copy the file "C:\src\dotnet-ru
ntime\artifacts\bin\coreclr\windows.x64.Debug\/corehost/singlefilehost.exe" because it was not found.

I ended up copying a dummy singlefilehost.exe into the right place and disabling the PrepareSingleFileHostWithEmbeddedDac target because I didn't want to build anything else.

@jkotas
Copy link
Member

jkotas commented Sep 19, 2022

Microsoft.NETCore.DotNetAppHost package is bundled into the SDK and installed under c:\Program Files\dotnet\packs\Microsoft.NETCore.App.Host.win-x64. We are adding more stuff to it for niche scenarios that do not look like a good fit for default .NET SDK download.

Do we need to rethink the package structure?

@rseanhall
Copy link
Contributor Author

@jkotas That is not what I see. I added libhostfxr.lib to the package in .NET 6 and I don't see that file in the path you provided for .NET 6.0.8 or .NET 7 RC1.

Sometime later, the code was refactored and parts of hostfxr were moved into the hostcommon static lib. This is just adding the missing code. I would be fine with a custom host package, but it seems like changing the distribution of these assets seems to be low priority for everyone.

@jkotas
Copy link
Member

jkotas commented Sep 20, 2022

@jkotas That is not what I see. I added libhostfxr.lib to the package in .NET 6 and I don't see that file in the path you provided for .NET 6.0.8 or .NET 7 RC1.

Ok, I got confused by the two very similar packages with overlapping content: Microsoft.NETCore.App.Host (bundled into SDK) and Microsoft.NETCore.DotNetAppHost (not bundled into SDK). This change is only adding this to Microsoft.NETCore.DotNetAppHost so my concern about SDK download size is moot.

@vitek-karas For my education: Why do have so much stuff in the package that is bundled into the SDK - how are we deciding what should go into each of these two package?

@jkotas
Copy link
Member

jkotas commented Sep 20, 2022

BTW: A solution that was implemented in #72896 for similar problem was to ship the relevant C/C++ sources in the nuget package. I expect we will prefer shipping C/C++ sources for these situations going forward. It avoids the problems with fragility of C/C++ .libs and compiler settings.

@rseanhall rseanhall changed the title Publish hostcommon static library Include hostcommon sources in hostfxr static library Oct 10, 2022
@build-analysis build-analysis bot mentioned this pull request Oct 10, 2022
2 tasks
@rseanhall
Copy link
Contributor Author

runtime (Build Build OSX x64 release NativeAOT) failure
Building tests via "/Users/runner/work/1/s/eng/common/msbuild.sh"  --warnAsError false /Users/runner/work/1/s/src/tests/build.proj /t:TestBuild /p:TargetArchitecture=x64 /p:Configuration=Release /p:TargetOS=OSX /nodeReuse:false    /maxcpucount "/flp:Verbosity=normal;LogFile=/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.log" "/flp1:WarningsOnly;LogFile=/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.wrn" "/flp2:ErrorsOnly;LogFile=/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.err" "/bl:/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.binlog" /p:NUMBER_OF_PROCESSORS=4
/Users/runner/work/1/s/.dotnet/sdk/7.0.100-rc.1.22431.12/MSBuild.dll /nologo -maxcpucount /m /maxcpucount -verbosity:m /v:minimal /bl:/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.binlog /clp:Summary /flp1:WarningsOnly;LogFile=/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.wrn /flp2:ErrorsOnly;LogFile=/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.err /flp:Verbosity=normal;LogFile=/Users/runner/work/1/s/artifacts/log/TestBuild.OSX.x64.Release.log /nr:true /nodeReuse:false /p:TreatWarningsAsErrors=false /p:ContinuousIntegrationBuild=false /p:TargetArchitecture=x64 /p:Configuration=Release /p:TargetOS=OSX /p:NUMBER_OF_PROCESSORS=4 /t:TestBuild /Users/runner/work/1/s/src/tests/build.proj
  [17:10:30.75] Restoring all packages...
    Determining projects to restore...
    Restored /Users/runner/work/1/s/src/tests/Common/test_dependencies_fs/test_dependencies.fsproj (in 30.52 sec).
    Determining projects to restore...
    Restored /Users/runner/work/1/s/src/tests/Common/test_dependencies/test_dependencies.csproj (in 10.14 sec).
/Users/runner/work/1/s/src/tests/build.proj(437,5): error MSB3073: The command ""/Users/runner/work/1/s/.dotnet/dotnet" restore -r osx-x64 Common/test_dependencies/test_dependencies.csproj  /p:SetTFMForRestore=true /p:TargetOS=OSX /p:TargetArchitecture=x64 /p:Configuration=Release /p:CrossBuild=" exited with code 1.

Build FAILED.

This is #74328?

runtime (Build mono llvmfullaot Pri0 Runtime Tests Run Linux x64 release) failure
2022-10-10T17:37:44.1809505Z /__w/1/s/src/mono/msbuild/aot-compile.proj(19,9): error MSB3073: The command "/__w/1/s/artifacts/tests/coreclr/Linux.x64.Release/Tests/Core_Root/corerun /__w/1/s/artifacts/tests/coreclr/Linux.x64.Release/Tests/Core_Root/xunit.execution.dotnet.dll" exited with code 255.
2022-10-10T17:37:44.1810593Z BEGIN : coreclr_initialize failed - error : 0x80004005 [/__w/1/s/src/mono/msbuild/aot-compile.proj]
2022-10-10T17:37:44.1811488Z END : coreclr_initialize failed - error : 0x80004005 [/__w/1/s/src/mono/msbuild/aot-compile.proj]

I don't think this was caused by my change. If it were, I would expect the arm64 build to have failed as well.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkotas
Copy link
Member

jkotas commented Oct 11, 2022

@vitek-karas @elinor-fung Any additional feedback?

Copy link
Member

@elinor-fung elinor-fung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @rseanhall!

@elinor-fung
Copy link
Member

runtime (Build mono llvmfullaot Pri0 Runtime Tests Run Linux x64 release) failure

2022-10-10T17:37:44.1809505Z /__w/1/s/src/mono/msbuild/aot-compile.proj(19,9): error MSB3073: The command "/__w/1/s/artifacts/tests/coreclr/Linux.x64.Release/Tests/Core_Root/corerun /__w/1/s/artifacts/tests/coreclr/Linux.x64.Release/Tests/Core_Root/xunit.execution.dotnet.dll" exited with code 255.
2022-10-10T17:37:44.1810593Z BEGIN : coreclr_initialize failed - error : 0x80004005 [/__w/1/s/src/mono/msbuild/aot-compile.proj]
2022-10-10T17:37:44.1811488Z END : coreclr_initialize failed - error : 0x80004005 [/__w/1/s/src/mono/msbuild/aot-compile.proj]

I don't think this was caused by my change. If it were, I would expect the arm64 build to have failed as well.

I think this is #72114.

@elinor-fung elinor-fung merged commit 7c4bd4f into dotnet:main Oct 13, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Host community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants