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

'Private' metadata is not propagated to transitive dependencies #1366

Open
jaredpar opened this issue Jun 22, 2017 · 16 comments
Open

'Private' metadata is not propagated to transitive dependencies #1366

jaredpar opened this issue Jun 22, 2017 · 16 comments
Labels
Milestone

Comments

@jaredpar
Copy link
Member

This was discovered moving Roslyn's VSIX projects to the new SDK.

Roslyn builds a number of VSIX projects and is very particular about which projects end up embedded in which VSIX. There are a number of patterns employed to accomplish this including

<ProjectReference Include="..\..\VisualStudio\Core\Impl\ServicesVisualStudioImpl.csproj">
  <Project>{c0e80510-4fbe-4b0c-af2c-4f473787722c}</Project>
  <Name>ServicesVisualStudioImpl</Name>
  <Private>false</Private>
</ProjectReference>

The intent here is to reference the project but not include its contents in the resulting VSIX by means of <Private>false</Private>. This continues to work in the new SDK.

However implicit transitive references ends up pulling in all of the projects that ServicesVisualStudioImpl.csproj. None of those projects are marked as <Private>false</Private> and as a result end up included in the VSIX. This both bloats (in some cases 100 fold) and functionally breaks our VSIX (some cases 100 fold).

@jaredpar
Copy link
Member Author

Note: this is not a partner blocking issue. @nguerrera helped me out with a work around here. For the VSIX projects we can just override the IncludeTransitiveProjectReferences target to do nothing. Still would be nice to have this fixed at some point.

CC @jasonmalinowski

@nguerrera
Copy link
Contributor

So private assets on the project reference did not work?

@nguerrera
Copy link
Contributor

I mean PrivateAssets=All

@jaredpar
Copy link
Member Author

@nguerrera I tried that but looking back at your message I worry I may have done it incorrectly. What I tried was adding the following to the ProjectReference node:

<PrivateAssets>All</PrivateAssets>

Your message suggested the following

<ItemDefinitionGroup>
   <ProjectReference>
     <PrivateAssets>All</PrivateAssets>
  </ProjectReference>
</ItemDefinitionGroup>

Not sure if that makes a difference or not.

@nguerrera
Copy link
Contributor

I expected either to work. The definition group was just a way to apply it to all project references.

@rainersigwald
Copy link
Member

The IDG would apply it to all ProjectReference items, even those dynamically created by NuGet. My guess is that NuGet isn't propagating that metadata from the reference to its transitive closure when it dynamically discovers/inserts the closure (based only on this issue, opinion unsullied by looking at any code).

@jaredpar
Copy link
Member Author

Here is the work around we employed

jaredpar/roslyn@eb66259

@nguerrera
Copy link
Contributor

Note there are two pieces of metadata with similar names and different meanings. Private and PrivateAssets. My expectation was that PrivateAssets (read by NuGet not build) would impact the assets file such that transitive project references would be suppressed coming through that reference. But I realize now that private assets decides whether a reference flows to your consumers, not that you want to consume something but not its dependencies.

IDG for Private=false might work, but it's not the semantic we're after. We're trying to prevent extra project references from being raised, not to raise them with Private=false.

@nguerrera
Copy link
Contributor

@natidea @emgarten Is there a better way to prevent transitive project refs than overriding the target?

@natidea
Copy link
Contributor

natidea commented Jun 23, 2017

@nguerrera from within the SDK, overriding the target seems like the best way. The only other way would be to take a direct dependency on each offending implicit project reference, and set them all to private.

jaredpar added a commit to jaredpar/roslyn that referenced this issue Jun 28, 2017
By default the new SDK will cause projecs to reference the full
transitive closure of project references, not just the direct
references. This is fine in most cases but caused breaks in our VSIX
projects.

Our VSIX projects often reference project with specific extra metadata
to control whether or not their contents are included in the final VSIX.
That extra metadata is not applied to projects which are implicitly
referenced by the new SDK. That caused our VSIX to bloat in size and
have functionally incorrect contents.

Fixed this by disabling implicit transitive references for our VSIX
projects.

dotnet/sdk#1366
@nguerrera nguerrera added this to the 2.1.0 milestone Aug 2, 2017
@nguerrera nguerrera added the Bug label Aug 2, 2017
@nguerrera nguerrera changed the title Implicit transitive projects change VSIX contents 'Private' metadata is not propagated to transitive dependencies Aug 2, 2017
@nguerrera
Copy link
Contributor

Changing title to match more general issue as #1467 stated it (marked as dupe now). The repro steps in #1467 are also very clear and should be used to construct a test case when fixing this. cc @sharwell

@dsplaisted dsplaisted self-assigned this Feb 21, 2018
@livarcocc livarcocc modified the milestones: 2.1.3xx, 2.1.4xx Mar 30, 2018
@livarcocc livarcocc modified the milestones: 2.1.4xx, 2.2.1xx Jun 27, 2018
@joaorosado
Copy link

joaorosado commented Sep 29, 2018

Wow I just lost hours on this issue.

My fix was to not only set Private to false like @nguerrera suggested, but also ExcludeAssets on the ProjectReference element. PrivateAssets did not seem to do anything.

My scenario is with a project that provides a API for a plugin implementation. The API assembly (and it's dependencies) must never be copied to the output folder of the plugin.
Before with packages.config the <Private>false</Private> was enough for the API assembly and all dependencies to not be copied.
Now after migrating everything to use PackageReferences the API assembly was not copied, but all the nuget dependencies did!!

My final file ended up something like this:


 <ProjectReference Include="..\MyAPIPRoject\MyAPIPRoject.csproj">
      <Project>{efb0d1e3-076d-4f16-a595-b22bfc5b3f18}</Project>
      <Name>MyAPIPRoject</Name>
      <Private>false</Private>
      <ExcludeAssets>all</ExcludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </ProjectReference>

(Also found this similar to #952 but with dependencies)

Now the big challenge will be how to enforce this to always be set and no developers make mistakes :(

Edit: this doesn't seem very consistent with the ExcludeAssets semantic on PackageReferences, since I had a similar situation there and add to change the "all" to "runtime" so it would still let me compile against it. So changed these as well to ExcludeAssets="runtime" to make them consistent in my projects and it seems to be enough.

@mhutch
Copy link
Member

mhutch commented Jan 17, 2019

This is also an issue for VS for Mac.

Here are my notes, in case it's helpful:

Right now, if project B has a private (i.e. the default) project reference to project A and
project C has a non-private ref to project B, project C will transitively get a private ref
to project A.

This seems very wrong, and is very much NOT what we want.

Extensions reference each other with non-private refs so we only get one copy of
each dll in the app. This breaks that.

Setting PrivateAssets="runtime,contentFiles" on the private reference does NOT
fix it.

And here's my workaround:

<Target Name="_MakeTransitiveProjectRefsNonPrivate" AfterTargets="IncludeTransitiveProjectReferences">
	<ItemGroup>
		<!-- remove the transitive project references and re-add them as non-private project references -->
		<ProjectReference Remove="@(_TransitiveProjectReferences)" />
		<ProjectReference Include="@(_TransitiveProjectReferences)" Private="False" />
	</ItemGroup>
</Target>

@nguerrera
Copy link
Contributor

I suspect there isn't enough information in the assets file today to raise the transitive refs correctly in this case. We likely need to coordinate the fix with nuget.

@tbolon
Copy link

tbolon commented Feb 15, 2019

Is there any workaround for this problem ? I am using the new sdk project format, and I have a transitive dependency between my ProjectReferences (A => B => C). A references B with Private=true and B references C with Private=true.

Currently, when I build project A, I have A and C assemblies in my output folder.

I have two requirements:

  • Still being able to generate my package using nuget pack with A.csproj, resulting in a valid nupkg with a dependency to B
  • When building, having only the assemblies from project A in my bin folder.

Any idea?

Thanks,

@livarcocc livarcocc modified the milestones: 2.2.1xx, 3.0.1xx Jun 12, 2019
@Nirmal4G
Copy link
Contributor

For developing Plugins/Extensions just like VSIX but for other Applications, there should be an easier way to Disable Transitive references throughout the entire reference graph.

Why wasn't this enabled at first when you guys implemented Project/PackageReference?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests