fix: use execv in proxyexe

[SpotlightKid]

ref https://forum.nim-lang.org/t/12524

Potential fix for #13 on POSIX and Windows systems.

On non-posix/windows systems still uses the old behaviour using startProcess. Use --undef:useExec to switch back to old behaviour on posix too.

[PMunch]

Great work! You do mention however on the forum that some manual steps are required to replace the proxy executables. Maybe it would be better to add something in updateSelf so that a simple choosenim update self would also rebuild these? Not sure why that's not already done, the way it is now the choosenim version only pertains to the actual choosenim binary and the proxy exes are just whichever version you happened to start your choosenim install on.

[SpotlightKid]

Maybe it would be better to add something in updateSelf so that a simple choosenim update self would also rebuild these?

The proxyexe binary is built into the choosenim executable. It gets written to ~/.nimble/bin/{nim,...}, when choosenim switches to a different Nim channel/version.

So actually, contrary to what I wrote on the forum, currently it isn't necessary to remove the old proxyexe binaries, just to switch to a new (or the same) version of Nim with choosenim after updating choosenim. See the code here:

choosenim/src/choosenimpkg/switcher.nim

Line 270 in fc240ac

let proxiesInstalled = params.areProxiesInstalled(proxiesToInstall)

(areProxiesInstalled also checks whether the proxyexe file is up-to-date.)

Updating the proxyexe binaries when choosenim updates itself, can be easily done (see below), but belongs in a different PR, imho, and also doesn't help when somebody just updates choosenim using nimble install choosenim or via a Linux distro package.

diff --git a/src/choosenim.nim b/src/choosenim.nim
index 3306e01..c1b628a 100644
--- a/src/choosenim.nim
+++ b/src/choosenim.nim
@@ -167,6 +167,27 @@ proc updateSelf(params: CliParams) =
   display("Info:", "Updated choosenim to version " & $version,
           Success, HighPriority)
 
+  # Any Nim installation currently activated by choosenim?
+  if $getCurrentVersion(params) != "":
+    var proxiesToInstall = @proxies
+
+    # Handle MingW proxies.
+    when defined(windows):
+      if not isDefaultCCInPath(params):
+        let mingwBin = getMingwBin(params)
+        if not fileExists(mingwBin / "gcc".addFileExt(ExeExt)):
+          let msg = "No 'gcc' binary found in '$1'." % mingwBin
+          raise newException(ChooseNimError, msg)
+
+        proxiesToInstall.add(mingwProxies)
+
+    if not params.areProxiesInstalled(proxiesToInstall):
+      # Create the proxy executables.
+      for proxy in proxiesToInstall:
+        writeProxy(proxy, params)
+
+
+
 proc update(params: CliParams) =
   if params.commands.len != 2:
     raise newException(ChooseNimError,
diff --git a/src/choosenimpkg/switcher.nim b/src/choosenimpkg/switcher.nim
index 2965859..c195e13 100644
--- a/src/choosenimpkg/switcher.nim
+++ b/src/choosenimpkg/switcher.nim
@@ -107,7 +107,7 @@ proc getSelectedPath*(params: CliParams): string =
 proc getProxyPath(params: CliParams, bin: string): string =
   return params.getBinDir() / bin.addFileExt(ExeExt)
 
-proc areProxiesInstalled(params: CliParams, proxies: openarray[string]): bool =
+proc areProxiesInstalled*(params: CliParams, proxies: openarray[string]): bool =
   result = true
   let proxyExe = proxyToUse()
   for proxy in proxies:
@@ -173,7 +173,7 @@ proc getNimbleVersion(toolchainPath: string): Version =
     display("Warning:", "Could not find toolchain's Nimble version.",
             Warning, MediumPriority)
 
-proc writeProxy(bin: string, params: CliParams) =
+proc writeProxy*(bin: string, params: CliParams) =
   # Create the ~/.nimble/bin dir in case it doesn't exist.
   createDir(params.getBinDir())
 

[SpotlightKid]

Now that there is an implementation of this PR for Windows as well, I will try to add sensible tests. Not sure how yet, but I will come up with something.

[RokkuCode]

Hi there, thanks for your work. I wanted to test your implementation, but i have seen, that your fork does not implement the newest additions. could you update your branch to the latest commits, so i can try it out? i would appreciate it.

[SpotlightKid]

could you update your branch to the latest commits, so i can try it out?

I've rebased the PR branch on master.

Ignore all the commits listed above. I'm not sure what I did there but I hope everything is in order again now.

[SpotlightKid]

I never seem to get around to writing proper tests for it. In fact, I don't really know how to best write these test and I could use some help!

I've bee using this patch in the AUR package of choosenim for months now and it works perfectly for me and I haven't got any reports of problems with it from other AUR user either.

[Araq]

@SpotlightKid yet the CI is red, what's up with that?

[SpotlightKid]

@Araq The CI uses an outdated runner. Support for ubuntu-20.04 has been removed from Github Actions recently.

I'll have a look what's up with the other OSes.

[ringabout]

CI failure is not caused by this PR, we need to wait for nightlies fixing

[RokkuCode]

a little feedback: i am using your patches since march and hadn't any problems with them. yes, there are missing test, but the current situation for linux users is bad, especially for newcomers, because they hit multiple walls until to find the right issues and fix it by hand. so i would vote for applying the patches, to get a step forward. better as staying in the current situation.