atlas: tests graph generation (#21990)

* atlas: tests graph generation

* silly typo

* make tests green; lockfile implementation begins to make sense

* make tests green on Windows

8 files changed, 144 insertions, 53 deletions

diff --git a/atlas/atlas.nim b/atlas/atlas.nim
index 9793f2637..cab09014c 100644
--- a/atlas/atlas.nim
+++ b/atlas/atlas.nim
@@ -81,11 +81,11 @@ const
   TestsDir = "atlas/tests"
 
 type
-  LockOption = enum
+  LockMode = enum
     noLock, genLock, useLock
 
   LockFileEntry = object
-    dir, url, commit: string
+    url, commit: string
 
   PackageName = distinct string
   CfgPath = distinct string # put into a config `--path:"../x"`
@@ -107,6 +107,9 @@ type
     processed: Table[string, int] # the key is (url / commit)
     byName: Table[PackageName, seq[int]]
 
+  LockFile = object # serialized as JSON so an object for extensibility
+    items: OrderedTable[string, LockFileEntry]
+
   AtlasContext = object
     projectDir, workspace, depsDir, currentDir: string
     hasPackageList: bool
@@ -116,9 +119,8 @@ type
     p: Table[string, string] # name -> url mapping
     errors, warnings: int
     overrides: Patterns
-    lockOption: LockOption
-    lockFileToWrite: seq[LockFileEntry]
-    lockFileToUse: Table[string, LockFileEntry]
+    lockMode: LockMode
+    lockFile: LockFile
     when MockupRun:
       step: int
       mockupSuccess: bool
@@ -424,7 +426,9 @@ proc toName(p: string): PackageName =
     result = PackageName p
 
 proc generateDepGraph(c: var AtlasContext; g: DepGraph) =
-  proc repr(w: Dependency): string = w.url / w.commit
+  proc repr(w: Dependency): string =
+    if w.url.endsWith("/"): w.url & w.commit
+    else: w.url & "/" & w.commit
 
   var dotGraph = ""
   for i in 0 ..< g.nodes.len:
@@ -433,7 +437,7 @@ proc generateDepGraph(c: var AtlasContext; g: DepGraph) =
     for p in items g.nodes[i].parents:
       if p >= 0:
         dotGraph.addf("\"$1\" -> \"$2\";\n", [g.nodes[p].repr, g.nodes[i].repr])
-  let dotFile = c.workspace / "deps.dot"
+  let dotFile = c.currentDir / "deps.dot"
   writeFile(dotFile, "digraph deps {\n$1}\n" % dotGraph)
   let graphvizDotPath = findExe("dot")
   if graphvizDotPath.len == 0:
@@ -461,42 +465,48 @@ proc getRequiredCommit(c: var AtlasContext; w: Dependency): string =
 proc getRemoteUrl(): string =
   execProcess("git config --get remote.origin.url").strip()
 
-proc genLockEntry(c: var AtlasContext; w: Dependency; dir: string) =
+proc genLockEntry(c: var AtlasContext; w: Dependency) =
   let url = getRemoteUrl()
   var commit = getRequiredCommit(c, w)
   if commit.len == 0 or needsCommitLookup(commit):
     commit = execProcess("git log -1 --pretty=format:%H").strip()
-  c.lockFileToWrite.add LockFileEntry(dir: relativePath(dir, c.workspace, '/'), url: url, commit: commit)
+  c.lockFile.items[w.name.string] = LockFileEntry(url: url, commit: commit)
 
-proc commitFromLockFile(c: var AtlasContext; dir: string): string =
+proc commitFromLockFile(c: var AtlasContext; w: Dependency): string =
   let url = getRemoteUrl()
-  let d = relativePath(dir, c.workspace, '/')
-  if d in c.lockFileToUse:
-    result = c.lockFileToUse[d].commit
-    let wanted = c.lockFileToUse[d].url
-    if wanted != url:
-      error c, PackageName(d), "remote URL has been compromised: got: " &
-          url & " but wanted: " & wanted
+  let entry = c.lockFile.items.getOrDefault(w.name.string)
+  if entry.commit.len > 0:
+    result = entry.commit
+    if entry.url != url:
+      error c, w.name, "remote URL has been compromised: got: " &
+          url & " but wanted: " & entry.url
   else:
-    error c, PackageName(d), "package is not listed in the lock file"
+    error c, w.name, "package is not listed in the lock file"
 
 proc dependencyDir(c: AtlasContext; w: Dependency): string =
   result = c.workspace / w.name.string
   if not dirExists(result):
     result = c.depsDir / w.name.string
 
+const
+  FileProtocol = "file://"
+  ThisVersion = "current_version.atlas"
+
 proc selectNode(c: var AtlasContext; g: var DepGraph; w: Dependency) =
   # all other nodes of the same project name are not active
   for e in items g.byName[w.name]:
     g.nodes[e].active = e == w.self
+  if c.lockMode == genLock:
+    if w.url.startsWith(FileProtocol):
+      c.lockFile.items[w.name.string] = LockFileEntry(url: w.url, commit: w.commit)
+    else:
+      genLockEntry(c, w)
 
 proc checkoutCommit(c: var AtlasContext; g: var DepGraph; w: Dependency) =
   let dir = dependencyDir(c, w)
   withDir c, dir:
-    if c.lockOption == genLock:
-      genLockEntry(c, w, dir)
-    elif c.lockOption == useLock:
-      checkoutGitCommit(c, w.name, commitFromLockFile(c, dir))
+    if c.lockMode == useLock:
+      checkoutGitCommit(c, w.name, commitFromLockFile(c, w))
     elif w.commit.len == 0 or cmpIgnoreCase(w.commit, "head") == 0:
       gitPull(c, w.name)
     else:
@@ -550,7 +560,7 @@ proc addUnique[T](s: var seq[T]; elem: sink T) =
   if not s.contains(elem): s.add elem
 
 proc addUniqueDep(c: var AtlasContext; g: var DepGraph; parent: int;
-                  tokens: seq[string]; lockfile: Table[string, LockFileEntry]) =
+                  tokens: seq[string]) =
   let pkgName = tokens[0]
   let oldErrors = c.errors
   let url = toUrl(c, pkgName)
@@ -564,13 +574,16 @@ proc addUniqueDep(c: var AtlasContext; g: var DepGraph; parent: int;
       let self = g.nodes.len
       g.byName.mgetOrPut(toName(pkgName), @[]).add self
       g.processed[key] = self
-      if lockfile.contains(pkgName):
-        g.nodes.add Dependency(name: toName(pkgName),
-                            url: lockfile[pkgName].url,
-                            commit: lockfile[pkgName].commit,
-                            rel: normal,
-                            self: self,
-                            parents: @[parent])
+      if c.lockMode == useLock:
+        if c.lockfile.items.contains(pkgName):
+          g.nodes.add Dependency(name: toName(pkgName),
+                              url: c.lockfile.items[pkgName].url,
+                              commit: c.lockfile.items[pkgName].commit,
+                              rel: normal,
+                              self: self,
+                              parents: @[parent])
+        else:
+          error c, toName(pkgName), "package is not listed in the lock file"
       else:
         g.nodes.add Dependency(name: toName(pkgName), url: url, commit: tokens[2],
                                rel: toDepRelation(tokens[1]),
@@ -579,13 +592,10 @@ proc addUniqueDep(c: var AtlasContext; g: var DepGraph; parent: int;
 
 template toDestDir(p: PackageName): string = p.string
 
-proc readLockFile(filename: string): Table[string, LockFileEntry] =
+proc readLockFile(filename: string): LockFile =
   let jsonAsStr = readFile(filename)
   let jsonTree = parseJson(jsonAsStr)
-  let data = to(jsonTree, seq[LockFileEntry])
-  result = initTable[string, LockFileEntry]()
-  for d in items(data):
-    result[d.dir] = d
+  result = to(jsonTree, LockFile)
 
 proc collectDeps(c: var AtlasContext; g: var DepGraph; parent: int;
                  dep: Dependency; nimbleFile: string): CfgPath =
@@ -593,11 +603,6 @@ proc collectDeps(c: var AtlasContext; g: var DepGraph; parent: int;
   # else return "".
   assert nimbleFile != ""
   let nimbleInfo = extractRequiresInfo(c, nimbleFile)
-
-  let lockFilePath = dependencyDir(c, dep) / LockFileName
-  let lockFile = if fileExists(lockFilePath): readLockFile(lockFilePath)
-                 else: initTable[string, LockFileEntry]()
-
   for r in nimbleInfo.requires:
     var tokens: seq[string] = @[]
     for token in tokenizeRequires(r):
@@ -615,7 +620,7 @@ proc collectDeps(c: var AtlasContext; g: var DepGraph; parent: int;
       tokens.add commit
 
     if tokens.len >= 3 and cmpIgnoreCase(tokens[0], "nim") != 0:
-      c.addUniqueDep g, parent, tokens, lockFile
+      c.addUniqueDep g, parent, tokens
   result = CfgPath(toDestDir(dep.name) / nimbleInfo.srcDir)
 
 proc collectNewDeps(c: var AtlasContext; g: var DepGraph; parent: int;
@@ -628,10 +633,6 @@ proc collectNewDeps(c: var AtlasContext; g: var DepGraph; parent: int;
 
 proc selectDir(a, b: string): string = (if dirExists(a): a else: b)
 
-const
-  FileProtocol = "file://"
-  ThisVersion = "current_version.atlas"
-
 proc copyFromDisk(c: var AtlasContext; w: Dependency) =
   let destDir = toDestDir(w.name)
   var u = w.url.substr(FileProtocol.len)
@@ -658,6 +659,10 @@ proc isLaterCommit(destDir, version: string): bool =
   result = isNewerVersion(version, oldVersion)
 
 proc traverseLoop(c: var AtlasContext; g: var DepGraph; startIsDep: bool): seq[CfgPath] =
+  if c.lockMode == useLock:
+    let lockFilePath = dependencyDir(c, g.nodes[0]) / LockFileName
+    c.lockFile = readLockFile(lockFilePath)
+
   result = @[]
   var i = 0
   while i < g.nodes.len:
@@ -690,6 +695,9 @@ proc traverseLoop(c: var AtlasContext; g: var DepGraph; startIsDep: bool): seq[C
       result.addUnique collectNewDeps(c, g, i, w)
     inc i
 
+  if c.lockMode == genLock:
+    writeFile c.currentDir / LockFileName, toJson(c.lockFile).pretty
+
 proc traverse(c: var AtlasContext; start: string; startIsDep: bool): seq[CfgPath] =
   # returns the list of paths for the nim.cfg file.
   let url = toUrl(c, start)
@@ -701,11 +709,7 @@ proc traverse(c: var AtlasContext; start: string; startIsDep: bool): seq[CfgPath
     return
 
   c.projectDir = c.workspace / toDestDir(g.nodes[0].name)
-  if c.lockOption == useLock:
-    c.lockFileToUse = readLockFile(c.projectDir / LockFileName)
   result = traverseLoop(c, g, startIsDep)
-  if c.lockOption == genLock:
-    writeFile c.projectDir / LockFileName, toJson(c.lockFileToWrite).pretty
   showGraph c, g
 
 const
@@ -1042,13 +1046,13 @@ proc main =
       of "autoinit": autoinit = true
       of "showgraph": c.showGraph = true
       of "genlock":
-        if c.lockOption != useLock:
-          c.lockOption = genLock
+        if c.lockMode != useLock:
+          c.lockMode = genLock
         else:
           writeHelp()
       of "uselock":
-        if c.lockOption != genLock:
-          c.lockOption = useLock
+        if c.lockMode != genLock:
+          c.lockMode = useLock
         else:
           writeHelp()
       of "colors":
diff --git a/atlas/tester.nim b/atlas/tester.nim
new file mode 100644
index 000000000..98a640fd3
--- /dev/null
+++ b/atlas/tester.nim
@@ -0,0 +1,43 @@
+# Small program that runs the test cases
+
+import std / [strutils, os, sequtils]
+from std/private/gitutils import diffFiles
+
+var failures = 0
+
+when defined(develop):
+  const atlasExe = "bin" / "atlas".addFileExt(ExeExt)
+  if execShellCmd("nim c -o:$# atlas/atlas.nim" % [atlasExe]) != 0:
+    quit("FAILURE: compilation of atlas failed")
+else:
+  const atlasExe = "atlas".addFileExt(ExeExt)
+
+proc exec(cmd: string) =
+  if execShellCmd(cmd) != 0:
+    quit "FAILURE: " & cmd
+
+proc sameDirContents(expected, given: string) =
+  for _, e in walkDir(expected):
+    let g = given / splitPath(e).tail
+    if fileExists(g):
+      if readFile(e) != readFile(g):
+        echo "FAILURE: files differ: ", e
+        echo diffFiles(e, g).output
+        inc failures
+    else:
+      echo "FAILURE: file does not exist: ", g
+      inc failures
+
+proc testWsConflict() =
+  const myproject = "atlas/tests/ws_conflict/myproject"
+  createDir(myproject)
+  exec atlasExe & " --project=" & myproject & " --showGraph --genLock use https://github.com/apkg"
+  sameDirContents("atlas/tests/ws_conflict/expected", myproject)
+  removeDir("atlas/tests/ws_conflict/apkg")
+  removeDir("atlas/tests/ws_conflict/bpkg")
+  removeDir("atlas/tests/ws_conflict/cpkg")
+  removeDir("atlas/tests/ws_conflict/dpkg")
+  removeDir(myproject)
+
+testWsConflict()
+if failures > 0: quit($failures & " failures occurred.")
diff --git a/atlas/tests/ws_conflict/expected/atlas.lock b/atlas/tests/ws_conflict/expected/atlas.lock
new file mode 100644
index 000000000..34ace9c44
--- /dev/null
+++ b/atlas/tests/ws_conflict/expected/atlas.lock
@@ -0,0 +1,20 @@
+{
+  "items": {
+    "apkg": {
+      "url": "file://./source/apkg",
+      "commit": "<invalid commit>"
+    },
+    "bpkg": {
+      "url": "file://./source/bpkg",
+      "commit": "1.0"
+    },
+    "cpkg": {
+      "url": "file://./source/cpkg",
+      "commit": "2.0"
+    },
+    "dpkg": {
+      "url": "file://./source/dpkg",
+      "commit": "1.0"
+    }
+  }
+}
\ No newline at end of file
diff --git a/atlas/tests/ws_conflict/expected/deps.dot b/atlas/tests/ws_conflict/expected/deps.dot
new file mode 100644
index 000000000..0cdfb6cdd
--- /dev/null
+++ b/atlas/tests/ws_conflict/expected/deps.dot
@@ -0,0 +1,11 @@
+digraph deps {
+"file://./source/apkg/<invalid commit>" [label=""];
+"file://./source/bpkg/1.0" [label=""];
+"file://./source/cpkg/1.0" [label="unused"];
+"file://./source/cpkg/2.0" [label=""];
+"file://./source/dpkg/1.0" [label=""];
+"file://./source/apkg/<invalid commit>" -> "file://./source/bpkg/1.0";
+"file://./source/apkg/<invalid commit>" -> "file://./source/cpkg/1.0";
+"file://./source/bpkg/1.0" -> "file://./source/cpkg/2.0";
+"file://./source/cpkg/2.0" -> "file://./source/dpkg/1.0";
+}
diff --git a/atlas/tests/ws_conflict/expected/myproject.nimble b/atlas/tests/ws_conflict/expected/myproject.nimble
new file mode 100644
index 000000000..863e7c171
--- /dev/null
+++ b/atlas/tests/ws_conflict/expected/myproject.nimble
@@ -0,0 +1 @@
+requires "https://github.com/apkg"
diff --git a/atlas/tests/ws_conflict/expected/nim.cfg b/atlas/tests/ws_conflict/expected/nim.cfg
new file mode 100644
index 000000000..6f7c73055
--- /dev/null
+++ b/atlas/tests/ws_conflict/expected/nim.cfg
@@ -0,0 +1,7 @@
+############# begin Atlas config section ##########
+--noNimblePath
+--path:"../apkg"
+--path:"../bpkg"
+--path:"../cpkg"
+--path:"../dpkg"
+############# end Atlas config section   ##########
diff --git a/atlas/tests/ws_conflict/source/dpkg/dpkg.nimble b/atlas/tests/ws_conflict/source/dpkg/dpkg.nimble
index e69de29bb..9ed72bb49 100644
--- a/atlas/tests/ws_conflict/source/dpkg/dpkg.nimble
+++ b/atlas/tests/ws_conflict/source/dpkg/dpkg.nimble
@@ -0,0 +1 @@
+# empty for now
diff --git a/koch.nim b/koch.nim
index c84cc9be4..e15215df0 100644
--- a/koch.nim
+++ b/koch.nim
@@ -613,6 +613,10 @@ proc runCI(cmd: string) =
       execFold("Run nimsuggest tests", "nim r nimsuggest/tester")
 
     execFold("Run atlas tests", "nim c -r -d:atlasTests atlas/atlas.nim clone https://github.com/disruptek/balls")
+    # compile it again to get rid of `-d:atlasTests`:
+    nimCompileFold("Compile atlas", "atlas/atlas.nim", options = "-d:release ",
+        outputName = "atlas")
+    execFold("Run more atlas tests", "nim c -r atlas/tester.nim")
 
     kochExecFold("Testing booting in refc", "boot -d:release --mm:refc -d:nimStrictMode --lib:lib")