Improve compatibility with nimskull (#167)

* project: correct wrong usage of defer

`defer` in Nim works at scope level and not function level. By calling
`defer` in an `if` like this, the pointer was freed immediately, causing
an use-after-free bug, breaking nimph on ARC/ORC.

This commit fixes the `defer` declaration, allowing nimph to work on ARC/ORC.

* config: migrate away from deprecated multimap

The multimap API has been deprecated for quite awhile and nimskull
already removed it from tables.

This commit switch to Table with seq value as an alternative.

* config: support nimskull config parser API

Requires nim-works/nimskull#1007

* dependency: rename symbolicMatch iterator

Overloading iterator and proc of the same name is currently buggy on NimSkull.

Rename symbolicMatch iterator to symbolicMatches to avoid this issue.

* treewide: convert group to singular map

NimSkull has removed the multimap API, and given that Group was not
very clear on whether it's a multimap or singular map centric, let's
temporary use singular map and convert back as needed.

* bootstrap: update cligen to 2.0.3

This release contains fixes for nimskull

bootstrap-nonimble.sh

@@ -2,7 +2,7 @@
 
 PASSES=""
 if [ "$GITHUB_ACTIONS" = "true" ]; then
-  if [ `uname -s` = "Linux" ]; then
+  if [ $(uname -s) = "Linux" ]; then
     LGEXT="so"
   else
     LGEXT="dylib"
@@ -18,7 +18,7 @@ cd temporary
 git clone --depth 1 --branch 1.8.31 https://github.com/disruptek/bump.git
 git clone --depth 1 --branch 2.0.1 https://github.com/disruptek/cutelog.git
 git clone --depth 1 --branch 3.1.0 https://github.com/disruptek/gittyup.git
-git clone --depth 1 --branch 2.0.2 https://github.com/disruptek/cligen.git
+git clone --depth 1 --branch 2.0.3 https://github.com/disruptek/cligen.git
 git clone --depth 1 --branch 0.26.0 https://github.com/zevv/npeg.git
 git clone --depth 1 --branch 1.0.2 https://github.com/disruptek/jsonconvert.git
 git clone --depth 1 --branch 2.1.3 https://github.com/disruptek/badresults.git

src/nimph.nim.cfg

@@ -22,7 +22,3 @@
 # fix nimble?
 --path="$config"
 --path="$nim"
-
-# arc exhibits corruption in git
---gc:refc
---define:useMalloc

src/nimph/config.nim

@@ -24,7 +24,7 @@ when defined(debugPath):
 
 type
   ProjectCfgParsed* = object
-    table*: TableRef[string, string]
+    table*: TableRef[string, seq[string]]
     why*: string
     ok*: bool
 
@@ -58,6 +58,12 @@ template setDefaultsForConfig(result: ConfigRef) =
   when compiles(hintLineTooLong):
     excludeAllNotes(result, hintLineTooLong)
 
+when defined(isNimSkull):
+  proc readConfigEventWriter(config: ConfigRef, evt: ConfigFileEvent,
+                             writeFrom: InstantiationInfo) =
+    ## Used to print config read events. Noop for now.
+    discard
+
 proc parseConfigFile*(path: string): Option[ConfigRef] =
   ## use the compiler to parse a nim.cfg without changing to its directory
   var
@@ -71,7 +77,12 @@ proc parseConfigFile*(path: string): Option[ConfigRef] =
 
   setDefaultsForConfig(config)
 
-  if readConfigFile(filename.AbsoluteFile, cache, config):
+  let success = when defined(isNimSkull):
+    readConfigFile(filename.AbsoluteFile, cache, config, readConfigEventWriter)
+  else:
+    readConfigFile(filename.AbsoluteFile, cache, config)
+
+  if success:
     result = some(config)
 
 when false:
@@ -163,8 +174,11 @@ proc loadAllCfgs*(directory: string): ConfigRef =
     # now follow the compiler process of loading the configs
     var cache = newIdentCache()
 
+    when isNimSkull:
+      # XXX: nimskull returns whether reading was successful, but unused atm
+      discard loadConfigs(NimCfg.RelativeFile, cache, result, readConfigEventWriter)
     # thanks, araq
-    when (NimMajor, NimMinor) >= (1, 5):
+    elif (NimMajor, NimMinor) >= (1, 5):
       var idgen = IdGenerator()
       loadConfigs(NimCfg.RelativeFile, cache, result, idgen)
     else:
@@ -228,7 +242,7 @@ proc appendConfig*(path: Target; config: string): bool =
 
 proc parseProjectCfg*(input: Target): ProjectCfgParsed =
   ## parse a .cfg for any lines we are entitled to mess with
-  result = ProjectCfgParsed(ok: false, table: newTable[string, string]())
+  result = ProjectCfgParsed(ok: false, table: newTable[string, seq[string]]())
   var
     table = result.table
 
@@ -257,11 +271,11 @@ proc parseProjectCfg*(input: Target): ProjectCfgParsed =
         otherkeys <- i"path" | i"p" | i"define" | i"d"
         keys <- nimblekeys | otherkeys
         strsetting <- hyphens * >keys * equals * >strvalue * ending:
-          table.add $1, unescape($2)
+          table.mgetOrPut($1, @[]).add unescape($2)
         anysetting <- hyphens * >keys * equals * >anyvalue * ending:
-          table.add $1, $2
+          table.mgetOrPut($1, @[]).add $2
         toggle <- hyphens * >keys * ending:
-          table.add $1, "it's enabled, okay?"
+          table.mgetOrPut($1, @[]).add "it's enabled, okay?"
         line <- strsetting | anysetting | toggle | (*(1 - nl) * nl)
         document <- *line * !1
       parsed = peggy.match(content)
@@ -539,29 +553,30 @@ proc removeSearchPath*(config: ConfigRef; nimcfg: Target; path: string): bool =
     var
       content = fn.readFile
     # iterate over the entries we parsed naively,
-    for key, value in parsed.table.pairs:
-      # skipping anything that it's a path,
-      if key.toLowerAscii notin ["p", "path", "nimblepath"]:
-        continue
-      # and perform substitutions to see if one might match the value
-      # we are trying to remove; the write flag is false so that we'll
-      # use any $nimbleDir substitutions available to us, if possible
-      for sub in config.pathSubstitutions(path, nimcfg.repo, write = false):
-        if sub notin [value, ///value]:
-          continue
-        # perform a regexp substition to remove the entry from the content
-        let
-          regexp = re("(*ANYCRLF)(?i)(?s)(-{0,2}" & key.escapeRe &
-                      "[:=]\"?" & value.escapeRe & "/?\"?)\\s*")
-          swapped = content.replace(regexp, "")
-        # if that didn't work, cry a bit and move on
-        if swapped == content:
-          notice &"failed regex edit to remove path `{value}`"
+    for key, values in parsed.table.pairs:
+      for value in values.items:
+        # skipping anything that it's a path,
+        if key.toLowerAscii notin ["p", "path", "nimblepath"]:
           continue
-        # make sure we search the new content next time through the loop
-        content = swapped
-        result = true
-        # keep performing more substitutions
+        # and perform substitutions to see if one might match the value
+        # we are trying to remove; the write flag is false so that we'll
+        # use any $nimbleDir substitutions available to us, if possible
+        for sub in config.pathSubstitutions(path, nimcfg.repo, write = false):
+          if sub notin [value, ///value]:
+            continue
+          # perform a regexp substition to remove the entry from the content
+          let
+            regexp = re("(*ANYCRLF)(?i)(?s)(-{0,2}" & key.escapeRe &
+                        "[:=]\"?" & value.escapeRe & "/?\"?)\\s*")
+            swapped = content.replace(regexp, "")
+          # if that didn't work, cry a bit and move on
+          if swapped == content:
+            notice &"failed regex edit to remove path `{value}`"
+            continue
+          # make sure we search the new content next time through the loop
+          content = swapped
+          result = true
+          # keep performing more substitutions
 
     # finally, write the edited content
     fn.writeFile(content)

src/nimph/dependency.nim

@@ -232,7 +232,7 @@ iterator matchingReleases(requirement: Requirement; head = "";
     if requirement.happyProvision(release, head = head, tags = tags):
       yield release
 
-iterator symbolicMatch*(project: Project; req: Requirement): Release =
+iterator symbolicMatches*(project: Project; req: Requirement): Release =
   ## see if a project can match a given requirement symbolically
   if project.dist == Git:
     if project.tags == nil:
@@ -270,7 +270,7 @@ iterator symbolicMatch*(project: Project; req: Requirement): Release =
 proc symbolicMatch*(project: Project; req: Requirement; release: Release): bool =
   ## convenience
   let release = project.peelRelease(release)
-  for match in project.symbolicMatch(req):
+  for match in project.symbolicMatches(req):
     result = match == release
     if result:
       break
@@ -284,7 +284,7 @@ proc symbolicMatch*(project: var Project; req: Requirement; release: Release): b
 
 proc symbolicMatch*(project: Project; req: Requirement): bool =
   ## convenience
-  for match in project.symbolicMatch(req):
+  for match in project.symbolicMatches(req):
     result = true
     break
 
@@ -475,7 +475,7 @@ proc addName(group: var DependencyGroup; req: Requirement; dep: Dependency) =
         assert group.imports.hasKey(name)
 
 proc add*(group: var DependencyGroup; req: Requirement; dep: Dependency) =
-  group.table.add req, dep
+  group.table[req] = dep
   group.addName req, dep
 
 proc addedRequirements*(dependencies: var DependencyGroup;
@@ -721,7 +721,7 @@ proc roll*(project: var Project; requirement: Requirement;
 
   # get the list of suitable releases as a seq...
   var
-    releases = toSeq project.symbolicMatch(requirement)
+    releases = toSeq project.symbolicMatches(requirement)
   case goal:
   of Upgrade:
     # ...so we can reverse it if needed to invert semantics
@@ -785,7 +785,7 @@ proc rollTowards*(project: var Project; requirement: Requirement): bool =
 
   # reverse the order of matching releases so that we start with the latest
   # valid release first and proceed to lesser versions thereafter
-  var releases = toSeq project.symbolicMatch(requirement)
+  var releases = toSeq project.symbolicMatches(requirement)
   releases.reverse
 
   # iterate over all matching tags

src/nimph/group.nim

@@ -98,15 +98,15 @@ proc `[]`*[K, V](group: Group[K, V]; key: K): V =
 
 proc add*[K: string, V](group: Group[K, V]; key: K; value: V) =
   ## add a key and value to the group
-  group.table.add key, value
+  group.table[key] = value
   group.addName(key.importName, key)
 
 proc add*[K: string, V](group: Group[K, V]; url: Uri; value: V) =
   ## add a (bare) url as a key
   let
     naked = url.bare
     key = $naked
-  group.table.add key, value
+  group.table[key] = value
   # this gets picked up during instant-instantiation of a package from
   # a project's url, a la asPackage(project: Project): Package ...
   group.addName naked.importName, key
@@ -120,7 +120,7 @@ proc `[]=`*[K, V](group: Group[K, V]; key: K; value: V) =
 {.warning: "nim bug #12818".}
 proc add*[K: Uri, V](group: Group[K, V]; url: Uri; value: V) =
   ## add a (full) url as a key
-  group.table.add url, value
+  group.table[url] = value
   group.addName url
 
 iterator pairs*[K, V](group: Group[K, V]): tuple[key: K; val: V] =

src/nimph/project.nim

@@ -346,11 +346,11 @@ proc sortByVersion*(tags: GitTagTable): GitTagTable =
         order.add (tag: tag, version: parsed.get.version, thing: thing)
         continue
     # if the tag isn't parsable as a version, store it in the result
-    result.add tag, thing
+    result[tag] = thing
 
   # now sort the sequence and add the tags with versions to the result
   for trio in order.sortedByIt(it.version):
-    result.add trio.tag, trio.thing
+    result[trio.tag] = trio.thing
 
 proc fetchTagTable*(project: var Project) =
   ## retrieve the tags for a project from its git repository
@@ -396,8 +396,8 @@ proc cuteRelease*(project: Project): string =
     let
       head = project.getHeadOid
     # free the oid if necessary
-    if head.isOk:
-      defer:
+    defer:
+      if head.isOk:
         free head.get
 
     # assign a useful release string using the head
@@ -423,8 +423,8 @@ proc findCurrentTag*(project: Project): Release =
   let
     head = project.getHeadOid
   # free the oid if necessary
-  if head.isOk:
-    defer:
+  defer:
+    if head.isOk:
       free head.get
   var
     name: string
@@ -1019,7 +1019,7 @@ proc allImportTargets*(config: ConfigRef; repo: string):
       found = target.search.found
 
     if found.isSome():
-      result.add found.get, target
+      result[found.get] = target
 
 iterator asFoundVia*(group: var ProjectGroup; config: ConfigRef;
                      repo: string): var Project =
@@ -1041,7 +1041,7 @@ iterator asFoundVia*(group: var ProjectGroup; config: ConfigRef;
         if found.get == project.nimble:
           # if it is, put it in the dedupe and yield it
           if project.importName notin dedupe:
-            dedupe.add project.importName, project
+            dedupe[project.importName] = project
             yield project
           break
 

src/nimph/versiontags.nim

@@ -29,7 +29,7 @@ proc addName*(group: var VersionTags; version: Version; thing: GitThing) =
 
 proc add*(group: var VersionTags; ver: Version; thing: GitThing) =
   ## add a version to the group; note that this overwrites semvers
-  group.table.add ver, thing
+  group.table[ver] = thing
   group.addName ver, thing
 
 proc del*(group: var VersionTags; ver: Version) =

tests/test.nim

@@ -50,12 +50,14 @@ block:
       check parsed.ok
       check "nimblePath" in parsed.table
       checkpoint $parsed.table
-      check parsed.table["path"].len > 1
+      check parsed.table["path"].len == 1
+      check parsed.table["path"][0].len > 1
       for find in ["test4", "test3:foo", "test2=foo"]:
         block found:
-          for value in parsed.table.values:
-            if value == find:
-              break found
+          for values in parsed.table.values:
+            for value in values.items:
+              if value == find:
+                break found
           fail "missing config values from parse"
 
     test "add a line to a config":