VS 1449000: Fix handling of satellite assemblies in ClickOnce #6665
Conversation
src/Tasks/ResolveManifestFiles.cs
Outdated
| if (identity != null && !String.Equals(identity.Culture, "neutral", StringComparison.Ordinal)) | ||
| { | ||
| // Use satellite assembly strong name signature as key | ||
| string key = identity.ToString().ToLowerInvariant(); |
There was a problem hiding this comment.
Instead of having to create this new string, can you use an invariant comparer on the dictionary?
There was a problem hiding this comment.
changed per suggestion.
src/Tasks/ResolveManifestFiles.cs
Outdated
| // decides to publish the en-us resource assemblies for other locales also. | ||
| if (!LauncherBasedDeployment && satelliteRefAssemblyMap.ContainsItem(item)) | ||
| { | ||
| Debug.Assert(false, $"Duplicate satellite assembly '{item.ItemSpec}' skipped in _managedAssemblies"); |
There was a problem hiding this comment.
I'm not sure I understand these asserts. Is it a bug in the task if we get to this point? If it's a bug in the user's projects or an unusual condition, I don't think the assert is appropriate.
There was a problem hiding this comment.
It's about unusual satellite assemblies in file/nuget references. I have removed the asserts as it's not a bug in the task.
src/Tasks/ResolveManifestFiles.cs
Outdated
| if (!PublishFlags.IsSatelliteIncludedByDefault(satelliteCulture, _targetCulture, _includeAllSatellites)) | ||
| { | ||
| continue; | ||
| } | ||
| else | ||
| { | ||
| satelliteRefAssemblyMap.Add(item); | ||
| } |
There was a problem hiding this comment.
for readability I'd prefer
| if (!PublishFlags.IsSatelliteIncludedByDefault(satelliteCulture, _targetCulture, _includeAllSatellites)) | |
| { | |
| continue; | |
| } | |
| else | |
| { | |
| satelliteRefAssemblyMap.Add(item); | |
| } | |
| if (PublishFlags.IsSatelliteIncludedByDefault(satelliteCulture, _targetCulture, _includeAllSatellites)) | |
| { | |
| satelliteRefAssemblyMap.Add(item); | |
| } | |
| else | |
| { | |
| continue; | |
| } |
src/Tasks/ResolveManifestFiles.cs
Outdated
| continue; | ||
| } | ||
|
|
||
| // If we get a resource assembly in managed references, determine whether to be publish it based on _targetCulture |
There was a problem hiding this comment.
Would it be accurate to say
| // If we get a resource assembly in managed references, determine whether to be publish it based on _targetCulture | |
| // apply the culture publishing rules to managed references as well as satellites |
?
There was a problem hiding this comment.
updated with tweaks.
src/Tasks/ResolveManifestFiles.cs
Outdated
| @@ -52,6 +52,8 @@ public sealed class ResolveManifestFiles : TaskExtension | |||
| // if signing manifests is on and not all app files are included, then the project can't be published. | |||
| private bool _canPublish; | |||
| private Dictionary<string, ITaskItem> _runtimePackAssets; | |||
| // map of satellite assemblies that are included in References | |||
| private SatelliteRefAssemblyMap satelliteRefAssemblyMap = new SatelliteRefAssemblyMap(); | |||
There was a problem hiding this comment.
I found this name unclear later on. Maybe call it _satelliteAssembliesPassedAsReferences?
(also nit: our style is that this should start with an _)
rainersigwald
added
the
merge-when-branch-open
Fixes AB#1449000
Context
Satellite assemblies are not being handled correctly after ClickOnce changes for .NET Core publishing were made in VS 16.8.
Following issues have been observed post those changes:
If packages publish en-us resource assembly for other locales as well, we will end up adding all of them in the ClickOnce manifest. Since the strong name signature will be identical for all these resource assemblies, we will now have multiple assemblies with identical signature in the ClickOnce manifest. This will cause CO Install to fail.
The References item list can also contain resource assemblies. If these assemblies are also present in the SatelliteAssembly item list, we will now end up writing 2 entries for the same assembly in the manifest. This will cause CO install to fail.
Changes Made
When we process the References item list, we will maintain a dictionary for satellite assemblies that are getting included in the list of assemblies. Later when we process the SatelliteAssemblies item list, we will look up in this dictionary and skip the entries that are already included through the References list.
Testing
Ongoing
Notes