Fix update of shared transitive dependencies

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - #1122: Packaging should recognize resources in dependency modules set to deploy
 - #1119: The update command should check version requirements using post-update values instead of what's currently installed
 - #1097: The Test resource processor now supports nested tests
+- #430: Updating shared transitive dependencies with lock-step version requirements now works instead of erroring out
 
 ### Security
 - requests Python wheel updated to 2.33.0

src/cls/IPM/Storage/Module.cls

@@ -159,12 +159,12 @@ Method HandleAllUpdateSteps(
 }
 
 /// For a given version, seed or execute update steps listed in the module's update package version class
-/// 
+///
 /// If seeding: For only steps that don't have a TimeStampEnd value or have an errored status, set it's TimeStampEnd to the current time
-/// 
+///
 /// If executing: Only start running steps at the first one found not to have a TimeStampEnd value or if an errored step is found
 ///               After finding this step, run it and all subsequent steps, regardless of if they have been run or not.
-/// 
+///
 /// newStepToApply - Gets marked as true to tell us to run all remaining steps in the list, whether or not they have been run/seeded before
 ///                  Can carry this over to subsequent calls by passing the newStepToApply variable to those HandleUpdateStepsFromList() calls
 Method HandleUpdateStepsFromList(
@@ -886,6 +886,7 @@ ClassMethod HasScope(
 /// <var>pIgnoreInstalledModules</var> - If true, don't use already-installed modules.<br>
 /// <var>pPermitDowngrade</var> - If true, allow downgrading installed modules.<br>
 /// <var>pCheckNestedScoped</var> - If true, check scoped dependencies against root module phases.<br>
+/// <var>pCoUpdatingModules</var> - %List of module names being co-updated in the same batch; excluded from installed-version SQL check to avoid stale constraint conflicts.<br>
 /// NOTE: Other parameters are placeholders to match signature of original BuildDependencyGraph method for backward compatibility.
 Method BuildDependencyGraph(
 	ByRef pDependencyGraph,
@@ -901,7 +902,8 @@ Method BuildDependencyGraph(
 	pPermitDowngrade As %Boolean = 0,
 	pCheckNestedScoped As %Boolean = 0,
 	ByRef pIgnore7,
-	pIgnore8) As %Status [ Internal ]
+	pIgnore8,
+	pCoUpdatingModules As %List = "") As %Status [ Internal ]
 {
     set sc = $$$OK
     try {
@@ -985,7 +987,8 @@ Method BuildDependencyGraph(
                         // Working data
                         .pDependencyGraph, .workQueue,
                         // Caches
-                        .repoSearchCache, .manifestCache, .moduleCache)
+                        .repoSearchCache, .manifestCache, .moduleCache,
+                        pCoUpdatingModules)
                     $$$ThrowOnError(sc)
                 }
 
@@ -1005,7 +1008,8 @@ Method BuildDependencyGraph(
                         // Working data
                         .pDependencyGraph, .workQueue,
                         // Caches
-                        .repoSearchCache, .manifestCache, .moduleCache)
+                        .repoSearchCache, .manifestCache, .moduleCache,
+                        pCoUpdatingModules)
                     // For fuzzy, continue even if one fails
                     if $$$ISERR(sc) {
                         // Error will surface later if unresolvable
@@ -1057,6 +1061,7 @@ Method BuildDependencyGraph(
 /// <var>pRepoSearchCache</var> - Cache of repository search results<br>
 /// <var>pManifestCache</var> - Cache of retrieved module manifests<br>
 /// <var>pModuleCache</var> - Cache of loaded module objects<br>
+/// <var>pCoUpdatingModules</var> - %List of module names being co-updated in the same batch; excluded from installed-version SQL check to avoid stale constraint conflicts.<br>
 Method ProcessSingleDependencyIterative(
 	pIsExact As %Boolean,
 	pDep As %IPM.Storage.ModuleReference,
@@ -1072,7 +1077,8 @@ Method ProcessSingleDependencyIterative(
 	ByRef pWorkQueue,
 	ByRef pRepoSearchCache,
 	ByRef pManifestCache,
-	ByRef pModuleCache) As %Status [ Internal, Private ]
+	ByRef pModuleCache,
+	pCoUpdatingModules As %List = "") As %Status [ Internal, Private ]
 {
     set sc = $$$OK
     try {
@@ -1098,7 +1104,8 @@ Method ProcessSingleDependencyIterative(
 
         if 'pIgnoreInstalledModules {
             // Apply requirements from other installed modules
-            set otherDepsList = pKnownDependencies
+            // Excludes sibling modules that are also being concurrently updated
+            set otherDepsList = pKnownDependencies _ pCoUpdatingModules
             set existingDepKey = ""
             for {
                 set existingDepKey = $order(pDependencyGraph(existingDepKey))
@@ -1435,7 +1442,7 @@ Method OverrideLifecycleClassSet(pValue As %Dictionary.Classname) As %Status
 
 /// This callback method is invoked by the <METHOD>%New</METHOD> method to
 /// provide notification that a new instance of an object is being created.
-/// 
+///
 /// <P>If this method returns an error then the object will not be created.
 /// <p>It is passed the arguments provided in the %New call.
 /// When customizing this method, override the arguments with whatever variables and types you expect to receive from %New().
@@ -1451,7 +1458,7 @@ Method %OnNew() As %Status [ Private, ServerOnly = 1 ]
 
 /// This callback method is invoked by the <METHOD>%Open</METHOD> method to
 /// provide notification that the object specified by <VAR>oid</VAR> is being opened.
-/// 
+///
 /// <P>If this method returns an error then the object will not be opened.
 Method %OnOpen() As %Status [ Private, ServerOnly = 1 ]
 {
@@ -1481,7 +1488,7 @@ Method %OnOpen() As %Status [ Private, ServerOnly = 1 ]
 
 /// This callback method is invoked by the <METHOD>%ValidateObject</METHOD> method to
 /// provide notification that the current object is being validated.
-/// 
+///
 /// <P>If this method returns an error then <METHOD>%ValidateObject</METHOD> will fail.
 Method %OnValidateObject() As %Status [ Private, ServerOnly = 1 ]
 {
@@ -1570,7 +1577,7 @@ Method %OnValidateObject() As %Status [ Private, ServerOnly = 1 ]
 /// either because %Save() was invoked on this object or on an object that references this object.
 /// %OnAddToSaveSet can modify the current object. It can also add other objects to the current
 /// SaveSet by invoking %AddToSaveSet or remove objects by calling %RemoveFromSaveSet.
-/// 
+///
 /// <P>If this method returns an error status then %Save() will fail and the transaction
 /// will be rolled back.
 Method %OnAddToSaveSet(
@@ -1605,7 +1612,7 @@ Method %OnAddToSaveSet(
 }
 
 /// Get an instance of an XML enabled class.<br><br>
-/// 
+///
 /// You may override this method to do custom processing (such as initializing
 /// the object instance) before returning an instance of this class.
 /// However, this method should not be called directly from user code.<br>
@@ -1836,9 +1843,9 @@ Method %Evaluate(
 /// This callback method is invoked by the <METHOD>%Save</METHOD> method to
 /// provide notification that the object is being saved. It is called before
 /// any data is written to disk.
-/// 
+///
 /// <P><VAR>insert</VAR> will be set to 1 if this object is being saved for the first time.
-/// 
+///
 /// <P>If this method returns an error then the call to <METHOD>%Save</METHOD> will fail.
 Method %OnBeforeSave(insert As %Boolean) As %Status [ Private, ServerOnly = 1 ]
 {

src/cls/IPM/Utils/Module.cls

@@ -349,7 +349,7 @@ ClassMethod GetModuleObjectFromString(
 
 /// Returns a semantic version expression capturing all version requirements for a given module name in the current namespace.
 /// A list of modules to exclude may be provided (for example, if these modules would be updated at the same time).
-/// 
+///
 /// If provided a dependency graph, will use versions defined there instead of what the SQL call returns.
 /// This is especially important for the update command, where we want to check requirements with post-update versions as opposed to what's currently installed.
 ClassMethod GetRequiredVersionExpression(
@@ -991,7 +991,7 @@ ClassMethod GetModuleNameFromXML(
 ///     <Parameter Name="NoLock">1</Parameter>
 /// </Defaults>
 /// ```
-/// 
+///
 /// Returns results as multidimensional array
 ClassMethod GetModuleDefaultsFromXML(
 	pDirectory As %String,
@@ -1295,8 +1295,9 @@ ClassMethod LoadDependencies(
     set reloadSnapshots = +$get(pParams("UpdateSnapshots"))
     set permitDowngrade = +$get(pParams("PermitDowngrade"))
     set ignoreInstalled = +$get(pParams("IgnoreInstalled"))
+    set coUpdatingModules = $get(pParams("CoUpdatingModules"))
     write !, "Building dependency graph..." // Intentionally always write even in non-verbose mode
-    set sc = pModule.BuildDependencyGraph(.dependencyGraph,,reloadSnapshots,,phaseList,,,,ignoreInstalled,,permitDowngrade)
+    set sc = pModule.BuildDependencyGraph(.dependencyGraph,,reloadSnapshots,,phaseList,,,,ignoreInstalled,,permitDowngrade,,,,.coUpdatingModules)
     $$$ThrowOnError(sc)
     write "Done."
 
@@ -1314,6 +1315,12 @@ ClassMethod LoadDependencies(
         write !
     }
 
+    set coUpdatingModules = ""
+    for i = 1:1:flatDepList.Count() {
+        set coUpdatingModules = coUpdatingModules _ $listbuild(flatDepList.GetAt(i).Name)
+    }
+    set pParams("CoUpdatingModules") = coUpdatingModules
+
     for i = 1:1:flatDepList.Count() {
         #dim moduleReference As %IPM.Storage.QualifiedModuleInfo
         set moduleReference = flatDepList.GetAt(i)

tests/integration_tests/Test/PM/Integration/Update.cls

@@ -641,6 +641,71 @@ Method TestInterchangeableNameUpdate()
     $$$ThrowOnError(##class(%IPM.Main).Shell("repo -n zot -delete"))
 }
 
+/// Tests that updating a module whose dependencies share a transitive dependency (simple diamond)
+/// succeeds without a version conflict error (pre-fix: ERROR #5001: Exact version matches do not agree).
+/// All modules update from 1.0.0 to 1.1.0.
+///     parent
+///     /    \
+///    a      b
+///     \    /
+///      base
+Method TestSharedTransitiveDependencyUpdate()
+{
+    set v1Dir = ..GetModuleDir("update-shared-transitive-dep", "simple-diamond", "1.0.0")
+    set v2Dir = ..GetModuleDir("update-shared-transitive-dep", "simple-diamond", "1.1.0")
+
+    set sc = ##class(%IPM.Main).Shell("load " _ v1Dir)
+    do $$$AssertStatusOK(sc, "Loaded update-shared-transitive-dep 1.0.0")
+
+    set sc = ##class(%IPM.Main).Shell("update update-shared-transitive-dep-parent -path " _ v2Dir)
+    do $$$AssertStatusOK(sc, "Updated to 1.1.0 without version conflict error")
+
+    for modName = "update-shared-transitive-dep-parent","update-shared-transitive-dep-a","update-shared-transitive-dep-b","update-shared-transitive-dep-base" {
+        set mod = ##class(%IPM.Storage.Module).NameOpen(modName, , .openSC)
+        do $$$AssertStatusOK(openSC, modName _ " opened after update")
+        if $$$ISOK(openSC) {
+            do $$$AssertEquals(mod.VersionString, "1.1.0", modName _ " updated to 1.1.0")
+        }
+    }
+
+    do ..CleanUp()
+}
+
+/// Tests that updating a module with an offset-diamond dependency topology succeeds without a version
+/// conflict error. All modules update from 1.0.0 to 1.1.0.
+/// Without the fix, installing c 1.1.0 sees stale d 1.0.0 in the DB constraining base to =1.0.0,
+/// conflicting with c's =1.1.0 requirement.
+///     parent
+///    /  |   \
+///   a   b    |
+///    \  /    |
+///    mid     d
+///     |      |
+///     c      |
+///      \    /
+///       base
+Method TestOffsetDiamondUpdate()
+{
+    set v1Dir = ..GetModuleDir("update-shared-transitive-dep", "offset-diamond", "1.0.0")
+    set v2Dir = ..GetModuleDir("update-shared-transitive-dep", "offset-diamond", "1.1.0")
+
+    set sc = ##class(%IPM.Main).Shell("load " _ v1Dir)
+    do $$$AssertStatusOK(sc, "Loaded offset-diamond 1.0.0")
+
+    set sc = ##class(%IPM.Main).Shell("update update-shared-transitive-dep-od-parent -path " _ v2Dir)
+    do $$$AssertStatusOK(sc, "Updated to 1.1.0 without version conflict error")
+
+    for modName = "update-shared-transitive-dep-od-parent","update-shared-transitive-dep-od-a","update-shared-transitive-dep-od-b","update-shared-transitive-dep-od-d","update-shared-transitive-dep-od-mid","update-shared-transitive-dep-od-c","update-shared-transitive-dep-od-base" {
+        set mod = ##class(%IPM.Storage.Module).NameOpen(modName, , .openSC)
+        do $$$AssertStatusOK(openSC, modName _ " opened after update")
+        if $$$ISOK(openSC) {
+            do $$$AssertEquals(mod.VersionString, "1.1.0", modName _ " updated to 1.1.0")
+        }
+    }
+
+    do ..CleanUp()
+}
+
 /// Tests that dev mode is not propagated to dependencies during update
 Method TestDevModePropagation()
 {

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.0.0/.modules/a/module.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-a.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-a</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+      <Dependencies>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-mid</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+      </Dependencies>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.0.0/.modules/b/module.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-b.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-b</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+      <Dependencies>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-mid</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+      </Dependencies>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.0.0/.modules/base/module.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-base.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-base</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.0.0/.modules/c/module.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-c.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-c</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+      <Dependencies>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-base</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+      </Dependencies>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.0.0/.modules/d/module.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-d.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-d</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+      <Dependencies>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-base</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+      </Dependencies>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.0.0/.modules/mid/module.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-mid.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-mid</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+      <Dependencies>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-c</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+      </Dependencies>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.0.0/module.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-parent.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-parent</Name>
+      <Version>1.0.0</Version>
+      <Packaging>module</Packaging>
+      <Dependencies>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-a</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-b</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-d</Name>
+          <Version>1.0.0</Version>
+        </ModuleReference>
+      </Dependencies>
+    </Module>
+  </Document>
+</Export>

tests/integration_tests/Test/PM/Integration/_data/update-shared-transitive-dep/offset-diamond/1.1.0/.modules/a/module.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Export generator="Cache" version="25">
+  <Document name="update-shared-transitive-dep-od-a.ZPM">
+    <Module>
+      <Name>update-shared-transitive-dep-od-a</Name>
+      <Version>1.1.0</Version>
+      <Packaging>module</Packaging>
+      <Dependencies>
+        <ModuleReference>
+          <Name>update-shared-transitive-dep-od-mid</Name>
+          <Version>1.1.0</Version>
+        </ModuleReference>
+      </Dependencies>
+    </Module>
+  </Document>
+</Export>