Don't update dependencies more than once

corral/cmd/_test_cmd_update.pony

@@ -10,6 +10,8 @@ actor _TestCmdUpdate is TestList
 
   fun tag tests(test: PonyTest) =>
     test(_TestEmptyDeps)
+    test(_TestRegression120)
+
 
 class iso _TestEmptyDeps is UnitTest
   fun name(): String =>
@@ -37,6 +39,44 @@ class iso _TestEmptyDeps is UnitTest
     h.long_test(2_000_000_000)
 
 
+class iso _TestRegression120 is UnitTest
+  fun name(): String =>
+    "cmd/update/" + __loc.type_name()
+
+  fun apply(h: TestHelper) ? =>
+    """
+    Issue #120 identified a problem with transitive dependencies that resulted
+    in too many operations being performaned across the loading of all
+    dependencies.
+
+    The test as currently constituted, consists of a bundles with 2
+    dependencies. One of those has 2 more in a transitive fashion, that should
+    result in 4 syncs and corresponding actions happening. However, due to a
+    bug in _Updater, it currently does 11.
+
+    With a real VCS like Git, the number that results from it is variable
+    based on timing. This test exists to prove that issue #120 is fixed and
+    to prevent a similar bug from being introduced in the future.
+    """
+    // given
+    let auth = h.env.root as AmbientAuth
+    let log = Log(LvlNone, h.env.err, SimpleLogFormatter)
+    let fp: FilePath = _TestData.file_path_from(h, "regression-120/bundle-entrypoint")?
+    let repo_cache = _TestRepoCache(auth)?
+    let ctx = Context(h.env, log, log, false, repo_cache)
+    let project = Project(auth, log, fp)
+    let bundle = Bundle.load(fp, log)?
+    let recorder = _OpsRecorder(h, 4, 4, 4)
+    let vcs_builder: VCSBuilder = _TestCmdUpdateVCSBuilder(recorder)
+
+    // when
+    let updater = _Updater(ctx, project, consume bundle, vcs_builder, recorder)
+
+    // when updater is finished, it will send a `cmd_completed` message to
+    // _OpsRecorder which will trigger pass/fail
+    h.long_test(2_000_000_000)
+
+
 actor _OpsRecorder is CmdResultReceiver
   let _h: TestHelper
 

corral/cmd/cmd_update.pony

@@ -30,6 +30,7 @@ actor _Updater
 
   let deps_seen: Map[Locator, Dep] ref = deps_seen.create()
   let deps_to_load: Map[Locator, Dep] ref = deps_to_load.create()
+  let deps_loading: Map[Locator, Dep] ref = deps_loading.create()
   let dep_tags: Map[Locator, Array[String] val] ref = dep_tags.create()
 
   let _vcs_builder: VCSBuilder
@@ -67,18 +68,20 @@ actor _Updater
 
   fun ref load_queued_deps() =>
     for dep in deps_to_load.values() do
-      try
-        load_dep(dep)?
-      else
-        ctx.uout.err("Error loading dep " + dep.name())
-        // It won't get a lock. How should we handle/report the error?
+      if not deps_loading.contains(dep.locator) then
+        try
+          load_dep(dep)?
+        else
+          ctx.uout.err("Error loading dep " + dep.name())
+          // It won't get a lock. How should we handle/report the error?
+        end
       end
     end
     if deps_to_load.size() == 0 then
       _results_receiver.cmd_completed()
     end
 
-  fun load_dep(dep: Dep) ? =>
+  fun ref load_dep(dep: Dep) ? =>
     let vcs = _vcs_builder(dep.vcs())?
     let repo = RepoForDep(ctx, project, dep)?
 
@@ -109,6 +112,8 @@ actor _Updater
     let sync_op = vcs.sync_op(sync_handler)
     sync_op(repo)
 
+    deps_loading(locator) = dep
+
   be load_transitive_dep(locator: Locator) =>
     ctx.log.info("Loading transitive dep: " + locator.path())
     try
@@ -127,6 +132,7 @@ actor _Updater
     try
       // If this remove fails, it just means another path got here first.
       (_, let dep) = deps_to_load.remove(locator)?
+      deps_loading.remove(locator)?
       ctx.log.fine("tags for " + dep.locator.string() + ": " + tags.size().string())
       dep_tags(locator) = tags
     end

corral/test/testdata/regression-120/bundle-entrypoint/corral.json

@@ -0,0 +1,11 @@
+{
+  "deps": [
+    {
+      "locator": "../bundle-transitive"
+    },
+    {
+      "locator": "../bundle-plain"
+    }
+  ],
+  "info": {}
+}

corral/test/testdata/regression-120/bundle-entrypoint/lock.json

@@ -0,0 +1,20 @@
+{
+  "locks": [
+    {
+      "locator": "../bundle-transitive-sub-2",
+      "revision": "master"
+    },
+    {
+      "locator": "../bundle-transitive",
+      "revision": "master"
+    },
+    {
+      "locator": "../bundle-plain",
+      "revision": "master"
+    },
+    {
+      "locator": "../bundle-transitive-sub-1",
+      "revision": "master"
+    }
+  ]
+}

corral/test/testdata/regression-120/bundle-plain/corral.json

@@ -0,0 +1,4 @@
+{
+  "deps": [],
+  "info": {}
+}

corral/test/testdata/regression-120/bundle-plain/lock.json

@@ -0,0 +1,3 @@
+{
+  "locks": []
+}

corral/test/testdata/regression-120/bundle-transitive-sub-1/corral.json

@@ -0,0 +1,4 @@
+{
+  "deps": [],
+  "info": {}
+}

corral/test/testdata/regression-120/bundle-transitive-sub-1/lock.json

@@ -0,0 +1,3 @@
+{
+  "locks": []
+}

corral/test/testdata/regression-120/bundle-transitive-sub-2/corral.json

@@ -0,0 +1,4 @@
+{
+  "deps": [],
+  "info": {}
+}

corral/test/testdata/regression-120/bundle-transitive-sub-2/lock.json

@@ -0,0 +1,3 @@
+{
+  "locks": []
+}

corral/test/testdata/regression-120/bundle-transitive/corral.json

@@ -0,0 +1,11 @@
+{
+  "deps": [
+    {
+      "locator": "../bundle-transitive-sub-2"
+    },
+    {
+      "locator": "../bundle-transitive-sub-1"
+    }
+  ],
+  "info": {}
+}

corral/test/testdata/regression-120/bundle-transitive/lock.json

@@ -0,0 +1,10 @@
+{
+  "locks": [
+    {
+      "locator": "../bundle-transitive-sub-2"
+    },
+    {
+      "locator": "../bundle-transitive-sub-1/"
+    }
+  ]
+}

corral/test/testdata/regression-120/lock.json

@@ -0,0 +1,10 @@
+{
+  "locks": [
+    {
+      "locator": "bundle-transitive"
+    },
+    {
+      "locator": "bundle-plain"
+    }
+  ]
+}