Fix discovery of payloads with mo container that need to be assigned …

[nirbar]

…to the default container. See #6174

WiX v3.x pull requests

Active development has now moved to WiX v4.

• Pull requests for new features are extremely unlikely to be accepted for WiX v3.x.
• Pull requests for important bug fixes might be accepted.

Pull requests for WiX v3.x must be approved before they can be reviewed or accepted.

• If an issue doesn't exist for the problem, create one.
• Provide lots of detail so someone just reading the issue can get a good understanding of the problem.
• If possible, attend the biweekly (fortnightly) online meeting for discussion. Meetings are announced on the wix-devs mailing list.

[rseanhall]

Thanks. We would need this fixed in v4 first before considering whether to take it in v3.14.

Maybe I'm missing something, but I don't understand how this fixes the issue of the cab file not being assigned to the detached container.

wixtoolset/issues#6174

[nirbar]

The original implementation had a list of all payloads that has a container.
MSI's external cab files were added in ChainPackageInfo so the list was not aware of these cab files.
Thus the list did not contain the cab payloads so these were added to the default container although they were already in the detached container

[rseanhall]

If I understand your explanation, can't you do this with in a couple lines by adding

if (!String.IsNullOrEmpty(payload.Container))
{
    continue;
}
else if (!PackagingType.Embedded == payload.Packaging)

and not doing anything else? The less lines touched, the better.

Also, that targeted fix seems incomplete without further investigation. The fact that the payload isn't added to the ContainerInfo's Payloads around line 3427 makes me wonder if other things are broken.

[nirbar]

The cab payloads are added to the container in ChainPackageInfo.ResolveMsiPackage() line 863.

I preferred this solution to make the code cleaner, as the only use of payloadsAddedToContainers was to detect which payloads belong to no container.
I think using Linq is more readable

[rseanhall]

Thanks.

Agreed, your code is cleaner. I probably would still take this as is. But at this point of v3's lifecycle, I would argue that it's more important that the diffs are cleaner. This PR is +17/-27 with changes scattered through the file. My proposed change would be +5/-1 with all of the lines together. Of course, all of this doesn't really matter until this is fixed (or verified as already fixed) in v4 and we agree to take this fix in v3.14.

[nirbar]

Well... most diffs are due to "Remove and Sort Usings" in VS

[BMurri]

Remove and sort usings are fine in v4 but not in v3 at this point in time.

Please remove extraneous code changes so we can better review the actual changes you propose.

[rseanhall]

This fix is more risky than I would like. If more people complain, then we might consider the small targeted fix that I proposed.