From e550b921dd32129676edfb7de58fc54766e6ab51 Mon Sep 17 00:00:00 2001
From: StevenvdSchoot <stevenvdschoot@gmail.com>
Date: Tue, 17 Jan 2023 20:39:39 +0100
Subject: [PATCH] Make syncVersions only sync versions of used tools

When syncVersions sync the versions of the provided tools, it should only change the version of a tool that has a defined version. Otherwise tools may get installed that were not meant to be installed.
In particular this solves an issue where, when an explicit version for clangtidy is specified and the compiler is gcc, we would install llvm on top of gcc.
---
 src/__tests__/main.test.ts | 19 ++++++++++++++++---
 src/versions/versions.ts   | 25 ++++++++-----------------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/__tests__/main.test.ts b/src/__tests__/main.test.ts
index 53c085f6..bc6095ef 100644
--- a/src/__tests__/main.test.ts
+++ b/src/__tests__/main.test.ts
@@ -29,9 +29,22 @@ describe("syncVersion", () => {
     expect(syncVersions(parseArgs(["--llvm", "13.0.0", "--clangtidy", "true"]), llvmTools)).toBe(true)
     expect(syncVersions(parseArgs(["--llvm", "13.0.0", "--clangtidy", "12.0.0"]), llvmTools)).toBe(false)
 
-    const opts = parseArgs(["--llvm", "14.0.0", "--clangtidy", "true"])
-    expect(syncVersions(opts, llvmTools)).toBe(true)
-    expect(opts.llvm).toBe(opts.clangtidy)
+    const opts1 = parseArgs(["--llvm", "14.0.0", "--clangtidy", "true"])
+    expect(syncVersions(opts1, llvmTools)).toBe(true)
+    expect(opts1.llvm).toBe(opts1.clangtidy)
+    expect(opts1.clangformat).toBe(undefined)
+
+    const opts2 = parseArgs(["--clangtidy", "15.0.0", "--clangformat", "true"])
+    expect(syncVersions(opts2, llvmTools)).toBe(true)
+    expect(opts2.llvm).toBe(undefined)
+    expect(opts2.clangtidy).toBe("15.0.0")
+    expect(opts2.clangformat).toBe("15.0.0")
+
+    const opts3 = parseArgs(["--llvm", "true", "--clangformat", "true"])
+    expect(syncVersions(opts3, llvmTools)).toBe(true)
+    expect(opts3.llvm).toBe("true")
+    expect(opts3.clangtidy).toBe(undefined)
+    expect(opts3.clangformat).toBe("true")
   })
 })
 
diff --git a/src/versions/versions.ts b/src/versions/versions.ts
index c0aa8a8c..bdd5ce5d 100644
--- a/src/versions/versions.ts
+++ b/src/versions/versions.ts
@@ -30,25 +30,16 @@ export function isDefault(version: string | undefined, name: string) {
 }
 
 export function syncVersions(opts: Opts, tools: Inputs[]): boolean {
-  for (let i = 0; i < tools.length; i++) {
-    // tools excluding i_tool
-    const otherTools = tools.slice(0, i).concat(tools.slice(i + 1))
+  const toolsInUse = tools.filter((tool) => opts[tool] !== undefined)
+  const toolsNonDefaultVersion = toolsInUse.filter((tool) => !isDefault(opts[tool], tool))
 
-    const tool = tools[i]
+  const targetVersion = toolsNonDefaultVersion.length ? opts[toolsNonDefaultVersion[0]] : "true"
 
-    if (!isDefault(opts[tool], tool)) {
-      for (let i_other = 0; i_other < otherTools.length; i_other++) {
-        const otherTool = otherTools[i_other]
-        const useDefaultOtherTool = isDefault(opts[otherTool], otherTools[i_other])
-        if (useDefaultOtherTool) {
-          // use the same version if the other tool was requested with the default
-          opts[otherTool] = opts[tool]
-        } else if (opts[tool] !== opts[otherTools[i_other]]) {
-          // error if different from the other given versions
-          return false
-        }
-      }
-    }
+  // return false if any explicit versions don't match the target version
+  if (toolsNonDefaultVersion.findIndex((tool) => opts[tool] !== targetVersion) !== -1) {
+    return false
   }
+
+  toolsInUse.forEach((tool) => (opts[tool] = targetVersion))
   return true
 }