From 2c8d484d5fcc82766aefec8d1840fb7e13d2fd0e Mon Sep 17 00:00:00 2001 From: alaviss Date: Sat, 4 Nov 2023 19:01:21 -0500 Subject: [PATCH] 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 https://github.com/nim-works/nimskull/pull/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 | 4 +-- src/nimph.nim.cfg | 4 --- src/nimph/config.nim | 73 +++++++++++++++++++++++---------------- src/nimph/dependency.nim | 12 +++---- src/nimph/group.nim | 6 ++-- src/nimph/project.nim | 16 ++++----- src/nimph/versiontags.nim | 2 +- tests/test.nim | 10 +++--- 8 files changed, 70 insertions(+), 57 deletions(-) diff --git a/bootstrap-nonimble.sh b/bootstrap-nonimble.sh index 11e6c6e..5e01507 100755 --- a/bootstrap-nonimble.sh +++ b/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 diff --git a/src/nimph.nim.cfg b/src/nimph.nim.cfg index 13929cb..5a3ae1e 100644 --- a/src/nimph.nim.cfg +++ b/src/nimph.nim.cfg @@ -22,7 +22,3 @@ # fix nimble? --path="$config" --path="$nim" - -# arc exhibits corruption in git ---gc:refc ---define:useMalloc diff --git a/src/nimph/config.nim b/src/nimph/config.nim index 119e128..f522928 100644 --- a/src/nimph/config.nim +++ b/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) diff --git a/src/nimph/dependency.nim b/src/nimph/dependency.nim index 5f35800..5f6f64e 100644 --- a/src/nimph/dependency.nim +++ b/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 diff --git a/src/nimph/group.nim b/src/nimph/group.nim index d116b1d..afd555a 100644 --- a/src/nimph/group.nim +++ b/src/nimph/group.nim @@ -98,7 +98,7 @@ 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) = @@ -106,7 +106,7 @@ proc add*[K: string, V](group: Group[K, V]; url: Uri; value: V) = 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] = diff --git a/src/nimph/project.nim b/src/nimph/project.nim index 92f98ad..7564b54 100644 --- a/src/nimph/project.nim +++ b/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 diff --git a/src/nimph/versiontags.nim b/src/nimph/versiontags.nim index f4b1f48..7ab3464 100644 --- a/src/nimph/versiontags.nim +++ b/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) = diff --git a/tests/test.nim b/tests/test.nim index 72f483f..503e0be 100644 --- a/tests/test.nim +++ b/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":