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

Bug dotnet/sdk-container-builds#559: PublishContainer Appends "dotnet <Target>.dll" to ENTRYPOINT for Self Contained Builds #40792

Merged

Conversation

erikbra
Copy link
Contributor

@erikbra erikbra commented May 9, 2024

Bug dotnet/sdk-container-builds#559: PublishContainer Appends "dotnet .dll" to ENTRYPOINT for Self Contained Builds

  • Fixed existing integration test that tested that ContainerEntryPoint can be deferred (it wasn't actually testing anything, due to a bug in the test)
  • Changed the test to test that we defer ContainerAppCommand instead (ContainerEntryPoint is not deferred anymore on .net 8)
  • Added runtime identifier dimension to the test (linux vs windows)
  • Fixed integration test ProjectInitializer to take into account Windows vs *nix when setting executable extension (.exe or nothing)
  • Fixed Microsoft.NET.Build.Containers.targets so that the behaviour is as expected on both Windows and linux containers

Fixes dotnet/sdk-container-builds#559

@erikbra erikbra requested a review from a team as a code owner May 9, 2024 14:46
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Containers Related to dotnet SDK containers functionality untriaged Request triage from a team member labels May 9, 2024
@erikbra erikbra force-pushed the bugs/559-publishcontainer-appends-dotnet branch from 3647099 to 54f65fc Compare May 9, 2024 14:53
Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

Excellent find on the old test gap!

@baronfel baronfel requested a review from a team May 9, 2024 17:13
@erikbra erikbra force-pushed the bugs/559-publishcontainer-appends-dotnet branch from 7b67edd to d6193fb Compare May 9, 2024 20:05
@erikbra erikbra changed the title Bug #559: PublishContainer Appends "dotnet <Target>.dll" to ENTRYPOINT for Self Contained Builds Bug dotnet/sdk-container-builds#559: PublishContainer Appends "dotnet <Target>.dll" to ENTRYPOINT for Self Contained Builds May 9, 2024
…RYPOINT for Self Contained Builds

* Fixed existing integration test that tested that ContainerEntryPoint can be deferred
  (it wasn't actually testing anything, due to a bug in the test)
* Changed the test to test that we defer ContainerAppCommand instead
  (ContainerEntryPoint is not deferred anymore on .net 8)
* Added runtime identifier dimension to the test (linux vs windows)
* Fixed integration test ProjectInitializer to take into account Windows vs *nix
  when setting executable extension (.exe or nothing)
* Fixed Microsoft.NET.Build.Containers.targets so that the behaviour is as expected on
  both Windows and linux containers
@erikbra erikbra force-pushed the bugs/559-publishcontainer-appends-dotnet branch from d6193fb to 151ef05 Compare May 10, 2024 05:01
@baronfel
Copy link
Member

@tmds if you have a moment I'd love to get your eyes on this as well - I'm quite happy with the changes but you also have some history in this space.

@baronfel baronfel enabled auto-merge (squash) May 15, 2024 18:04
@baronfel
Copy link
Member

Applied that fix and marked for merge. Thank you @erikbra for this contribution, and the related MSBuild issue writeup!

@baronfel baronfel merged commit 1b46132 into dotnet:main May 15, 2024
24 checks passed
@erikbra
Copy link
Contributor Author

erikbra commented May 18, 2024

Thanks for the follow-up, fix and merge, @baronfel ! I'm excited about which way the MsBuild issue/concern will go. I think that one's more deep-rooted in the core templates.

@erikbra erikbra deleted the bugs/559-publishcontainer-appends-dotnet branch May 18, 2024 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Containers Related to dotnet SDK containers functionality untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PublishContainer Appends "dotnet <Target>.dll" to ENTRYPOINT for NativeAOT Builds
4 participants