Fix rebuild logic binary checks for windows

[gradha]

Looking at the logic of #23 the code performs the nakefile check like:

binaryTime = toSeconds(getLastModificationTime("nakefile"))

This doesn't include the exe extension for the windows binary, so it will likely fail. Or did windows drop file extensions for binaries?

[fowlmouth]

turns out those dont work on windows, it just raises the exception and goes along, can be fixed with findExe("nakefile"), we can expect there to not be another nakefile.exe on PATH, right?

[fowlmouth]

ok so I had this mostly working then I discovered why findExe("nake") wasnt working, in my nimble/bin the exes don't have exe in them, just .cmd launchers

[gradha]

Instead of using findExe I was more thinking of something like getFilePermissions("."/addFileExt("nakefile", ExeExt)) to avoid looking at the path and be more in line with the last shell call which also prefixes the current directory to avoid troubles.

The .cmd extension you mention looks problematic, should then nim's stdlib findExe be upgraded to look for such extension on windows? nake could of course provide it's own implementation, but if this is the usual way of installing binaries on windows with nimble, other programs depending on calling findExe for nimble binaries would fail too?

[fowlmouth]

os.findExe doesnt look like it would be easy to fix, it relies on a const that says what an "exe" is for a platform and it only considers there to be one exe suffix for any platform. I'll write a specialized version for nake

[gradha]

Maybe a good idea to avoid painting ourselves into a corner would be to leave the interface open through a default parameter or a global variable:

import os

when defined(windows):
  const defaultBinaryExtensions = ["exe", "com", "bat"]
else:
  const defaultBinaryExtensions = ["", "sh", "bin"]

proc findBinary(filename: string,
    extensions: varargs[string] = defaultBinaryExtensions) =
  let dummyPaths = ["dir1", "dir2", "dir3"]
  for path in dummyPaths:
    for extension in extensions:
      echo "Testing ", path/filename.addFileExt(extension)

when isMainModule:
  findBinary("compiler")
  findBinary("compiler", ["cmd", "shell"])

The global variable version would have the advantage of affecting all findBinary calls, even those not in reach by end user code, so maybe its better? Your call.

[fowlmouth]

I'd prefer something like this, only difference is slightly less string ops (we can delay adding the path until the file is found)

proc findFile(filename: string,
    extensions: openarray[string] = defaultBinaryExtensions,
    paths: varargs[string]): string =
  result = ""
  if paths.len < 1: return
  let curdir = getCurrentDir()
  block search:
    for path in paths:
      setCurrentDir(path)
      for extension in extensions:
        let f = filename.addFileExt(extension)
        if fileExists(f):
          result = path / f
          break search
  setCurrentDir(curdir)

[gradha]

I imagine changing the current environment directory would be more expensive than a few string concatenations, but then it's not like performance is critical, it is already going to be slow with the calls to fileExists.

[Araq]

Newer Nim's findExe also considers .cmd and .bat extensions out of the box.