Checkks all custom actions for cycles. Version 3. Fixes wixtoolset/issues#6201
Ronald Martin committed
Feb 12, 2021 at 22:37 UTC
9dd02e0c1bb6463de5d79f14ce06d078ef876a32
3 files changed
+114
-21
src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs
+63
-15
@@ -67,7 +67,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
67
}
68
else // not a supported unscheduled action.
69
{
70
- throw new InvalidOperationException("Found an action with no Sequence, Before, or After column set.");
70
+ throw new InvalidOperationException($"Found an action [{actionSymbol.Id.Id}] at [{actionSymbol.SourceLineNumbers}] with no Sequence, Before, or After column set.");
71
}
72
}
73
@@ -129,6 +129,9 @@ namespace WixToolset.Core.WindowsInstaller.Bind
129
}
130
}
131
132
+ // A dictionary used for detecting cyclic references among action symbols.
133
+ var firstReference = new Dictionary<WixActionSymbol, WixActionSymbol>();
134
+
135
// Build up dependency trees of the relatively scheduled actions.
136
// Use ToList() to create a copy of the required action symbols so that new symbols can
137
// be added while enumerating.
@@ -142,7 +145,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
145
this.Messaging.Write(ErrorMessages.StandardActionRelativelyScheduledInModule(actionSymbol.SourceLineNumbers, actionSymbol.SequenceTable.ToString(), actionSymbol.Action));
146
}
147
145
- this.SequenceActionSymbol(actionSymbol, requiredActionSymbols);
148
+ this.SequenceActionSymbol(actionSymbol, requiredActionSymbols, firstReference);
149
}
150
else if (SectionType.Module == this.Section.Type && 0 < actionSymbol.Sequence && !WindowsInstallerStandard.IsStandardAction(actionSymbol.Action)) // check for custom actions and dialogs that have a sequence number
151
{
@@ -566,7 +569,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
569
/// </summary>
570
/// <param name="actionSymbol">The action symbol to be sequenced.</param>
571
/// <param name="requiredActionSymbols">Collection of actions which must be included.</param>
569
- private void SequenceActionSymbol(WixActionSymbol actionSymbol, Dictionary<string, WixActionSymbol> requiredActionSymbols)
572
+ private void SequenceActionSymbol(WixActionSymbol actionSymbol, Dictionary<string, WixActionSymbol> requiredActionSymbols, Dictionary<WixActionSymbol, WixActionSymbol> firstReference)
573
{
574
var after = false;
575
@@ -576,7 +579,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
579
}
580
else if (actionSymbol.Before == null)
581
{
579
- throw new InvalidOperationException("Found an action with no Sequence, Before, or After column set.");
582
+ throw new InvalidOperationException($"Found an action [{actionSymbol.Id.Id}] at [{actionSymbol.SourceLineNumbers}] with no Sequence, Before, or After column set.");
583
}
584
585
var parentActionName = (after ? actionSymbol.After : actionSymbol.Before);
@@ -597,10 +600,8 @@ namespace WixToolset.Core.WindowsInstaller.Bind
600
throw new InvalidOperationException(String.Format(CultureInfo.CurrentUICulture, "Found an action with a non-existent {0} action: {1}.", (after ? "After" : "Before"), parentActionName));
601
}
602
}
600
- else if (actionSymbol == parentActionSymbol || this.ContainsChildActionSymbol(actionSymbol, parentActionSymbol)) // cycle detected
601
- {
602
- throw new WixException(ErrorMessages.ActionCircularDependency(actionSymbol.SourceLineNumbers, actionSymbol.SequenceTable.ToString(), actionSymbol.Action, parentActionSymbol.Action));
603
- }
603
+
604
+ this.CheckForCircularActionReference(actionSymbol, requiredActionSymbols, firstReference);
605
606
// Add this action to the appropriate list of dependent action symbols.
607
var relativeActions = this.GetRelativeActions(parentActionSymbol);
@@ -608,19 +609,66 @@ namespace WixToolset.Core.WindowsInstaller.Bind
609
relatedSymbols.Add(actionSymbol);
610
}
611
611
- private bool ContainsChildActionSymbol(WixActionSymbol childSymbol, WixActionSymbol parentSymbol)
612
+ /// <summary>
613
+ /// Check the specified action symbol to see if it leads to a cycle.
614
+ /// </summary>
615
+ /// <para> Use the provided dictionary to note the initial action symbol that first led to each action
616
+ /// symbol. Any action symbol encountered that has already been encountered starting from a different
617
+ /// initial action symbol inherits the loop characteristics of that initial action symbol, and thus is
618
+ /// also not part of a cycle. However, any action symbol encountered that has already been encountered
619
+ /// starting from the same initial action symbol is an indication that the current action symbol is
620
+ /// part of a cycle.
621
+ /// </para>
622
+ /// <param name="actionSymbol">The action symbol to be checked.</param>
623
+ /// <param name="requiredActionSymbols">Collection of actions which must be included.</param>
624
+ /// <param name="firstReference">The first encountered action symbol that led to each action symbol.</param>
625
+ private void CheckForCircularActionReference(WixActionSymbol actionSymbol, Dictionary<string, WixActionSymbol> requiredActionSymbols, Dictionary<WixActionSymbol, WixActionSymbol> firstReference)
626
{
613
- var result = false;
627
+ WixActionSymbol currentActionSymbol = null;
628
+ var parentActionSymbol = actionSymbol;
629
615
- if (this.RelativeActionsForActions.TryGetValue(childSymbol.Id.Id, out var relativeActions))
630
+ do
631
{
617
- result = relativeActions.NextActions.Any(a => a.SequenceTable == parentSymbol.SequenceTable && a.Id.Id == parentSymbol.Id.Id) ||
618
- relativeActions.PreviousActions.Any(a => a.SequenceTable == parentSymbol.SequenceTable && a.Id.Id == parentSymbol.Id.Id);
619
- }
632
+ var previousActionSymbol = currentActionSymbol ?? parentActionSymbol;
633
+ currentActionSymbol = parentActionSymbol;
634
621
- return result;
635
+ if (!firstReference.TryGetValue(currentActionSymbol, out var existingInitialActionSymbol))
636
+ {
637
+ firstReference[currentActionSymbol] = actionSymbol;
638
+ }
639
+ else if (existingInitialActionSymbol == actionSymbol)
640
+ {
641
+ throw new WixException(ErrorMessages.ActionCircularDependency(currentActionSymbol.SourceLineNumbers, currentActionSymbol.SequenceTable.ToString(), currentActionSymbol.Action, previousActionSymbol.Action));
642
+ }
643
+
644
+ parentActionSymbol = this.GetParentActionSymbol(currentActionSymbol, requiredActionSymbols);
645
+ } while (null != parentActionSymbol);
646
}
647
648
+ /// <summary>
649
+ /// Get the action symbol that is the parent of the given action symbol.
650
+ /// </summary>
651
+ /// <param name="actionSymbol">The given action symbol.</param>
652
+ /// <param name="requiredActionSymbols">Collection of actions which must be included.</param>
653
+ /// <returns>Null if there is no parent. Used for loop termination.</returns>
654
+ private WixActionSymbol GetParentActionSymbol(WixActionSymbol actionSymbol, Dictionary<string, WixActionSymbol> requiredActionSymbols)
655
+ {
656
+ if (null == actionSymbol.Before && null == actionSymbol.After)
657
+ {
658
+ return null;
659
+ }
660
+
661
+ var parentActionKey = actionSymbol.SequenceTable.ToString() + "/" + (actionSymbol.After ?? actionSymbol.Before);
662
+
663
+ if (!requiredActionSymbols.TryGetValue(parentActionKey, out var parentActionSymbol))
664
+ {
665
+ WindowsInstallerStandard.TryGetStandardAction(parentActionKey, out parentActionSymbol);
666
+ }
667
+
668
+ return parentActionSymbol;
669
+ }
670
+
671
+
672
private RelativeActions GetRelativeActions(WixActionSymbol action)
673
{
674
if (!this.RelativeActionsForActions.TryGetValue(action.Id.Id, out var relativeActions))
src/test/WixToolsetTest.CoreIntegration/CustomActionFixture.cs
+31
-6
@@ -11,7 +11,7 @@ namespace WixToolsetTest.CoreIntegration
11
12
public class CustomActionFixture
13
{
14
- [Fact(Skip = "https://github.com/wixtoolset/issues/issues/6201")]
14
+ [Fact]
15
public void CanDetectCustomActionCycle()
16
{
17
var folder = TestData.Get(@"TestData");
@@ -22,8 +22,8 @@ namespace WixToolsetTest.CoreIntegration
22
var intermediateFolder = Path.Combine(baseFolder, "obj");
23
var msiPath = Path.Combine(baseFolder, @"bin\test.msi");
24
25
- var result = WixRunner.Execute(new[]
26
- {
25
+ var exception = Assert.Throws<WixException>(() => WixRunner.Execute(new[]
26
+ {
27
"build",
28
Path.Combine(folder, "CustomAction", "CustomActionCycle.wxs"),
29
Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"),
@@ -31,10 +31,35 @@ namespace WixToolsetTest.CoreIntegration
31
"-bindpath", Path.Combine(folder, "SingleFile", "data"),
32
"-intermediateFolder", intermediateFolder,
33
"-o", msiPath
34
- });
34
+ }));
35
+
36
+ Assert.Equal("The InstallExecuteSequence table contains an action 'Action1' that is scheduled to come before or after action 'Action3', which is also scheduled to come before or after action 'Action1'. Please remove this circular dependency by changing the Before or After attribute for one of the actions.", exception.Message);
37
+ }
38
+ }
39
+
40
+ [Fact]
41
+ public void CanDetectCustomActionCycleWithTail()
42
+ {
43
+ var folder = TestData.Get(@"TestData");
44
+
45
+ using (var fs = new DisposableFileSystem())
46
+ {
47
+ var baseFolder = fs.GetFolder();
48
+ var intermediateFolder = Path.Combine(baseFolder, "obj");
49
+ var msiPath = Path.Combine(baseFolder, @"bin\test.msi");
50
+
51
+ var exception = Assert.Throws<WixException>(() => WixRunner.Execute(new[]
52
+ {
53
+ "build",
54
+ Path.Combine(folder, "CustomAction", "CustomActionCycleWithTail.wxs"),
55
+ Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"),
56
+ Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
57
+ "-bindpath", Path.Combine(folder, "SingleFile", "data"),
58
+ "-intermediateFolder", intermediateFolder,
59
+ "-o", msiPath
60
+ }));
61
36
- var warnings = result.Messages.Where(m => m.Level == MessageLevel.Warning).ToArray();
37
- Assert.False(result.ExitCode == 0 && warnings.Length == 0);
62
+ Assert.Equal("The InstallExecuteSequence table contains an action 'Action2' that is scheduled to come before or after action 'Action4', which is also scheduled to come before or after action 'Action2'. Please remove this circular dependency by changing the Before or After attribute for one of the actions.", exception.Message);
63
}
64
}
65
src/test/WixToolsetTest.CoreIntegration/TestData/CustomAction/CustomActionCycleWithTail.wxs
new
+20
@@ -0,0 +1,20 @@
1
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
2
+ <Fragment>
3
+ <ComponentGroup Id="ProductComponents">
4
+ <ComponentGroupRef Id="MinimalComponentGroup" />
5
+ </ComponentGroup>
6
+
7
+ <Binary Id="Binary1" SourceFile="test.txt" />
8
+ <CustomAction Id="Action1" DllEntry="EntryPoint1" BinaryRef="Binary1" />
9
+ <CustomAction Id="Action2" DllEntry="EntryPoint2" BinaryRef="Binary1" />
10
+ <CustomAction Id="Action3" DllEntry="EntryPoint3" BinaryRef="Binary1" />
11
+ <CustomAction Id="Action4" DllEntry="EntryPoint4" BinaryRef="Binary1" />
12
+
13
+ <InstallExecuteSequence>
14
+ <Custom Action="Action1" After="Action2" />
15
+ <Custom Action="Action2" After="Action3" />
16
+ <Custom Action="Action3" After="Action4" />
17
+ <Custom Action="Action4" After="Action2" />
18
+ </InstallExecuteSequence>
19
+ </Fragment>
20
+</Wix>
\ No newline at end of file