-
Notifications
You must be signed in to change notification settings - Fork 35
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
PublishContainer Appends "dotnet <Target>.dll" to ENTRYPOINT for NativeAOT Builds #559
Comments
Oof, good spot. |
You can work around this by overriding the arguments, right? And what version of the SDK/package are you using? |
Since I don't currently have applications which use the command line arguments, it's harmless to me. I only noticed it while trying to diagnose something else. I'm sorry, that completely slipped my mind. I'm using SDK 8.0.203. (Updated OP.) |
I'm a rank amateur at MSBuild, but I dove into the SDK repo, and I think the issue happens here: <ItemGroup Label="AppCommand Assignment" Condition="'$(ContainerAppCommandInstruction)' != 'None'">
<!-- For self-contained, invoke the native executable as a single arg -->
<ContainerAppCommand Condition="'$(ContainerAppCommand)' == '' and '$(_ContainerIsSelfContained)' == 'true' and !$(_ContainerIsTargetingWindows)" Include="$(ContainerWorkingDirectory)/$(AssemblyName)$(_NativeExecutableExtension)" />
<ContainerAppCommand Condition="'$(ContainerAppCommand)' == '' and '$(_ContainerIsSelfContained)' == 'true' and $(_ContainerIsTargetingWindows)" Include="$(AssemblyName)$(_NativeExecutableExtension)" />
<!-- For non self-contained, invoke `dotnet` `app.dll` as separate args -->
<ContainerAppCommand Condition="'$(ContainerAppCommand)' == ''" Include="dotnet;$(TargetFileName)" />
</ItemGroup> …because the third <ContainerAppCommand Condition="'$(ContainerAppCommand)' == '' and '$(_ContainerIsSelfContained)' == 'false'" Include="dotnet;$(TargetFileName)" /> Does that seem right? And if so, would Microsoft accept a PR like this to the SDK repo from outside the org. – that is, from me? |
First off, we would definitely accept a PR! We love PRs in this repo :) The containers work specifically had been highly driven by the community. I agree with your analysis of where the issue is being introduced, but I'm going to tag in @tmds as well because the details of this are a bit fuzzy. This area was changed in dotnet/sdk#33037, but I think it makes complete sense that we should not use the |
Yes, the problem is here. This was written up as: |
It regressed in dotnet/sdk#34863. |
Looks like you guys beat me to this by 2 days 😆 - just hit this when trying to migrate from using a Dockerfile to using I've never contributed to this repo before. Have you submitted a PR already, @tmds ? I don't see any in the links here, but you might have either way. If you're already on the case, I can just step quietly back into the corner, but please, if I can help in fixing this, could someone please give me a small clue on where to start? |
@erikbra the actual code for the SDK Containers feature does live in dotnet/sdk, as you saw - that repo has a developer guide here that you can use to build the repo. The code change itself I would expect to be pretty small, and I'd want to see tests that cover the MSBuild logic in the TargetsTests file. The test I linked to show the usage of the older ContainerEntrypoint MSBuild Item, but it should be doable to create matching tests for ContainerAppCommand. The |
Thanks, @baronfel - I'll see if I can take a shot at it (but knowing myself, don't hold your breath all of you ;) ) |
There we go, my attempt. It seems to solve the issue, but I'm a little worried, because the existing test, that tested on ContainerEntryPoint wasn't actually testing anything, due to a bug in the test. The test should possibly be fixed as well, and back-ported to the .NET 7 branch, as we don't set the ContainerEntryPoint anymore on .NET8+. I don't know if you should bother. Here is the PR in the dotnet/sdk repo: #40792 |
… <Target>.dll" to ENTRYPOINT for Self Contained Builds (#40792)
When publishing a natively compiled application:
…without customizing the entry point or command, the produced image has an
Entrypoint
which contains extra parameters:➜ docker inspect -f '{{ .Config.Entrypoint }}' success:latest [/app/Success dotnet Success.dll]
I'd expect this to be only
[/app/Success]
. Since this is NativeAOT compiled, the publish process has chosenmcr.microsoft.com/dotnet/nightly/runtime-deps:8.0-jammy-chiseled-aot
as the base image. There's nodotnet
in this base image, and using the string "dotnet" as an argument doesn't seem meaningful.SDK Version: 2.0.203
Operating System: macOS Sonoma 14.3
But I'm running these commands in: mcr.microsoft.com/dotnet/nightly/sdk:8.0-jammy-aot
The text was updated successfully, but these errors were encountered: