Azure.Storage.Blobs: Should [assembly: InternalsVisibleTo] test attributes be removed in the released NuGet package?

[stakx]

Query/Question
This issue lies somewhere between a question and a bug report.

The main assembly of the Azure.Storage.Blobs NuGet package (version 12.4.0) carries these two custom attributes:

[assembly: InternalsVisibleTo("Azure.Storage.Blobs.Tests, PublicKey=" +
    "0024000004800000940000000602000000240000525341310004000001000100d15ddcb2968829" +
    "5338af4b7686603fe614abd555e09efba8fb88ee09e1f7b1ccaeed2e8f823fa9eef3fdd60217fc" +
    "012ea67d2479751a0b8c087a4185541b851bd8b16f8d91b840e51b1cb0ba6fe647997e57429265" +
    "e85ef62d565db50a69ae1647d54d7bd855e4db3d8a91510e5bcbd0edfbbecaa20a7bd9ae74593d" +
    "aa7b11b4")]
[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]

They're obviously needed for the test project. But is it wise to include them in the released NuGet package?

I cannot be certain at this time, but the latter attribute possibly causes trouble for downstream users; see castleproject/Core#488 and devlooped/moq#991.

Environment:

• Azure.Storage.Blobs 12.4.0
• I'm investigating the issues linked in the description above on Windows 10, using Visual Studio 2017 and .NET Core 2.2

[ghost]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

[AlexGhiondea]

/cc @pakrym @tg-msft

[pakrym]

The usual practice is not to remove the attribute.

Quick search showed that there are a lot of libraries shipping with it.

I also think there is a bug in how castle core detects if members are visible (I've commented on a linked issue)

azure.data.appconfiguration
azure.identity
azure.messaging.eventhubs
azure.messaging.eventhubs.processor
azure.security.keyvault.certificates
azure.security.keyvault.keys
azure.storage.blobs
azure.storage.common
azure.storage.files.shares
azure.storage.queues
castle.core
dotnet-format
microsoft.applicationinsights
microsoft.applicationinsights.dependencycollector
microsoft.applicationinsights.windowsserver.telemetrychannel
microsoft.aspnet.mvc
microsoft.aspnetcore.antiforgery
microsoft.aspnetcore.app.ref
microsoft.aspnetcore.dataprotection
microsoft.aspnetcore.diagnostics
microsoft.aspnetcore.mvc.abstractions
microsoft.aspnetcore.mvc.core
microsoft.aspnetcore.mvc.taghelpers
microsoft.aspnetcore.mvc.viewfeatures
microsoft.aspnetcore.razor.design
microsoft.aspnetcore.razor.language
microsoft.codeanalysis.common
microsoft.codeanalysis.razor
microsoft.codeanalysis.workspaces.common
microsoft.codeanalysis.workspaces.msbuild
microsoft.codecoverage
microsoft.dotnet.internalabstractions
microsoft.dotnet.platformabstractions
microsoft.entityframeworkcore
microsoft.entityframeworkcore.relational
microsoft.entityframeworkcore.sqlserver
microsoft.extensions.configuration.azurekeyvault
microsoft.extensions.diagnosticadapter
microsoft.extensions.fileproviders.physical
microsoft.extensions.http
microsoft.extensions.http.polly
microsoft.identity.client
microsoft.identitymodel.clients.activedirectory
microsoft.internal.netsdkbuild.mgmt.tools
microsoft.net.compilers.toolset

[stakx]

@pakrym, thanks for the hint in the other issue... this seems to indeed be the main problem, which we should be able to fix in DynamicProxy.

Also, thanks for commenting on the usual practice on Microsoft's side, that's good to know and basically answers my question, so I think we can close this issue.