IsExternalInit must be declared for all TFMs

[AArnott]

Unlike other polyfill packages where you can get away with declaring attributes only where the TFM does not declare them, IsExternalInit must be declared in the same assembly forever and for all TFMs or else it's a binary breaking change.

I see in your nuspec that you omit the declaration for net5 but you can't do that. You must declare it there as well.
The reason being the compiler generates metadata on the init property accessors that references this attribute using its assembly-qualified type name. When other assemblies reference this one and invoke an init accessor, the compiler embeds that exact same assembly-qualified reference into their invocation of that accessor. Now imagine this referencing library runs in a process where the referenced library is no longer the one that targeted net472 but targets net5. The CLR will throw a MissingMethodException when it encounters the init accessor because the attribute on the accessor does not match the IsExternalInit reference in the caller.

See proof of the breaking change

[manuelroemer]

Ah, yes, that's bad. Thanks for raising this!
@AraHaan already created #6 to fix this. I'll make sure that this lands on NuGet quickly!

[gravity00]

@AArnott @manuelroemer
I do understand the problem but in my opinion the inicial behavior of ignoring NET 5.0 is the correct one.

If I'm creating some library that I want to be able to run in older TFMs and still use init properties, it is my job not to had dependencies if I don't need them by using a conditional reference, something like this:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>netstandard1.3;net5.0</TargetFrameworks>
    <LangVersion>9</LangVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="IsExternalInit" Version="1.0.0" Condition=" '$(TargetFramework)' == 'netstandard1.3' ">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>

</Project>

This is very similar to using async/await keywords in .NET 4.0 where you could add the Microsoft.Bcl.Async nuget, but you shouldn't add it when compiling for .NET 4.5+ or you could bring problems to whoever was using your lib since some conflicts could happen.

[AArnott]

@gravity00 async/await in .NET Framework 4.0 brought in a runtime dependency, which helps the case that you should drop it in later frameworks. It also didn't (AFAIK) impact your public API since Task was already defined in .NET Framework 4.0.

IsExternalInit has no runtime dependency. The code it contains is directly added to your project's compilation. Removing the dependency (including via Condition) represents a binary build break for callers that were compiled on a build that included the dependency and later ran on a build that removed that dependency.

This is why I (still) strongly recommend that if you use this package, you forever consider yourself "stuck" using it, for all TFMs and for all time. Not that I have any qualms with that. It's an empty attribute in your dll, and for most purposes can be internal. A small price to pay to avoid a very confusing breaking change.

[gravity00]

@AArnott just to be sure I do understand the racional with some practical use case, lets consider conditional references:

LibraryA
  Dependencies
    .NET Standard 2.0
      SDK
      IsExternalInit
    .NET 5.0
      SDK

LibraryB
  Dependencies
    .NET Standard 2.0
      SDK
      LibraryA (will have an internal IsExternalInit)

LibraryC
  Dependencies
    .NET 5.0
      SDK
      LibraryB (will depend on a LibraryA with an internal IsExternalInit, but will receive one without it, will fail)

I can see the point and you are correct!

I'm very careful to ensure my libs also compile for the different TFMs the libs I depend on compile (in this example the problem would be prevented if LibraryB also compiled for .NET 5.0, depending on LibraryA without IsExternalInit) but we can't enforce good behavior and considering compatibility problems versus an internal class, clearly the later is a better option.

[AArnott]

@gravity00 LibraryB and LibraryC should not depend on IsExternalInit, even transitively. LibraryA's dependency on IsExternalInit should be done with PrivateAssets="all" so that it's entirely a build detail of that project and has no dependency impact downstream.

[manuelroemer]

Despite way too much time having passed since the creation of this issue, it should now be fixed with version 1.0.1 on NuGet.
Thanks to @AraHaan for creating the fix! 🚀

I tested the newest version using a local package feed before uploading to NuGet and everything seems to work fine, but please let me know if there is any regression. I'm not primarily working with .NET at the moment, so I might not realize myself. Thanks!

[gravity00]

@gravity00 LibraryB and LibraryC should not depend on IsExternalInit, even transitively. LibraryA's dependency on IsExternalInit should be done with PrivateAssets="all" so that it's entirely a build detail of that project and has no dependency impact downstream.

Never said it would depend on IsExternalInit, I was saying it would have (or not) an internal class IsExternalInit.

LibraryB (will depend on a LibraryA with an internal IsExternalInit, but will receive one without it, will fail)

But we are both agreeing in your point.

[AraHaan]

@gravity00 LibraryB and LibraryC should not depend on IsExternalInit, even transitively. LibraryA's dependency on IsExternalInit should be done with PrivateAssets="all" so that it's entirely a build detail of that project and has no dependency impact downstream.

Never said it would depend on IsExternalInit, I was saying it would have (or not) an internal class IsExternalInit.

LibraryB (will depend on a LibraryA with an internal IsExternalInit, but will receive one without it, will fail)

But we are both agreeing in your point.

Actually they wont receive the internal copy for usage unless one provides InternalsVisibleTo attribute in LibraryA for LibraryB and LibraryC. So without that attribute all 3 of them would have to reference IsExternalInit.

[AArnott]

The C# compiler team has informed me that this attribute being internal is actually problematic. But making it public comes with trouble of its own. A future version of the compiler will be made smarter so that in the face of multiple definitions of this attribute, the one from the local project, or the one from the authoritative reference assembly, will be used.
Until such a C# compiler is widely used however, I think we should stick with the attribute being internal.

[AraHaan]

@AArnott I think a better option would be to have the compiler emit the attribute itself until then then it would make this package or manually making the attribute internal ourselves unneeded.

[AArnott]

Yes, and for many new attributes this is exactly what the compiler does. I suspect the reason they didn't in this case is that the assembly-identity of the attribute is important to the runtime. If they ever emitted it, they would have to always emit it. I don't know why they didn't make that choice.

[Silvenga]

The C# compiler team has informed me that this attribute being internal is actually problematic.

@AArnott do you have a public source of that comment by chance? FWIW, the R# team recommends making this internal - https://youtrack.jetbrains.com/issue/RSRP-487263#focus=Comments-27-5657790.0-0

[AraHaan]

I prefer it to be public so that way for example I could have a dummy project that brings in all the attributes at runtime that are normally internal in the regular runtime (dotnet/runtime) in there, then I could just reference that for that way I could use all the newer language features in those older projects if needed (like in roslyn source generators for example perhaps).

[Nirmal4G]

I just hit this now. This is one of the things I hate about .NET Runtime. This also stems from the root cause of needing to maintain compatibility that you have several intricate and unsolvable problems in the Runtime.

Just break things across major versions. We now have Analyzers, Fixers and Source Generators to help us migrate.