From a4aab1d0e55922d9f0c79d23ad1fd14687e828c1 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Wed, 14 Jul 2021 19:56:47 -0700 Subject: [PATCH 1/5] moduleoverride --- compiler/commands.nim | 9 +++++++++ compiler/debugutils.nim | 2 ++ compiler/modulepaths.nim | 3 +++ compiler/options.nim | 29 ++++++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/compiler/commands.nim b/compiler/commands.nim index 1c3de29bad228..447c86e2d751e 100644 --- a/compiler/commands.nim +++ b/compiler/commands.nim @@ -931,6 +931,15 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo; else: conf.spellSuggestMax = parseInt(arg) of "declaredlocs": processOnOffSwitchG(conf, {optDeclaredLocs}, arg, pass, info) + of "moduleoverride": + # --moduleoverride:std/sequtils:/tmp/sequtils2:this/test1,this/test2 + let args = arg.split(":") + if args.len != 3: + localError(conf, info, "invalid arg: $1" % arg) + else: + let prefixes = args[2].split(",") + for a in prefixes: + conf.localOverrides.add LocalOverride(lhs: args[0], rhs: args[1], prefix: a) of "dynliboverride": dynlibOverride(conf, switch, arg, pass, info) of "dynliboverrideall": diff --git a/compiler/debugutils.nim b/compiler/debugutils.nim index d109d2121ba69..7ed431feea224 100644 --- a/compiler/debugutils.nim +++ b/compiler/debugutils.nim @@ -54,3 +54,5 @@ proc isCompilerDebug*(): bool = {.undef(nimCompilerDebug).} echo 'x' conf0.isDefined("nimCompilerDebug") + +include timn/exp/nim_compiler_debugutils diff --git a/compiler/modulepaths.nim b/compiler/modulepaths.nim index a16b669c45910..c1fcb787c943d 100644 --- a/compiler/modulepaths.nim +++ b/compiler/modulepaths.nim @@ -156,6 +156,9 @@ proc checkModuleName*(conf: ConfigRef; n: PNode; doLocalError=true): FileIndex = # This returns the full canonical path for a given module import let modulename = getModuleName(conf, n) let fullPath = findModule(conf, modulename, toFullPath(conf, n.info)) + dbgIf modulename, fullPath, conf.localOverrides + if isCompilerDebug(): + dbg getStacktrace() if fullPath.isEmpty: if doLocalError: let m = if modulename.len > 0: modulename else: $n diff --git a/compiler/options.nim b/compiler/options.nim index 45ca14e7628bf..189b883018e0c 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -278,7 +278,14 @@ type foLegacyRelProj # legacy, shortest of (foAbs, foRelProject) foName # lastPathPart, e.g.: foo.nim foStacktrace # if optExcessiveStackTrace: foAbs else: foName - + LocalOverrideAtom* = object + + LocalOverride* = object + # StringTableRef + # tab*: Table[string, string] + lhs*: string + rhs*: string + prefix*: string ConfigRef* {.acyclic.} = ref object ## every global configuration ## fields marked with '*' are subject to ## the incremental compilation mechanisms @@ -344,6 +351,7 @@ type jsonBuildFile*: AbsoluteFile prefixDir*, libpath*, nimcacheDir*: AbsoluteDir dllOverrides, moduleOverrides*, cfileSpecificOptions*: StringTableRef + localOverrides*: seq[LocalOverride] projectName*: string # holds a name like 'nim' projectPath*: AbsoluteDir # holds a path like /home/alice/projects/nim/compiler/ projectFull*: AbsoluteFile # projectPath/projectName @@ -852,6 +860,8 @@ proc findFile*(conf: ConfigRef; f: string; suppressStdlib = false): AbsoluteFile result = rawFindFile2(conf, RelativeFile f.toLowerAscii) patchModule(conf) +proc canonicalImport*(conf: ConfigRef, file: AbsoluteFile): string + proc findModule*(conf: ConfigRef; modulename, currentModule: string): AbsoluteFile = # returns path to module var m = addFileExt(modulename, NimExt) @@ -870,6 +880,23 @@ proc findModule*(conf: ConfigRef; modulename, currentModule: string): AbsoluteFi result = AbsoluteFile currentPath / m if not fileExists(result): result = findFile(conf, m) + + if not result.isEmpty: + let canon = canonicalImport(conf, result) + let canonCurrentModule = canonicalImport(conf, AbsoluteFile(currentModule)) & '/' + for ai in conf.localOverrides: + if ai.lhs == canon: + # dbg ai, canon, modulename, currentModule + dbg canonCurrentModule, ai.prefix + if canonCurrentModule.startsWith(ai.prefix & '/'): + result = findModule(conf, ai.rhs, currentModule) + break + # # PRTEMP + # let key = getPackageName(conf, result.string) & "_" & splitFile(result).name + # if conf.moduleOverrides.hasKey(key): + # let ov = conf.moduleOverrides[key] + # if ov.len > 0: result = AbsoluteFile(ov) + patchModule(conf) proc findProjectNimFile*(conf: ConfigRef; pkg: string): string = From 604aae06acba55c150a2a7335b874f01d676ecfd Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Wed, 14 Jul 2021 20:32:49 -0700 Subject: [PATCH 2/5] cleanup --- compiler/modulepaths.nim | 3 --- compiler/options.nim | 9 --------- 2 files changed, 12 deletions(-) diff --git a/compiler/modulepaths.nim b/compiler/modulepaths.nim index c1fcb787c943d..a16b669c45910 100644 --- a/compiler/modulepaths.nim +++ b/compiler/modulepaths.nim @@ -156,9 +156,6 @@ proc checkModuleName*(conf: ConfigRef; n: PNode; doLocalError=true): FileIndex = # This returns the full canonical path for a given module import let modulename = getModuleName(conf, n) let fullPath = findModule(conf, modulename, toFullPath(conf, n.info)) - dbgIf modulename, fullPath, conf.localOverrides - if isCompilerDebug(): - dbg getStacktrace() if fullPath.isEmpty: if doLocalError: let m = if modulename.len > 0: modulename else: $n diff --git a/compiler/options.nim b/compiler/options.nim index 189b883018e0c..fc139a44bc070 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -880,23 +880,14 @@ proc findModule*(conf: ConfigRef; modulename, currentModule: string): AbsoluteFi result = AbsoluteFile currentPath / m if not fileExists(result): result = findFile(conf, m) - if not result.isEmpty: let canon = canonicalImport(conf, result) let canonCurrentModule = canonicalImport(conf, AbsoluteFile(currentModule)) & '/' for ai in conf.localOverrides: if ai.lhs == canon: - # dbg ai, canon, modulename, currentModule - dbg canonCurrentModule, ai.prefix if canonCurrentModule.startsWith(ai.prefix & '/'): result = findModule(conf, ai.rhs, currentModule) break - # # PRTEMP - # let key = getPackageName(conf, result.string) & "_" & splitFile(result).name - # if conf.moduleOverrides.hasKey(key): - # let ov = conf.moduleOverrides[key] - # if ov.len > 0: result = AbsoluteFile(ov) - patchModule(conf) proc findProjectNimFile*(conf: ConfigRef; pkg: string): string = From cfef1622bd1d7e4bc57e235ddeddab441f6c5881 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Thu, 15 Jul 2021 00:43:01 -0700 Subject: [PATCH 3/5] improve --- compiler/commands.nim | 19 ++++++++---- compiler/options.nim | 38 +++++++++++++++++++----- tests/misc/trunner.nim | 65 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 12 deletions(-) diff --git a/compiler/commands.nim b/compiler/commands.nim index 447c86e2d751e..74bd270fd117d 100644 --- a/compiler/commands.nim +++ b/compiler/commands.nim @@ -932,14 +932,23 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo; of "declaredlocs": processOnOffSwitchG(conf, {optDeclaredLocs}, arg, pass, info) of "moduleoverride": - # --moduleoverride:std/sequtils:/tmp/sequtils2:this/test1,this/test2 + # --moduleoverride:std/sequtils:pkg/foo/sequtils2:prefix1,prefix2/sub + proc validate(path: string): string = + if path.isAbsolute or path.isCanonicalPath: result = path + else: localError(conf, info, "module path must be canonical or absolute, got: $1" % path) let args = arg.split(":") - if args.len != 3: + if args.len notin {2, 3}: localError(conf, info, "invalid arg: $1" % arg) else: - let prefixes = args[2].split(",") - for a in prefixes: - conf.localOverrides.add LocalOverride(lhs: args[0], rhs: args[1], prefix: a) + let lhs = args[0].validate + let rhs = args[1].validate + if args.len == 2: + conf.localOverrides.add LocalOverride(lhs: lhs, rhs: rhs, prefix: "/") + elif args.len == 3: + let prefixes = args[2].split(",") + for a in prefixes: + let prefix = a.validate + conf.localOverrides.add LocalOverride(lhs: lhs, rhs: rhs, prefix: prefix) of "dynliboverride": dynlibOverride(conf, switch, arg, pass, info) of "dynliboverrideall": diff --git a/compiler/options.nim b/compiler/options.nim index fc139a44bc070..86a05679ac70e 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -281,8 +281,6 @@ type LocalOverrideAtom* = object LocalOverride* = object - # StringTableRef - # tab*: Table[string, string] lhs*: string rhs*: string prefix*: string @@ -831,6 +829,12 @@ const stdlibDirs = [ const pkgPrefix = "pkg/" stdPrefix = "std/" + systemPrefix = "system/" + +proc isCanonicalPath*(path: string): bool = + # in future work we can support also `this/` for current package. + let path = path & '/' + result = path.startsWith(stdPrefix) or path.startsWith(systemPrefix) or path.startsWith(pkgPrefix) proc getRelativePathFromConfigPath*(conf: ConfigRef; f: AbsoluteFile, isTitle = false): RelativeFile = let f = $f @@ -862,7 +866,22 @@ proc findFile*(conf: ConfigRef; f: string; suppressStdlib = false): AbsoluteFile proc canonicalImport*(conf: ConfigRef, file: AbsoluteFile): string -proc findModule*(conf: ConfigRef; modulename, currentModule: string): AbsoluteFile = +proc canonicalImportPkg*(conf: ConfigRef, file: AbsoluteFile): string = + result = canonicalImport(conf, file) + let tmp = result & '/' + if not (tmp.startsWith(stdPrefix) or tmp.startsWith(systemPrefix)): + result = pkgPrefix & result + +proc pathMatchesPrefix(conf: ConfigRef, path: string, prefix: string): bool = + # see also `prefixmatches.prefixMatch` + if prefix == "/": result = true + elif prefix.isAbsolute: + result = path.isRelativeTo(prefix) + else: # canonical, eg std/foo or pkg/fusion/bar + let canon = canonicalImportPkg(conf, AbsoluteFile(path)) & '/' + result = canon.startsWith(prefix & '/') + +proc findModule*(conf: ConfigRef; modulename, currentModule: string, depth = 0): AbsoluteFile = # returns path to module var m = addFileExt(modulename, NimExt) if m.startsWith(pkgPrefix): @@ -881,12 +900,17 @@ proc findModule*(conf: ConfigRef; modulename, currentModule: string): AbsoluteFi if not fileExists(result): result = findFile(conf, m) if not result.isEmpty: - let canon = canonicalImport(conf, result) - let canonCurrentModule = canonicalImport(conf, AbsoluteFile(currentModule)) & '/' + let canon = canonicalImportPkg(conf, result) for ai in conf.localOverrides: if ai.lhs == canon: - if canonCurrentModule.startsWith(ai.prefix & '/'): - result = findModule(conf, ai.rhs, currentModule) + if pathMatchesPrefix(conf, currentModule, ai.prefix): + if depth > 10: + # can happen with: `--moduleoverride:std/foo2:std/foo2` or `--moduleoverride:std/foo2:/pathto/std/foo2` + # or more complex cases with cycles. Future work could improve things but this is a rare edge case. + # `localError` not defined in scope, alternative is some simple refactoring. + stderr.write "module resolution too deep, possible cyclic overrides detected\n" + return AbsoluteFile"" + result = findModule(conf, ai.rhs, currentModule, depth + 1) break patchModule(conf) diff --git a/tests/misc/trunner.nim b/tests/misc/trunner.nim index 8426e9aeee7a6..8f3772f5456b6 100644 --- a/tests/misc/trunner.nim +++ b/tests/misc/trunner.nim @@ -377,4 +377,69 @@ mused3.nim(75, 10) Hint: duplicate import of 'mused3a'; previous import here: mu """ else: + block: # moduleoverride + proc fn(opt: string, expected: string) = + let output = runNimCmdChk("nimble/mmoduleoverride.nim", fmt"--warnings:off --hints:off {opt}") + doAssert output == expected, opt & "\noutput:\n" & output & "expected:\n" & expected + fn(""): """ +in pkgA.module2 +in pkgC.module2 +in pkgB.module2 +""" + fn("--moduleoverride:pkg/pkgC/module2:pkg/pkgC/module2b"): """ +in pkgA.module2 +in pkgC.module2b +in pkgB.module2 +""" + fn("--moduleoverride:pkg/pkgC/module2:pkg/pkgC/module2b:pkg/pkgA"): """ +in pkgA.module2 +in pkgC.module2b +in pkgB.module2 +in pkgC.module2 +""" + fn("--moduleoverride:pkg/pkgC/module2:pkg/pkgC/module2b:pkg/pkgB"): """ +in pkgA.module2 +in pkgC.module2 +in pkgB.module2 +in pkgC.module2b +""" + fn("--moduleoverride:pkg/pkgC/module2:pkg/pkgC/module2b:pkg"): """ +in pkgA.module2 +in pkgC.module2b +in pkgB.module2 +""" + fn("--moduleoverride:pkg/pkgC/module2:pkg/pkgC/module2b:pkg/pkgA/module2"): """ +in pkgA.module2 +in pkgC.module2b +in pkgB.module2 +in pkgC.module2 +""" + fn("--moduleoverride:pkg/pkgC/module2:pkg/pkgC/module2b:pkg/pkgA/module"): """ +in pkgA.module2 +in pkgC.module2 +in pkgB.module2 +""" + fn("--moduleoverride:std/strutils:pkg/pkgC/module2b"): """ +in pkgC.module2b +in pkgA.module2 +in pkgC.module2 +in pkgB.module2 +""" + let path = testsDir / "nimble/nimbleDir/simplePkgs/pkgA-0.1.0/pkgA/module2.nim" + fn(fmt"--moduleoverride:pkg/pkgC/module2:pkg/pkgC/module2b:{path}"): """ +in pkgA.module2 +in pkgC.module2b +in pkgB.module2 +in pkgC.module2 +""" + # edge case, whether to consider `pkg/tests/nimble/mmoduleoverride` as canonical + fn("--moduleoverride:std/strutils:pkg/pkgC/module2b:pkg/tests/nimble/mmoduleoverride"): """ +in pkgC.module2b +in pkgA.module2 +in pkgC.module2 +in pkgB.module2 +""" + doAssertRaises(AssertionDefect): fn("--moduleoverride:pkgC/module2:pkg/pkgC/module2b:pkg/pkgA/module", "") # pkg/ missing + doAssertRaises(AssertionDefect): fn("--moduleoverride:pkg/pkgC/module2:pkg/pkgC/nonexistent", "") # pkg/pkgC/nonexistent is nonexistent + discard # only during debugging, tests added here will run with `-d:nimTestsTrunnerDebugging` enabled From facd752526c6705a2089142cd1419de3d2f7ad57 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Thu, 15 Jul 2021 00:43:06 -0700 Subject: [PATCH 4/5] add tests --- tests/nimble/mmoduleoverride.nim | 8 ++++++++ tests/nimble/mmoduleoverride.nims | 5 +++++ .../nimbleDir/simplePkgs/pkgA-0.1.0/pkgA/module2.nim | 2 ++ .../nimbleDir/simplePkgs/pkgB-#head/pkgB/module2.nim | 2 ++ .../nimbleDir/simplePkgs/pkgC-#head/pkgC/module2.nim | 1 + .../nimbleDir/simplePkgs/pkgC-#head/pkgC/module2b.nim | 1 + 6 files changed, 19 insertions(+) create mode 100644 tests/nimble/mmoduleoverride.nim create mode 100644 tests/nimble/mmoduleoverride.nims create mode 100644 tests/nimble/nimbleDir/simplePkgs/pkgA-0.1.0/pkgA/module2.nim create mode 100644 tests/nimble/nimbleDir/simplePkgs/pkgB-#head/pkgB/module2.nim create mode 100644 tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2.nim create mode 100644 tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2b.nim diff --git a/tests/nimble/mmoduleoverride.nim b/tests/nimble/mmoduleoverride.nim new file mode 100644 index 0000000000000..19c5536e89bae --- /dev/null +++ b/tests/nimble/mmoduleoverride.nim @@ -0,0 +1,8 @@ +discard """ + matrix: "--moduleoverride:pkgC/module2:std/foo2b:$nim_prs_D/lib/" + # matrix: "--moduleoverride:std/foo2:std/foo2b:$nim_prs_D/lib/" +""" +import std/strutils +import pkgA/module2 as A +import pkgB/module2 as B +import pkgC/module2 as C diff --git a/tests/nimble/mmoduleoverride.nims b/tests/nimble/mmoduleoverride.nims new file mode 100644 index 0000000000000..9621fb29fc12f --- /dev/null +++ b/tests/nimble/mmoduleoverride.nims @@ -0,0 +1,5 @@ +switch("clearNimblePath") +switch("nimblePath", "$projectdir/nimbleDir/simplePkgs") +switch("path", "$nimblepath/pkgA-0.1.0") +switch("path", "$nimblepath/pkgB-#head") +switch("path", "$nimblepath/pkgC-#head") diff --git a/tests/nimble/nimbleDir/simplePkgs/pkgA-0.1.0/pkgA/module2.nim b/tests/nimble/nimbleDir/simplePkgs/pkgA-0.1.0/pkgA/module2.nim new file mode 100644 index 0000000000000..8295bf75606ea --- /dev/null +++ b/tests/nimble/nimbleDir/simplePkgs/pkgA-0.1.0/pkgA/module2.nim @@ -0,0 +1,2 @@ +static: echo "in pkgA.module2" +import pkgC/module2 diff --git a/tests/nimble/nimbleDir/simplePkgs/pkgB-#head/pkgB/module2.nim b/tests/nimble/nimbleDir/simplePkgs/pkgB-#head/pkgB/module2.nim new file mode 100644 index 0000000000000..384a0cba1c0df --- /dev/null +++ b/tests/nimble/nimbleDir/simplePkgs/pkgB-#head/pkgB/module2.nim @@ -0,0 +1,2 @@ +static: echo "in pkgB.module2" +import pkgC/module2 diff --git a/tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2.nim b/tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2.nim new file mode 100644 index 0000000000000..91fd4931d8d81 --- /dev/null +++ b/tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2.nim @@ -0,0 +1 @@ +static: echo "in pkgC.module2" diff --git a/tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2b.nim b/tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2b.nim new file mode 100644 index 0000000000000..565ab2cb60e34 --- /dev/null +++ b/tests/nimble/nimbleDir/simplePkgs/pkgC-#head/pkgC/module2b.nim @@ -0,0 +1 @@ +static: echo "in pkgC.module2b" From 6474b9e777fb14b5acac3c52b9ba6801ae331564 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Thu, 15 Jul 2021 15:57:29 -0700 Subject: [PATCH 5/5] fixup --- compiler/debugutils.nim | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/debugutils.nim b/compiler/debugutils.nim index 7ed431feea224..d109d2121ba69 100644 --- a/compiler/debugutils.nim +++ b/compiler/debugutils.nim @@ -54,5 +54,3 @@ proc isCompilerDebug*(): bool = {.undef(nimCompilerDebug).} echo 'x' conf0.isDefined("nimCompilerDebug") - -include timn/exp/nim_compiler_debugutils