Skip to content

fixes #25235; skipParentCfg in nim.cfg #25307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: devel
Choose a base branch
from
+80 −18
Open

fixes #25235; skipParentCfg in nim.cfg #25307

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a96ada7
init
Graveflo Nov 22, 2025
3f75c15
forgot about nimscript
Graveflo Nov 22, 2025
964357b
fix `skipParentCfg` in project cfg
Graveflo Nov 25, 2025
2cb8dd6
concept patch
Graveflo Nov 26, 2025
d1ada55
add dryrun error handler
Graveflo Nov 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/commands.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,10 @@ proc pathRelativeToConfig(arg: string, pass: TCmdLinePass, conf: ConfigRef): str

proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
conf: ConfigRef) =
if conf.skipParentDetectionMode:
if switch.normalize == "skipparentcfg":
processOnOffSwitchG(conf, {optSkipParentConfigFiles}, arg, pass, info)
return
var key = ""
var val = ""
case switch.normalize
Expand Down
74 changes: 56 additions & 18 deletions compiler/nimconf.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,28 +128,32 @@ proc parseDirective(L: var Lexer, tok: var Token; config: ConfigRef; condStack:
of wEnd: doEnd(L, tok, condStack)
of wWrite:
ppGetTok(L, tok)
msgs.msgWriteln(config, strtabs.`%`($tok, config.configVars,
{useEnvironment, useKey}))
if not config.skipParentDetectionMode:
msgs.msgWriteln(config, strtabs.`%`($tok, config.configVars,
{useEnvironment, useKey}))
ppGetTok(L, tok)
else:
case tok.ident.s.normalize
of "putenv":
ppGetTok(L, tok)
var key = $tok
ppGetTok(L, tok)
os.putEnv(key, $tok)
if not config.skipParentDetectionMode:
os.putEnv(key, $tok)
ppGetTok(L, tok)
of "prependenv":
ppGetTok(L, tok)
var key = $tok
ppGetTok(L, tok)
os.putEnv(key, $tok & os.getEnv(key))
if not config.skipParentDetectionMode:
os.putEnv(key, $tok & os.getEnv(key))
ppGetTok(L, tok)
of "appendenv":
ppGetTok(L, tok)
var key = $tok
ppGetTok(L, tok)
os.putEnv(key, os.getEnv(key) & $tok)
if not config.skipParentDetectionMode:
os.putEnv(key, os.getEnv(key) & $tok)
ppGetTok(L, tok)
else:
lexMessage(L, errGenerated, "invalid directive: '$1'" % $tok)
Expand Down Expand Up @@ -244,6 +248,21 @@ proc getSystemConfigPath*(conf: ConfigRef; filename: RelativeFile): AbsoluteFile
if not fileExists(result): result = p / RelativeDir"etc/nim" / filename
if not fileExists(result): result = AbsoluteDir"/etc/nim" / filename

proc configEnablesSkipParent(conf: ConfigRef; cache: IdentCache;
cfgPath: AbsoluteFile): bool =
let prevMode = conf.skipParentDetectionMode
let prevSkip = optSkipParentConfigFiles in conf.globalOptions
conf.skipParentDetectionMode = true
try:
result = readConfigFile(cfgPath, cache, conf) and
Copy link
Member

@ringabout ringabout Nov 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to --skipParentCfg in .nims files?

Copy link
Contributor Author

@Graveflo Graveflo Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change to --skipParentCfg in .nims files after this PR. That is going to require significantly more invasive changes and it's not worth it. configEnablesSkipParent should only be applied to nim.cfg files as this happens after the system configs have been loaded in and before the final compilation target specific one.

Now that you mention it though, I think this is wrong because module specific configs have to be checked too:

Nim/compiler/nimconf.nim

Line 299 in 3f75c15

if optSkipParentConfigFiles notin conf.globalOptions and not configEnablesSkipParent(conf, cache, pd / cfg):

Copy link
Member

@ringabout ringabout Nov 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, if skipParentCfg is only necessary for cfg files. Perhaps a warning or something is needed if skipParentCfg is used in the .nims files

Copy link
Contributor Author

@Graveflo Graveflo Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a bad idea, but I don't really care about that. I believe you can push to this branch if you want since you are a maintainer.

Copy link
Contributor

@arnetheduck arnetheduck Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm .. the way nimble works, it puts a config section in config.nims - not having nims support would mean it also has to modify nim.cfg - in other words, having support for .cfg files is a good step forwards but it also makes it harder to teach and use the feature, so longer-term, .nims support is also important.

Copy link
Contributor Author

@Graveflo Graveflo Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can agree with the rationale, but the way I see it this feature solves a very specific problem. I think its relatively common to run into seeing as how both of us did, but it's still more of a "bail out" feature. Why would nimble need to use this? It's hard to imagine why this feature would be used for general purposes. The way I see it, this feature depends on how projects are nested on a specific machine, or in some cases how different entry points are nested in self contained repos (meaning this is required in a subdir of the AIO structure not the root). Self contained projects should not worry about the host machine's configuration hierarchy. It would be an impolite move to put skipParentCfg at the root of a code repo, for example.
Also, as a side note, Nim 3 is posturing to have 4 different config files with separate features on top of supporting these two legacy ones so apparently we are moving in this direction.

Copy link
Contributor

@arnetheduck arnetheduck Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in .nims files

in nims files, for ease of processing, this could also be a {.pragma.} which falls outside of the usual side-effecty processing.

Why would nimble need to use this

Nimble controls how workspaces are created, for the user - when creating the workspace, we want nimble test inside a "nested" library to work independently of the parent project configuration - therefore, when nimble injects a package into a nested workspace, it would also "stop" parent cfg configuration for that library (and adjust --path options accordingly) as part of its "install the package" setup - ie this is not something a library developer would do - instead, the package manager would be responsible for it and the skipParentCfg is the means of getting it done.

The mechanism nimble uses for this is to patch config.nims with a special section (atlas also does this) - it's not the prettiest of approaches, but it has the winner property that it's backwards-compatible with old nim releases.

Copy link
Contributor Author

@Graveflo Graveflo Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright then. I don't use nimble or atlas so that might help clear it up for other people like me. When you say it "injects" the packages I assume this is some weird stuff that isn't solved by making a nimble template that drops the nim.cfg by default or just having one set up in the "tests" directory.

optSkipParentConfigFiles in conf.globalOptions
finally:
conf.skipParentDetectionMode = prevMode
if prevSkip:
incl(conf.globalOptions, optSkipParentConfigFiles)
else:
excl(conf.globalOptions, optSkipParentConfigFiles)

proc loadConfigs*(cfg: RelativeFile; cache: IdentCache; conf: ConfigRef; idgen: IdGenerator) =
setDefaultLibpath(conf)
template readConfigFile(path) =
Expand Down Expand Up @@ -275,28 +294,47 @@ proc loadConfigs*(cfg: RelativeFile; cache: IdentCache; conf: ConfigRef; idgen:

if cfg == DefaultConfig:
runNimScriptIfExists(getUserConfigPath(DefaultConfigNims))

let pd = if not conf.projectPath.isEmpty: conf.projectPath else: AbsoluteDir(getCurrentDir())
if optSkipParentConfigFiles notin conf.globalOptions:
for dir in parentDirs(pd.string, fromRoot=true, inclusive=false):
readConfigFile(AbsoluteDir(dir) / cfg)


var
hasProjectCfg = conf.projectName.len != 0
projectConfig = AbsoluteFile ""
if hasProjectCfg:
projectConfig = changeFileExt(conf.projectFull, "nimcfg")
if not fileExists(projectConfig):
projectConfig = changeFileExt(conf.projectFull, "nim.cfg")
if not fileExists(projectConfig):
hasProjectCfg = false

let pd = if conf.projectPath.isEmpty: AbsoluteDir(getCurrentDir()) else: conf.projectPath
if optSkipParentConfigFiles notin conf.globalOptions and
not configEnablesSkipParent(conf, cache, pd / cfg) and
not(hasProjectCfg and configEnablesSkipParent(conf, cache, projectConfig)):
var parentDirs: seq[tuple[path: AbsoluteDir, hasNs: bool]] = @[]
for dir in parentDirs(pd.string, inclusive=false):
var thisReg = (path: AbsoluteDir(dir), hasNs: false)
let cfgPath = thisReg.path / cfg
if cfg == DefaultConfig:
runNimScriptIfExists(AbsoluteDir(dir) / DefaultConfigNims)
thisReg.hasNs = fileExists(thisReg.path / DefaultConfigNims)
if not (thisReg.hasNs or fileExists(cfgPath)):
continue
parentDirs.add thisReg
if configEnablesSkipParent(conf, cache, cfgPath):
break

for i in countdown(parentDirs.len - 1, 0):
let thisReg = parentDirs[i]
readConfigFile(thisReg.path / cfg)
if thisReg.hasNs:
runNimScriptIfExists(thisReg.path / DefaultConfigNims)

if optSkipProjConfigFile notin conf.globalOptions:
readConfigFile(pd / cfg)
if cfg == DefaultConfig:
runNimScriptIfExists(pd / DefaultConfigNims)

if conf.projectName.len != 0:
# new project wide config file:
var projectConfig = changeFileExt(conf.projectFull, "nimcfg")
if not fileExists(projectConfig):
projectConfig = changeFileExt(conf.projectFull, "nim.cfg")
if hasProjectCfg:
readConfigFile(projectConfig)


let scriptFile = conf.projectFull.changeFileExt("nims")
let scriptIsProj = scriptFile == conf.projectFull
template showHintConf =
Expand Down
1 change: 1 addition & 0 deletions compiler/options.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ type
expandPosition*: TLineInfo

currentConfigDir*: string # used for passPP only; absolute dir
skipParentDetectionMode*: bool # true while probing configs for skipParentCfg
clientProcessId*: int


Expand Down
Loading