Missing [assembly: AssemblyMetadata("Serviceable", "True")]

[bricelam]

If a project is serviceable and can potentially run on .NET Framework (i.e. targets net472, netstandard2.0, etc.) it should include the following assembly-level attribute.

[assembly: AssemblyMetadata("Serviceable", "True")]

cc @Eilon

[Eilon]

Here's an old thread where this is also discussed: https://github.com/dotnet/cli/issues/3345

[markwilkie]

Thanks @bricelam - good timing as we'll be tackling servicing of Arcade soon.

[tmat]

@bricelam How is this related to Arcade? Are you suggesting that we add a target that adds this attribute to all assemblies built with Arcade or that Arcade tools themselves are marked with this attribute?

[bricelam]

Yes, add a target to Arcade that adds this attribute to the assembly if the MSBuild Serviceable property is ture. KoreBuild did it here.

[bricelam]

Doesn't seem important for Arcade tools to use this since they won't be serviced via Windows Update.

[Eilon]

Yeah the attribute is supposed to be present in all DLLs that might run on .NET FX. It causes a breadcrumb trail to be left (in the registry?) so that WU/MU can scan and see if a DLL was ever used on the machine, in case it needs to be serviced.

[tmat]

Sounds reasonable.

[vatsan-madhavan]

I just noticed this as well. System.Windows.Forms.dll, System.Xaml.dll etc. are produced from an Arcade Sdk based repo, and do not contain this attribute. Whereas assemblies like WindowsBase.dll that are produced in a buildtools based repo contain the following attributes:

[assembly: AssemblyMetadata(".NETFrameworkAssembly", "")]
[assembly: AssemblyMetadata("PreferInbox", "True")]
[assembly: AssemblyMetadata("Serviceable", "True")]

These attributes are populated in BuildTools here

Can this be added in Arcade Sdk as well ?

cc @ericstj, @rladuca, @stevenbrix

[tmat]

Should these be actually used for WPF built for .NET Core?

[assembly: AssemblyMetadata(".NETFrameworkAssembly", "")]
[assembly: AssemblyMetadata("PreferInbox", "True")]

These won't be shipping in desktop .NET Framework, right?

[tmat]

Another question about just the serviceable metadata. Should [assembly: AssemblyMetadata("Serviceable", "True")] be by default added to all assemblies that are shipping (IsShipping=true) and target framework identifier is .NET Framework or .NET Standard (with a switch to override this default)?

[vatsan-madhavan]

I agree that the metadata values generated by buildtools by default, and inherited by WPF today, don't make good sense.

.NETFrameworkAssembly is plain wrong, (maybe it should be something else for .NET Core assemblies?). How is [AssemblyIdentity.IsFrameworkAssembly](https://docs.microsoft.com/en-us/dotnet/api/microsoft.build.tasks.deployment.manifestutilities.assemblyidentity.isframeworkassembly?view=netframework-4.7.2&viewFallbackFrom=netcore-2.2) supposed to function on .NET Core CoreFx assemblies? Is it supposed to return False because .NET Core <> .NET Framework? If so, we should probably not even add the .NETFrameworkAssembly metadata value - and perhaps no replacement is necessary.

I'm not even sure what semantics PreferInbox is supposed to convey.

Perhaps Serviceable is the only one we will end up with eventually? If so, who/what is the intended reader of this metadata - do we know ?

[tmat]

@Eilon Explained the meaning of Serviceable here: https://github.com/dotnet/arcade/issues/1526#issuecomment-447079742

[Eilon]

@tmat said:

Another question about just the serviceable metadata. Should [assembly: AssemblyMetadata("Serviceable", "True")] be by default added to all assemblies that are shipping (IsShipping=true) and target framework identifier is .NET Framework or .NET Standard (with a switch to override this default)?

This seems reasonable to me.

[markwilkie]

@Pilchie - given the recent ASP servicing changes, do you know if this is still "a thing"?

[Pilchie]

I don't know of any other work that would address it. We still do have several packages that are .NET Standard, and our other servicing work is only related to reference assemblies, where I assume this needs to be in reference asssemblies.

[blowdart]

If assemblies aren't marked, and are targeted at Core, are we still dropping the breadcrumbs needed for hammer patching?

[ericstj]

On core the host writes breadcrumbs based on what's in the deps file, not what's in the assembly. @vitek-karas

[blowdart]

Just checking :)

[dougbu]

We think the ASP.NET assemblies are fine but an automated confirmation would be worth adding to the Arcade infrastructure. @jcagme is this already done in the validation ring? If not, could it be❔

In particular, could this be combined w/ signing validation (where the assemblies are already available) @joeloff❔

[joeloff]

It makes sense to add a validation step in the build. I'd image it could be an inline task, unless you want something like an assembly validation tool that can extract assemblies from other containers, e.g. zip, nupkg, installer package, etc. Sign validation mostly runs on larger containers (as those need to be signed too). I'm unsure whether it's being executed on the raw assemblies when they are signed.

[Eilon]

If only someone had written a tool to verify NuGet packages 🙄

https://github.com/Eilon/NuGetPackageVerifier/blob/master/NuGetPackageVerifier/Rules/AssemblyHasServicingAttributeRule.cs

😁

[markwilkie]

I wonder if this should be a candidate for the validation stage? cc/ @jcagme

https://github.com/dotnet/arcade/issues/2891

[jcagme]

Yes, this could be easily added to the validation stage just need to understand:

• What things we should be checking for?
• Are these things the same across repos?

[Eilon]

Feel free to look at all of these rules to see what type of stuff has been checked for in the past, as well as how to actually do the checks (if you want them):

https://github.com/Eilon/NuGetPackageVerifier/tree/master/NuGetPackageVerifier/Rules

These rules use Mono.Cecil to read the DLLs because it allows you to inspect DLLs without "loading" them in the traditional sense. This is especially important when you have to load multiple versions of the same DLL (e.g. netcoreapp3.1 and netstandard2.0).

[jcagme]

Ok so without looking much on the code it looks like we could use NuGetPackageVerifier to do the validation? Would that be enough? Do we run it after we've signed assets? What are other constraints to have in mind?

[dougbu]

Yes. These rules are what aspnetcore previously used (before migrating to Arcade). Our ask here is, as @Eilon implied, related to dotnet/aspnetcore#18930 in particular but dotnet/aspnetcore#18931 in general. Using the existing infrastructure in a new way would be grand.

[dougbu]

Are these things the same across repos?

NuGetPackageVerifier is controlled by files in repos it's applied to 😺

[jcagme]

Are these things the same across repos?

NuGetPackageVerifier is controlled by files in repos it's applied to 😺

Ok, so NuGetPackageVerifier executes on top of source code or more of already compiled assemblies?

[dougbu]

NuGetPackageVerifier uses files like https://github.com/dotnet/aspnetcore/blob/release/2.1/eng/NuGetPackageVerifier.json from the repo but (mainly?) executes against the built packages.

[jcagme]

Ok. I guess the best way to fully understand how it works is to give it a spin! Also, I'm on FR this and next week but I'll talk to my team and see if someone has the cycles to start working on it. I've included details about this in https://github.com/dotnet/arcade/issues/5631