AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET
https://aka.ms/msal-net
MIT License
1.4k stars 341 forks source link

[Bug] Microsoft.Identity.Client.NativeInterop.targets doesn't respect `GenerateAssemblyInfo=false` #4832

Open RussKie opened 4 months ago

RussKie commented 4 months ago

The embedding must be conditional, otherwise it breaks the build image

Repro steps:

test.csproj ```xml net472 Library latest false ```
Class1.cs ```cs namespace test; public class Class1 { } ```

Try to compile the above - it fails because the targets unconditionally embeds non-existing non-generated assembly info:

PS D:\Development\throwaway\Microsoft.Identity.Client.NativeInterop> dotnet build
  Determining projects to restore...
  All projects are up-to-date for restore.
C:\Program Files\dotnet\sdk\8.0.400-preview.0.24312.1\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(311,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [D:\Development\throwaway\Microsoft.Identity.Client.NativeInterop\test.csproj]
CSC : error CS2001: Source file 'D:\Development\throwaway\Microsoft.Identity.Client.NativeInterop\obj\Debug\net472\test.AssemblyInfo.cs' could not be found. [D:\Development\throwaway\Microsoft.Identity.Client.NativeInterop\test.csproj]

Build FAILED.

CSC : error CS2001: Source file 'D:\Development\throwaway\Microsoft.Identity.Client.NativeInterop\obj\Debug\net472\test.AssemblyInfo.cs' could not be found. [D:\Development\throwaway\Microsoft.Identity.Client.NativeInterop\test.csproj]
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:00.89
xinyuxu1026 commented 3 months ago

Hi @RussKie , thanks for opening this! Would <EmbeddedFiles Condition="'$(GenerateAssemblyInfo)' == 'true'" Include="$(GeneratedAssemblyInfoFile)"/> resolve this issue? Thanks.

RussKie commented 3 months ago

Yes, that should fix the issue.

xinyuxu1026 commented 3 months ago

Thanks! @RussKie , I checked in the change, it should be in next release.

RussKie commented 3 months ago

Great, thank you.

bgavrilMS commented 3 months ago

Hi @RussKie , thanks for opening this! Would <EmbeddedFiles Condition="'$(GenerateAssemblyInfo)' == 'true'" Include="$(GeneratedAssemblyInfoFile)"/> resolve this issue? Thanks.

If it will be fixed in the next release, please set the "milestone" to the next version of MSAL.NET (a milestone should already exist). It'll help partners keep track of fixes.

KirillOsenkov commented 3 months ago

My question is why is it adding the generated assembly info file to embedded files in the first place? Do we know? It's good to have this fix, but what is it trying to do in the first place?

xinyuxu1026 commented 1 month ago

Hi @RussKie , the new version of Microsoft.Identity.Client.NativeInterop is released https://www.nuget.org/packages/Microsoft.Identity.Client.NativeInterop/0.17.0, please update to Version="0.17.0"

KirillOsenkov commented 1 month ago

@xinyuxu1026 do you know the answer to my question above? thanks!

xinyuxu1026 commented 1 month ago

Hi @KirillOsenkov , I don't know about that.

@gladjohn might know more context.

KirillOsenkov commented 3 weeks ago

Hi @xinyuxu1026, I followed up with @gladjohn and we both agree that the EmbeddedFiles line should just be deleted.

Instead of adding the condition, could you please just delete the whole line? Thanks!