Populate ReferenceSatellitePaths with Resources

[natidea]

This is a proposed fix to missing satellite assemblies in the SatelliteDllsProjectOutputGroupDependencies when using PackageReference mode for NuGet dependencies (Bug 465204)

Since I'm not familiar with how this was intended to work, here are some concerns I can think of:

• In the packages.config scenario, ResolveAssemblyReferences does this -- not clear why it is not doing so for this scenario. I tried removing Private=false from the Reference but that did not make a difference here.
• I thought about removing the items from ReferenceSatellitePaths before re-adding, to prevent duplicates, but its not clear how they could have gotten in there in the first place
• I see two separate code paths that create copy local items with DestinationSubDirectory: one is for packages with locale information, the other is for copy-local items with an "outputPath". Should both be included in the satellite groups?
• The metadata included in the final outputgroup after using this approach is not exactly the same as the result of ResolveAssemblyReferences from packages.config scenario. Whether this is important depends on how consumers are using these output groups. from packages.config from PackageReference

/cc @rohit21agrawal @emgarten @jasonmalinowski @nguerrera

[emgarten]

I'm not familiar with this area either, is there someone who is that can sign off on this?

[rohit21agrawal]

from what i can understand, it might be better if you set "Culture" metadata on the items being created here: https://github.com/NuGet/NuGet.BuildTasks/blob/18aedb7eb0fc26012246af61e4df7cda4ab10026/src/Microsoft.NuGet.Build.Tasks/ResolveNuGetPackageAssets.cs#L782

and then check for the existence of culture metadata in the "ReferenceCopyLocalPaths" instead of destination sub directory.

[nguerrera]

@cdmihai @AndyGerlicher Can you help review this?

@natidea We need to check what happens in SDK-based projects as well. I think you said that the same problem was happening there, right? Is there a bug tracking that?

[cdmihai]

The question here is why isn't RAR finding these? Sounds like that should be the better fix.

Here's how RAR searches for the satellites: https://github.com/Microsoft/msbuild/blob/master/src/Tasks/AssemblyDependency/ReferenceTable.cs#L930-L947

[cdmihai]

Also, does this happen during a real build, or during a special design time build / restore build? I see that finding satellites is based on BuildingProject, and BuildingProject appears to be set only during real builds.

[natidea]

@cdmihai This scenario is for a real build from the command line.

One note is that all these satellites are apparently in the lock/asset file. So RAR searching for them may be redundant (which is why we speculated that RAR was avoiding this action on purpose). But if RAR is always supposed to find satellites, then we need to figure out why the file locations are different for packages.config and PackageReference-based projects

[Pilchie]

Pin @cdmihai and @AndyGerlicher - do we want to take this fix, or move the issue to msbuild?

[nguerrera]

I don't think that RAR can be the one to find satellites in the nuget case because of the ref/, lib/ split. Satellites to deploy aren't necessarily next to the assemblies passed to the compiler.

[cdmihai]

@nguerrera

I see. Is that why RAR isn't finding the satellites for the case this PR addresses? Is it because RAR sees the ref dlls, but the satellites are in lib?

If so, then we need to decide whether:

• RAR should still be the one finding them, with the help of additional inputs (like metadata on each reference saying where the root of the satellites is).
• Another entity (nuget, Microsoft.Net.SDK, etc) owns the knowledge of how to find the satellites up by adding to items after RAR runs. Like this PR does.

Pros for RAR:

• logic is in one place but with many control knobs. May be easier to debug, because hunting down all the places in the build that inject satellites does not sound like fun.

Cons for RAR:

• if the logic of finding satellites ever gets more complicated than "look under this other root directory instead of the dll directory", then we probably can't express it as an input to RAR. Would this really be the case though? If it's truly the case, then the entity which owns the complicated logic can compute the satellite root path for each dll and pass it to RAR.

[dmonroym]

We've moved things around this repo. Please reopen this PR against master if you want us to take a look at this again.