Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declare internal source attributes in consuming projects #218

Closed
wants to merge 3 commits into from

Conversation

jnm2
Copy link

@jnm2 jnm2 commented Apr 10, 2020

Alternative to the approach taken in #130

I tested the nupkg and was able to use the attributes in a new console project just by referencing the package. When I added <IncludeOwnershipAttributes>false</IncludeOwnershipAttributes> to the csproj, the attributes were no longer declared.

Resulting package file structure:

 IDisposableAnalyzers.3.3.4
 │   IDisposableAnalyzers.nuspec
 │
 ├───analyzers
 │   └───dotnet
 │       └───cs
 │               Gu.Roslyn.Extensions.dll
 │               IDisposableAnalyzers.dll
 │
+├───build
+│       IDisposableAnalyzers.targets
+│       OwnershipAttributes.cs
 │
 └───tools
         install.ps1
         uninstall.ps1

@JohanLarsson
Copy link
Collaborator

This looks perfect!
Gonna wait for a bit with merging, trying to close some bug issues first.

@jnm2
Copy link
Author

jnm2 commented Apr 27, 2020

@JohanLarsson We need to test this in a WPF project because I think it will suffer from the same issue as tunnelvisionlabs/ReferenceAssemblyAnnotator#81.

Copy link

@jeremyVignelles jeremyVignelles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't see that you implemented it :)

I'm ok with the set, with a few remarks

@jeremyVignelles
Copy link

For your remark about WPF, I don't know if that can be an issue. I'd think that the way both package work are quite different.
In our case we add a source file into the project directly, we're not adding things to the build pipeline dynamically (I don't know what ReferenceAssemblyAnnotator does, but from the name, it seems that it does specific things, right?)

@jnm2
Copy link
Author

jnm2 commented May 8, 2020

For your remark about WPF, I don't know if that can be an issue. I'd think that the way both package work are quite different.
In our case we add a source file into the project directly, we're not adding things to the build pipeline dynamically (I don't know what ReferenceAssemblyAnnotator does, but from the name, it seems that it does specific things, right?)

The problem with WPF is only related to including a source file into the project. I.e. it's related to added <Compile> items, not to <Reference> items. WPF creates a temp copy of the csproj using the file system and only carries certain things over into it. In the mix, our added <Compile> item is never provided a way to show up in this temp project.

Specifically, the problem is that the .targets file is not included in the WPF temp project and so <Compile Include="$(OwnershipAttributesPath)" Visible="false" /> never happens for the temp project.

@jnm2
Copy link
Author

jnm2 commented Jul 30, 2020

@jnm2
Copy link
Author

jnm2 commented Oct 2, 2022

Closing since this PR was mainly a demonstration. If you'd like to take this approach in the future, this can serve as a reference.

@jnm2 jnm2 closed this Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants