checkModuleName uses non-canonicalized path name #6547
Conversation
added the non-canonicalized path to TFileInfo
| @@ -476,6 +476,7 @@ type | |||
| TFileInfo* = object | |||
| fullPath: string # This is a canonical full filesystem path | |||
| projPath*: string # This is relative to the project's root | |||
| path*: string # The original path when it was first encountered | |||
There was a problem hiding this comment.
Ugh no, not yet another path thing in here, patch shortName instead perhaps.
There was a problem hiding this comment.
This is getting more complicated now I have thought about it. If the nim compiler were to treat symlinks as normal files, for one unique fullPath (canonicalized as it is now) could correspond to different modules from different symlinks. The information inside TFileInfo does not seem to be enough. I'll try to make a more elaborate test case with symlinks to check.
There was a problem hiding this comment.
We don't really support symlinks, they are a clusterfuck.
There was a problem hiding this comment.
If only we could all run plan9.
There was a problem hiding this comment.
Completely besides the point, Windows doesn't really support symlinks anyway so your codebase will never work with one of the most widespread OSes out there, whether you like it or not. On Unix we can error on symlink'ed files since they are ambiguous for nim's purpose as you noted.
There was a problem hiding this comment.
Supporting symlinks wouldn't make nim break on any OS that lacks symlinks (only plan9 comes to mind).
We can already use nim to create symlinks on Windows, #6287.
|
In light of #6587, do you think if I would think it doesn't make any difference for cases where there is no symlink or hard links, and it would only introduce breakage where people have been using symlinks or hard links. In addition, replacing |
Code evolves, I don't want to have symlinks in the git repo (would that even work accross OSes?) and without having tests in the git repo I cannot ensure that they continue to work.
Supporting a feature has ongoing costs.
I don't know. |
|
I'll wait for #6587 to be merged and try it out. This branch doesn't have to be associated with symlinks, and Nim can of course stay clear from symlinks. |
|
My remarks were ignored. Closing. |
added the non-canonicalized path to TFileInfo.
Should fix #6543 without destroying other things.