Skip to content
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.

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

import os + use of existsDir/dirExists/existsFile/fileExists/findExe in config.nims causes "ambiguous call' error #14142

Closed
kaushalmodi opened this issue Apr 27, 2020 · 7 comments · Fixed by #14658

Comments

@kaushalmodi
Copy link
Contributor

In config.nims, when the os module is imported and one of these procs are used:

  • existsDir or dirExists
  • existsFile or fileExists
  • findExe

we get the "ambiguous call" error (because these are defined in both system and os.

Example config.nims

  1. Create this config.nims in any directory
  2. Run nim temp123
# config.nims
import os # commenting this out fixes the error
# import os except existsDir, dirExists, existsFile, fileExists, findExe # this also does not give that error

task temp123, "Temp task":
  echo existsDir("/usr")
  echo dirExists("/usr")
  echo existsFile("/usr/foo")
  echo fileExists("/usr/foo")
  echo findExe("nim")

Current Output

/home/kmodi/sandbox/nim/bug_reports/nimscript_import_os/config.nims(4, 6) template/generic instantiation of `task` from here
/home/kmodi/sandbox/nim/bug_reports/nimscript_import_os/config.nims(5, 17) Error: ambiguous call; both system.existsDir(dir: string) [declared in /home/kmodi/usr_local/apps/6/nim/devel/lib/system/nimscript.nim(135, 6)] and os.existsDir(dir: string) [declared in /home/kmodi/usr_local/apps/6/nim/devel/lib/pure/os.nim(1109, 6)] match for: (string)

Expected Output

Outputs of the echo statements in the above example.

Workaround

  • Use import os except existsDir, dirExists, existsFile, fileExists, findExe, or system. qualifiers with the ambiguous identifiers.

Additional Information

This issue is very closely related to #12835 .

$ nim -v
Nim Compiler Version 1.3.1 [Linux: amd64]
Compiled at 2020-04-27
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 2f1aad02642576d13df018c9e5869c8de7e3a539
active boot switches: -d:release

This used to work at some point (probably around Nim 19.x).

@timotheecour
Copy link
Member

reduced example:
nim e foo.nim

# foo.nim
import os
echo dirExists("/tmp")

This used to work at some point (probably around Nim 19.x).

when ? 1.0.0 gives same error and 0.19.0 gives Error: cannot 'importc' variable at compile time

however, this is annoying so I'm (hopefully) fixing it here: #14143

@metagn
Copy link
Collaborator

metagn commented Apr 28, 2020

Also getEnv, existsEnv, putEnv, delEnv, getCurrentDir. The following procs are also the same idea in os and nimscript but are renamed: removeDir -> rmDir, removeFile -> rmFile, moveDir -> mvDir, moveFile -> mvFile, createDir -> mkDir, copyFile -> cpFile, copyDir -> cpDir.

@Araq
Copy link
Member

Araq commented Apr 28, 2020

They are renamed for a reason!

@metagn
Copy link
Collaborator

metagn commented Apr 28, 2020

Yes, I advocate for that. I don't think any procs should be deleted from nimscript, I just think they should be moved to os then import/exported. That will allow the renamed procs to be imported with an alias like from os import removeDir as rmDir, then export rmDir. This makes maintaining nimscript procs easier, you can find their declarations in the same place their compiled Nim versions are declared. Of course they aren't implemented there, but that's easy to clarify with a doc comment or a code comment in the when defined(nimscript) branch.

@Araq
Copy link
Member

Araq commented Apr 28, 2020

The rmDir set of operatings could be in a new module std / scripting but shouldn't be part of os.nim, their design is based on the idea that you can have a "what if" emulation run that doesn't touch your FS.

timotheecour added a commit to timotheecour/Nim that referenced this issue Apr 29, 2020
…dirExists/existsFile/fileExists/findExe in config.nims
@kaushalmodi
Copy link
Contributor Author

@Araq Would a fix similar to 3d2459b fix this too?

@Araq
Copy link
Member

Araq commented Apr 30, 2020

I think my fix mostly works. :-)

Araq added a commit that referenced this issue May 1, 2020
Araq added a commit that referenced this issue May 1, 2020
@Araq Araq added Nimscript and removed Showstopper labels May 2, 2020
@Araq Araq closed this as completed in 8e04ef3 May 2, 2020
Araq added a commit that referenced this issue May 2, 2020
@Araq Araq reopened this May 2, 2020
EchoPouet pushed a commit to EchoPouet/Nim that referenced this issue Jun 13, 2020
EchoPouet pushed a commit to EchoPouet/Nim that referenced this issue Jun 13, 2020
timotheecour added a commit to timotheecour/Nim that referenced this issue Jun 14, 2020
…dirExists/existsFile/fileExists/findExe in config.nims
timotheecour added a commit to timotheecour/Nim that referenced this issue Jun 15, 2020
…dirExists/existsFile/fileExists/findExe in config.nims
Araq pushed a commit that referenced this issue Jun 16, 2020
* fix #14142: no more clash with: import os + use of existsDir/dirExists/existsFile/fileExists/findExe in config.nims

* remove a comment

* Revert "fixes the regression #12860 caused; hotfix"

This reverts commit 3d2459b.

* Revert "Undefine `paramCount` & `paramStr` in nimscript.nim for *.nims (#12860)"

This reverts commit d38853c.

* noNimScript => noWeirdTarget + noNimJs
kaushalmodi added a commit to kaushalmodi/nim_config that referenced this issue Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment