--useSystemNim flag broken

[daylinmorgan]

When running 0.22.0 on NixOS I found the --useSystemNim expects the resolved nim binary from findExe to exist in a specific layout, I think based on what the tar.xz from nim-lang is with something like parentDir/bin/nim .

When attempting to corroborate if this was a NixOS specific issue in a container it seemed to also not properly resolve choosenim installs of nim for the same reason that the binary itself is located in ~/.nimble/bin/nim so it sets the dir to ~/.nimble.

nimble/src/nimblepkg/vnext.nim

Line 153 in 802e158

let dir = effectivePnim.parentDir.parentDir

related to #1607

[daylinmorgan]

a generalized solution relying on the resolved nim binary could be something like this:

diff --git a/src/nimblepkg/vnext.nim b/src/nimblepkg/vnext.nim
index 70ebdbf571..f9f8283092 100644
--- a/src/nimblepkg/vnext.nim
+++ b/src/nimblepkg/vnext.nim
@@ -150,7 +150,12 @@
                 effectivePnim = resolvedPath
           except CatchableError:
             discard # Fall back to original pnim
-    let dir = effectivePnim.parentDir.parentDir
+    var dir = effectivePnim.parentDir.parentDir
+    if not fileExists(dir / "nim.nimble"):
+      # fallback for distribution-specific non-standard nim layout
+      let (output, exitCode) = doCmdEx(fmt("{pnim} --hints:off --eval:\"import std/compilesettings;echo querySetting(libPath)\""))
+      if exitCode == 0:
+        dir = output.parentDir
     try:
       return some getPkgInfoFromDirWithDeclarativeParser(dir, options, nimBin = "") #Can be empty as the code path for nim doesnt need it. 
     except CatchableError:

[jmgomez]

Please, PR it

[daylinmorgan]

Actually my proposed solution doesn't work (at least not in the case of NixOS) and also wouldn't work for any situation in which one defines a wrapper for nim.

i.e. if we have something simple like this:

#!/usr/bin/env  bash

# something specific to this "wrapper"
# could be path manipulation, custom flags/env vars
exec /custom/path/to/nim "$@"

Then any attempt to resolve the binary as existing within a specific folder would fail, since with the proposed change we would just end up with /custom/path/to/nim but none of the necessary wrapper environment.

I'd expect that with --useSystemNim, findExe("nim") should be the path used (or --nim:nim-wrapper then nim-wrapper) rather than some pkgInfo and pkgInfo.getRealDir / bin / "nim". But it doesn't seem like that works even with the above changes, I think this is related to what vnext.setNimBin and nimenv.useNimFromDir are doing.

[jmgomez]

We are going to remove the whole setNimBin which is part of the legacy flag after the next minor patch, doing that now is risky cause it can introduce other regressions in legacy. So if you can find a minimal change with low impact that isnt specific to Nix, submit it. Otherwise the real fix will need to wait a bit.

[daylinmorgan]

Seems there is a two-fold issue in the current implementation, getting the nim resolution to find the correct directory and then getting to use the path provided by --nim:<path> or what seemed to be the previous behavior from --useSystemNim which was essentially findExe("nim").

Does the actual compilation exec call:

nimble/src/nimblepkg/vnext.nim

Lines 1092 to 1094 in 1267aa8

	let cmd = "$# $# --colors:$# --noNimblePath $# $# $#" % [
	options.satResult.getNimBin().quoteShell, pkgInfo.backend, if options.noColor: "off" else: "on", join(args, " "),
	outputOpt, input.quoteShell]

need to use satResult.getNimBin() or could it be replaced with option.nim whenever --nim:<path> is provided allowing a combination of --useSystemNim and --nim: to both actually use a system nim and respect the path to the compiler (which might actually not live at /path-to-nim/bin/nim)

Sorry I'm struggling a bit to grok which parts of the current codebase are legacy vs vnext as they seem slightly entangled

I didn't even realize how many setNimBin procs there were, at least one of which is being called in vnext.buildFromDir and then calling useNimFromDir telling the user a particular binary is being used when in fact it isn't the nim being used... because its actually calling a binary directly not using the PATH manipulation from that proc at all.

At least in terms of what I think is vnext related code the resolveAndConfigureNim is not compatible with any kind of wrapper script because it only interprets the nimBin as an indicator of where to search for a nim directory it doesn't actually use that path beyond resolving the directory because the below proc just uses pkgInfo instead:

nimble/src/nimblepkg/packageinfotypes.nim

Lines 189 to 197 in 1267aa8

	proc getNimPath*(pkgInfo: PackageInfo): string =
	var binaryPath = "bin" / "nim"
	when defined(windows):
	binaryPath &= ".exe"
	pkgInfo.getNimbleFileDir() / binaryPath
	proc getNimBin*(nimResolved: NimResolved): string =
	assert nimResolved.pkg.isSome, "Nim is not resolved yet"
	return nimResolved.pkg.get.getNimPath().quoteShell

But it's actually not even that that's used to determine the compiler path, it's determined by a different getNimBin that possibly has a related TODO that hasn't been addressed yet:

nimble/src/nimblepkg/vnext.nim

Lines 977 to 979 in 1267aa8

	proc getNimBin(satResult: SATResult): string =
	#TODO change this so nim is passed as a parameter but we also need to change getPkgInfo so for the time being its also in options
	if satResult.nimResolved.pkg.isSome:

But I think the build command only actually uses that to get the nim path which is independent of the other hard-coded nim binary lookup proc for NimResolved.

Meaning we can have a situation where setNimBin announces to the user a particular nim is being used even when it might differ from the satResult.