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

Can't run ContainerPublish in solution with multi targeting #578

Open
afscrome opened this issue Jun 24, 2024 · 4 comments
Open

Can't run ContainerPublish in solution with multi targeting #578

afscrome opened this issue Jun 24, 2024 · 4 comments

Comments

@afscrome
Copy link

#37072 was merged to allow you to run dotnet publish /t:ContainerPublish against a solution and have it ignore projects in which SDK Container Publishing was not enabled.

Whilt this works for many projects, if your solution has any multi targeted projects (e.g. <TargetFrameworks>net472;netstandard2.0;net6.0;net8.0</TargetFrameworks>), trying to publish the solution fails on the multi targeted project(s)

error MSB4057: The target "PublishContainer" does not exist in the project.

Looking at the bin log, the SDK declined to import Microsoft.NET.Build.Containers.targets. (although oddly it did import Microsoft.NET.Build.Containers.props)

image

Interestingly, whilst the Microsoft.NET.Build.Containers.targets wasn't imported, Microsoft.NET.Build.Containers.props was - I'd expect either both or neither of these files to be imported, and not just one.

NoImport: $ (_ContainersTargetsDir)Microsoft.NET.Build.Containers.targets at (64;3) false condition; (Exists('$(_ContainersTargetsDir)Microsoft.NET.Build.Containers.targets') AND '$(TargetFramework)' != '' was evaluated as Exists('C:\Program Files\dotnet\sdk\8.0.302\Sdks\Microsoft.NET.Sdk\Sdk......\Containers\build\Microsoft.NET.Build.Containers.targets') AND '' != '').

(Note a space was added in $(_ContainerTargetsDir) above as otherwise github tired to format it as a mathematical equation.

The problem seems to be that the target import includes a '$(TargetFramework)' != ''" check https://github.com/dotnet/sdk/blob/3cfb3ea0bf20d99ffba14e1cb56e07f0806ce1eb/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets#L65

To be clear, if Publishing Containers doesn't work with multi targeting, that's fine by me. However I'm expecting the target to be there and be a no-op if IsPublishable == false or if the project doesn't have container configuration.

@baronfel
Copy link
Member

Hi, sorry for the delay here - publishing containers doesn't currently work with multi-targeting out of the box, you'd need to publish for a specific TFM, which I know doesn't work super-great with solution-level publishing. We'll need to do some work here to light this up.

One question we'd need to answer for multi-TFM publishing is - how should the ContainerImageTag(s) be inferred when multi-targeting? Take for example a Web SDK project name myapp targeting net8.0 and net9.0 TFMs - this project would call PublishContainer for each TFM, resulting in base images of mcr.microsoft.com/dotnet/aspnet:8.0 and mcr.microsoft.com/dotnet/aspnet:9.0 respectively, but without some kind of customization both generated images would have the same default tag(s) - myapp:latest. Should we prepend the TFM to each image in this case? Or something different?

Proposals

Prepend TFM to each default-generated tag

By default we infer the latest tag for a generated image. With this proposal, for multi-targeted builds only we would change the default to be $(_TargetFrameworkVersionWithoutV)-latest. For example, 9.0-latest, 8.0-latest.

Append TFM to each default-generated tag

By default we infer the latest tag for a generated image. With this proposal, for multi-targeted builds only we would change the default to be latest-$(_TargetFrameworkVersionWithoutV). For example, latest-9.0, latest-8.0.

Do not allow defaults for multi-targeted image builds

With this proposal, we would require the user to set a ContainerImageTag or ContainerImageTags for each TFM explicitly - this could be as simple as a single <ContainerImageTag>some-name-that-uses-$(_TargetFrameworkVersionWithoutV)</ContainerImageTag> in the project file, or something more bespoke that has conditions based on TFM:

<PropertyGroup>
  <ContainerImageTag Condition="$(TargetFrameworkVersion) == 'net9.0">speedy</ContainerImageTag>
  <ContainerImageTag Condition="$(TargetFrameworkVersion) == 'net8.0">normal</ContainerImageTag>
</PropertyGroup>

In this mode, we would error if a user tried a multi-TFM build without specifying ContainerImageTag explicitly.

Thoughts?

Would love feedback on these proposals - what do users expect to happen?

@afscrome
Copy link
Author

afscrome commented Aug 12, 2024

Sorry for not responding sooner.

Honestly I'm not too fussed how multi target builds work for containers. My main concern is that if I have a multi targeted class library marked as IsPublishable = false, or a project that is publishable but has no container configuration, the project should get skipped rather than failing with MSB4057: The target "PublishContainer" does not exist in the project.

So specifically if a solution contains the following project

<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <TargetFrameworks>net8.0;net472</TargetFrameworks>
        <IsPublishable>false</IsPublishable>
    </PropertyGroup>
</Project>

I'd expect that to be ignored because I've explicitly marked it as not publishable - the container publishing process can ignore the project long before the complication of multi targeted container publishing comes up.

In terms of multi targeted container builds, I'd personally want to manually control tags rather than have an automated one applied. I'll pick one framework version and ship that and not give my users the choice. I guess the only time I can think of is a brief transition period, but in that time I'd still want one of the frameworks to remain the latest tag, with the other framework being some other tag (e.g. the .Net v.next would start out as next tag until a point at which that becomes latest and I either kill off the old dotent image completely, or publish it for a bit with some kind of legacy tag.

If you do go with an automated prefix/suffix then I'd rather see dotnet8.0 rather than 8.0. Tagging images witha version number (e.g. 6.0) is a common pattern. Auto generating a tag of 6.0-8.0 suddenly makes this rather confusing. 6.0-dotnet8.0 at least makes it clear that the 8.0 portion relates to .Net 8.0

@afscrome
Copy link
Author

A fourth proposal could be to allow one of the target frameworks to be specified as "Primary". This will then be left without a prefix (e.g. latest) whilst all the others get the automated suffix (or prefix) (e.g. latest-dotnet8.0.)

@baronfel
Copy link
Member

My main concern is that if I have a multi targeted class library marked as IsPublishable = false, or a project that is publishable but has no container configuration, the project should get skipped rather than failing with MSB4057: The target "PublishContainer" does not exist in the project.

Ah yes, this makes perfect sense. We should do this at a minimum!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants