Add support for @(InternalsVisibleTo) items that turn into assembly attributes #3439
Conversation
|
Hi @kzu. This looks good. Could you add some test coverage to this PR? |
|
This is going to conflict with the unofficial feature in Arcade so would need to be coordinated. That one supports using $(PublicKey) or %(Key) metadata AFAICT. I think we'll want this to be a superset of that in functionality, ideally. Or we have to be able to turn this one off to enable that one. cc @tmat |
|
We shouldn't end up with two ways of doing it. Let's have one impl that supports all we need. |
|
Good point @nguerrera and @tmat. Added support for the existing arcade stuff too, which should make it a drop-in replacement. It can also be turned off now. |
|
@peterhuene just like for the other PR, if you can point me to existing tests for |
|
@nguerrera Where would a new feature like this get documented, such that once it's available we can point users to it? |
|
@kzu here is a test that is setting the individual properties that cause additional We should probably add a test to this file to cover setting |
|
|
|
@peterhuene finally got to implement the unit tests, they cover (I think) all the cases supported by the targets. |
…ttributes
Given that `InternalsVisibleTo` (IVT) is such a common assembly-level attribute, this adds support
for specifying it directly via simple items, such as:
```
<ItemGroup>
<InternalsVisibleTo Include="MyLibrary.Tests" />
</ItemGroup>
```
Optionally, a `Key` metadata can be specified to provide a strong-named IVT:
```
<ItemGroup>
<InternalsVisibleTo Include="MyLibrary.Tests" Key="PUBLIC_KEY" />
</ItemGroup>
```
The targets will also use automatically a `$(PublicKey)` if available and no `%(Key)` metadata
override is found. Otherwise, it will default to an IVT without a key.
This also avoids having to learn the `_Parameter1` syntax in `AssemblyAttribute` elements, and is
more similar to the way other higher-level properties like `AssemblyTitle` or `Product` are also
turned into assembly attributes.
To turn off this feature, set `$(GenerateInternalsVisibleToAttributes)` to `false`.
Partially fixes dotnet#3166
Adds documentation for dotnet/sdk#3439.
|
Added docs PR too :) |
|
Was that fail a fluke? |
|
Is there something I can do to get this PR green? |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
One leg was failing but the results were no longer available, so I've restarted CI |
There was a problem hiding this comment.
This is green now and it looks good to me. @dsplaisted is this ready to go?
| new XAttribute("Include", "Tests")))); | ||
| }); | ||
|
|
||
| new RestoreCommand(Log, testAsset.TestRoot).Execute().Should().Pass(); |
There was a problem hiding this comment.
nit: Same as #3440, restore is no longer necessary
|
@DanVicarel as this seems to be associated with milestone 5.0.1, I won't expect it prior next .Net Core major release (which is currently planed to be re-branded to .Net 5), then next Visual Studio update to integrate it... |
|
@kzu probably it's already late to ask/comment, but I would like to Why did you name item metadata Why did you introduce PublicKey property? <ItemDefinitionGroup>
<InternalsVisibleTo>
<PublicKey>...</PublicKey>
</InternalsVisibleTo>
</ItemDefinitionGroup> |
|
@renatfx it's never late to ask good questions :). The item metadata was named Finally, doing this in an item definition group is something customers can do themselves. Granted, the SDK could provide that item definition group conditionally, like: But it's not entirely clear that every time the current project is strong named, its IVT will point to |
|
Well, for me as a .NET (SDK) consumer rather than developer, this is asbolutely unclear: Why should public .NET SDK be compatible wth internal Arcade SDK? btw, I didn't realize that However, I still would vote (if I could) for |
It does not. We can rename the |
|
@tmat ok, great! |
….2 (dotnet#3439) - Microsoft.DotNet.Cli.Runtime - 3.1.100-preview3.19552.2
|
I see in the binlog that the right public key is being passed in the target, but the emitted assemblyinfo.cs is missing the key. I have VS 17.10.4 and .Net 8 304 SDK. What might cause the target to emit the attribute without the key? |
If the $(PublicKey) property is used, the SDK targets will automatically use it for the assembly attributes. See dotnet/sdk#3439 Simplifies spectreconsole#1623
If the $(PublicKey) property is used, the SDK targets will automatically use it for the assembly attributes. See dotnet/sdk#3439 Simplifies #1623
Given that
InternalsVisibleTo(IVT) is such a common assembly-level attribute, this adds supportfor specifying it directly via simple items, such as:
Optionally, a
Keymetadata can be specified to provide a strong-named IVT:The targets will also use automatically a
$(PublicKey)if available and no%(Key)metadataoverride is found. Otherwise, it will default to an IVT without a key.
This also avoids having to learn the
_Parameter1syntax inAssemblyAttributeelements, and ismore similar to the way other higher-level properties like
AssemblyTitleorProductare alsoturned into assembly attributes.
To turn off this feature, set
$(GenerateInternalsVisibleToAttributes)tofalse.Partially fixes #3166