Path resolution docs (current state only) v2 #11442
Conversation
|
|
||
| Remix may add other source code providers in the future. | ||
| For a complete description of the virtual filesystem and the path resolution logic used by the | ||
| compiler see :ref:`Path Resolution <path-resolution>`. |
There was a problem hiding this comment.
Maybe I should put the docs back here now that they are shorter?
They're still long enough to warrant a separate page so I left them as a separate page for now but I can easily change it.
cameel
force-pushed
the
path-resolution-docs-current-state-v2
branch
from
97d065f to
0ae7463
Compare
|
I have just pushed an update. Most of the intro to the section about remappings has been rewritten. I think that addresses all the review commments. |
|
For reference, I found the first version of the remapping docs: #348 |
docs/path-resolution.rst
Outdated
| The Host Filesystem Loader will attempt to use it as a path and look up the file on disk. | ||
| At this point the platform-specific normalization rules kick in and ``/project/lib/math.sol`` and | ||
| ``/project/lib/../lib///math.sol`` may actually result in the same file being loaded. | ||
| Note, however, that the compiler will still see them as separate source units that just happen to |
There was a problem hiding this comment.
Maybe we could put this also in a .. note:: section?
There was a problem hiding this comment.
I tried to change it but left it as is after all. This sentence is strongly connected to this paragraph and I think that putting in a note (or even just in a separate paragraph) makes it read weird.
There was a problem hiding this comment.
It's rather confusing because the two import paths somehow appear out of nowhere and it's not clear what they refer to.
There was a problem hiding this comment.
ok, now I get what the problem is. I have reworded that paragraph and changed how the examples are introduced so that now it makes sense as an actual note.
|
Updated. All new changes are in a single fixup commit on top. |
docs/path-resolution.rst
Outdated
| Imports | ||
| ======= | ||
|
|
||
| The import statement specifies an *import path*, which, after being lightly processed, becomes a |
There was a problem hiding this comment.
| The import statement specifies an *import path*, which, after being lightly processed, becomes a | |
| The import statement specifies an *import path*. This path, when relative (it starts with `./` or `../`), is combined with the path of the importing source unit to form the imported source unit name. If it is not relative, it is taken as the imported source unit name directly. |
There was a problem hiding this comment.
Ok, this ignores remappings. But still, I think we should replace "lightly processed" by a short summary of the processing.
There was a problem hiding this comment.
I moved the paragraph about two kinds of import above the example and mentioned ./ and ../. Is that ok?
docs/path-resolution.rst
Outdated
| Note the difference between a *relative import* (e.g. ``import "./util.sol"``) and a | ||
| *direct import with a relative path* (e.g. ``import "util.sol"``). | ||
| It is a relative import only if it starts with ``./`` or ``../``. | ||
| Otherwise it is a direct import. |
There was a problem hiding this comment.
I'm worried that the term "direct import with a relative path" could be confusing. Maybe we could say that unless the import starts with ./ or ../ it is a direct import and is thus absolute and imports do not have to start with / to be absolute or something...
There was a problem hiding this comment.
How about this?
.. note::
``import "./util.sol"`` is a relative import but ``import "util.sol"`` is a direct one.
Relative imports **must** start with ``./`` or ``../``.
VFS does not have a root directory and absolute paths do not have to start with a slash.
While both paths would be considered relative in the host filesystem, ``util.sol`` is actually
absolute in the VFS.There was a problem hiding this comment.
("must start" sound like it is an error otherwise)
.. note::
Imports that start with `./` or `../` are relative imports, while all others are direct (absolute) imports.
Direct imports do to need to start with `/`. This, ``import "./util.sol"`` is a relative import but ``import "util.sol"`` is a direct one.
While both paths would be considered relative in the host filesystem, ``util.sol`` is actually
absolute in the VFS.
"The VFS does not have a root directory" - of course it has, that's where all the topmost files are. You just don't need to use / to reference the root directory.
But yeah, this is probably not too important.
There was a problem hiding this comment.
I really, really do not want to use the term absolute import :)
Point taken about must. Replaced with always, which I hope does not have this connotation.
"The VFS does not have a root directory" - of course it has, that's where all the topmost files are. You just don't need to use
/to reference the root directory.
I think one could also argue that all the topmost items are just floating there without any directory containing them but that would be splitting hairs and a root with an empty name makes sense too so I just removed that part for simplicity:)
So here's another attempt, combining my and your version:
.. note::
Relative imports **always** start with ``./`` or ``../`` so ``import "util.sol"``, unlike
``import "./util.sol"``, is a direct import.
While both paths would be considered relative in the host filesystem, ``util.sol`` is actually
absolute in the VFS.
docs/path-resolution.rst
Outdated
| 1. First a prefix is computed | ||
|
|
||
| - Prefix is initialized with the source unit name of the importing source unit. | ||
| - The last segment is removed from the prefix. |
There was a problem hiding this comment.
| - The last segment is removed from the prefix. | |
| - The last segment (the file name) is removed from the prefix. |
There was a problem hiding this comment.
Also "segment" is introduced here without explanation.
There was a problem hiding this comment.
I was sure it was a common term but apparently it's not. The Wikipedia page for filename does not even mention it.
While segment was not defined, "removal of the last segment" was.
Anyway, added a definition.
docs/path-resolution.rst
Outdated
|
|
||
| - Prefix is initialized with the source unit name of the importing source unit. | ||
| - The last segment is removed from the prefix. | ||
| - Then, for every leading ``../`` segment in the normalized import path the last segment is repeatedly removed from the prefix. |
There was a problem hiding this comment.
This is the first mention of "normalized" - in which way is it normalized?
There was a problem hiding this comment.
It's explained below. I think it flows better that way because first you see the outline and only then the details are filled in. I could add "see below" but there would be a lot of that so I thought it was better as it is now.
docs/using-the-compiler.rst
Outdated
|
|
||
| For security reasons the compiler has restrictions on what directories it can access. | ||
| Paths of source files (and their subdirectories) specified on the command line and paths defined by |
There was a problem hiding this comment.
| Paths of source files (and their subdirectories) specified on the command line and paths defined by | |
| Directories of source files (and their subdirectories) specified on the command line and paths defined by |
(Is it directories or paths?)
There was a problem hiding this comment.
It's directories. Changed to:
Parent directories of source files specified on the command line and target paths of remappings ...I removed the part about subdirectories because I think it should be pretty intuitive. I'd expect this detail to be mentioned only if it was the opposite.
There was a problem hiding this comment.
But if you write 'Parent directories', I would interpret this as all (super-)directories leading up to the file, i.e. if you call solc /a/b/c/d.sol, then everything inside /a is accessible.
There was a problem hiding this comment.
Right. Why do all the terms have to be so ambiguous!
Anyway, changed parent directories to directories now.
cameel
force-pushed
the
path-resolution-docs-current-state-v2
branch
2 times, most recently
from
296dda0 to
23a3781
Compare
|
Pushed an updated version. Ready for another round. I think I have addressed all the comments. |
…ppings, base path and standard input
cameel
force-pushed
the
path-resolution-docs-current-state-v2
branch
from
dd89f53 to
5e37888
Compare
|
Added fixes for two of @chriseth's comments. I also squashed the fixups to make it ready for merge. |
Replaces #11263 and #11105.
Related to #11138, #11036 and #9353.
This PR contains the parts of #11263 that are suitable for inclusion in the docs. I removed most of the examples and focused on laying out the rules. Some parts were also rewritten to make sense without the examples - for example the rules for relative imports are now stated more explicitly.