Burn duplicates autogenerated ExePackage/@CacheId

[wixbot]

Consider a Bundle with two ExePackages with the same @sourcefile but different PayLoad. Both packages will have the same autogenerated CacheId. If one of the packages is cached, the other is marked partial (because of the same exe). The 'partial' package will be uncached and because of the shared @cacheid the other package gets uncached too!
Which algorithm is used to generate the CacheId for an Exepackage?

Originally opened by erict

[wixbot]

Which version of WiX are you using?

[wixbot]

Version 3.9.1006.0.

Originally posted by erict

[wixbot]

Detect duplicate cache ids at bind time and fail.

Release changed from v3.9 to v3.x

[rseanhall]

Although the CacheId attribute is planned to be removed (#6329), the duplicate check still needs to be done.

[cpuwzd]

I tried working on this issue, but the failing test is not valid. When I enable it, it passes because the test is looking for non-specific errors. Sure enough, non-specific errors occur. The code that generates the errors is:

            // Compiler_Bundle.cs, lines 2196 - 2208

            // Detect condition is recommended or required for Exe and Msu packages
            // (depending on whether uninstall arguments were provided).
            if ((packageType == WixBundlePackageType.Exe || packageType == WixBundlePackageType.Msu) && String.IsNullOrEmpty(detectCondition))
            {
                if (String.IsNullOrEmpty(uninstallArguments))
                {
                    this.Core.Write(WarningMessages.DetectConditionRecommended(sourceLineNumbers, node.Name.LocalName));
                }
                else
                {
                    this.Core.Write(ErrorMessages.ExpectedAttributeWithValueWithOtherAttribute(sourceLineNumbers, node.Name.LocalName, "DetectCondition", "UninstallArguments"));
                }
            }

Adding DetectCondition="anything" to each ExePackage makes these two errors go away. Now the test fails as intended.

I don't know how far the "build" will get under WixRunner, but it would need to get to burn in order to check duplicate cache id's. Having DetectCondition true would prevent both copies from being cached. Having them both false leaves us with the errors I first mentioned at the beginning of this comment. Can I assume that burn is going to evaluate DetectCondition as an expression? If so, what could I pass that will evaluate to false so that the caching will be attempted?

I haven't gotten into burn, itself, yet. Is this failing test in the wrong repo? Is there a test similar enough to this one that would help me sniff out the missing bits?

Ron Martin

[rseanhall]

The test is in the right repo, this needs to be an error at build time. It was valid when it was written, the DetectConditionRecommended warning was added later. So yes, adding DetectCondition="anything" is necessary to make these tests valid again.

[barnson]

A downside to long-Skipped tests...

[cpuwzd]

Thanks, Sean. Does the auto-generation of the cache id's combine or separate exe's with the same cache id across bundles? Do you think it would be better to use an external collection to track duplicates or do you think that it is doable within the Symbol mechanism?/ Ron

…

On 4/4/2021 10:30 PM, Sean Hall wrote: The test is in the right repo, this needs to be an error at build time. It was valid when it was written, the |DetectConditionRecommended| warning was added later. So yes, adding |DetectCondition="anything"| is necessary to make these tests valid again. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4628 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHIYUBTFSSEEMGSWFIKXVB3THEOD3ANCNFSM4WNUKHXA>.

[rseanhall]

It is valid for multiple bundles to have a package with the same CacheId. Reusing a CacheId like that has restrictions, but we can't enforce those since we only build one bundle at a time.

Auto-generated CacheIds are generated at bind time. The Symbol mechanism takes place at link time, which is too early.

[cpuwzd]

Failing tests should be aged like accounts receivable.

…

On 4/4/2021 10:45 PM, Bob Arnson wrote: A downside to long-|Skip|ped tests... — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4628 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHIYUBV3Y6HWUBCPNOYKWUDTHEP37ANCNFSM4WNUKHXA>.

[cpuwzd]

OK, so I'm just trying to prevent the burn pass from running when there is a CacheId collision within the same bundle. The feature request is going to require more planning and discussion. I don't think that I'm ready to take on a chunk of work that large at this time. There's still a lot of code for me to absorb. I'm still trying to learn all of the phases that the processing goes through (including the general responsibilities of each phase) and to sort out the history of names and naming conventions. I should be able to use a suitable collection, initialized for each bundle, to hold a set of unique cache id's. I'd use the messaging system to report the error, which would terminate the operation unless errors are being ignored. Should this error ever be downgraded to a warning? Does/should processing generally continue to the end of a phase (in an effort to report as many errors as possible in a single build) or end when the first error/warning is discovered?

…

On 4/5/2021 10:44 AM, Sean Hall wrote: It is valid for multiple bundles to have a package with the same |CacheId|. Reusing a |CacheId| like that has restrictions, but we can't enforce those since we only build one bundle at a time. Auto-generated |CacheId|s are generated at bind time. The Symbol mechanism takes place at link time, which is too early. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4628 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHIYUBRB4CZESPLSZ2VXEZTTHHEFPANCNFSM4WNUKHXA>.