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

Add option to always open files at their canonical path #130082

Open
koenlek opened this issue Aug 4, 2021 · 24 comments
Open

Add option to always open files at their canonical path #130082

koenlek opened this issue Aug 4, 2021 · 24 comments
Labels
feature-request Request for new features or functionality file-io File I/O keep Issues we should not close as out of scope workbench-editors Managing of editor widgets in workbench window
Milestone

Comments

@koenlek
Copy link

koenlek commented Aug 4, 2021

Problem

Symlinks can sometimes lead to the same files being opened more than once in VSCode. An example in which this happens is in a bazel run sandbox, which symlinks files together into a sandbox environment. Another example I've seen is when python modules (spread over various folders in a git repo) are symlinked into 1 virtual env python path tree. More generally speaking, symlinks are sometimes used by build tooling to create one consolidated environment out of files spread over different places.

Unfortunately, this can lead to a poor user experience in VS Code. Debuggers can start opening multiple copies of the same file (the one you'd like to edit, which likely is under source control, and one at the path at which it appears in the sandbox) and the UI/UX experience is clumsy at best (breakpoints can be set in each copy of the file and actually work for both, but visually aren't synced). Language servers like pylance can resolve imports to the symlinked venv. So opening the definition of a function can open the symlinked copy, rather than the original (which is likely under source control and the one that you'd actually like to edit)... Again: clumsy UI/UX, such as that that symlinked copy doesn't show source control change info (lines added/rm/changed, gitlens).

Proposed solution

I've seen various tickets related to this and generally things seem to get stuck in the discussion on defining a solution. I'd like to take another attempt at that.

How about we introduce a boolean option openFilesAtCanonicalPath (which people can set on a user or a workspace level)?

The "canonical path" would be the path that readlink -f {some_path} would give on linux: i.e. the path with all symlinks resolved. This is the 'original' path for the file. This would:

  1. Make sure there's always only 1 copy of a file opened
  2. Have the file opened at the path that the user is most likely interested in. If it's part of the workspace, then this would be the path that is under version control (and all things like git/gitlens would work). If it's not part of the workspace, so be it, it's likely some system level dependency or so, still you'd be looking at the original file, which is great).

I wonder what VS Code devs thinks of this.

Related tickets: microsoft/pylance-release#478, #34627, #100533, microsoft/vscode-python#15897

Final remarks

We could even expand this idea to extensions if desired (extensions could retrieve both the requested path and canonical path of a file through extensions api), though I think many of the discussions around better extension support for symlinked files are maybe already solved by just opening the canonical file instead.

Please note that I'm focussing purely on this problem as introduced by symlinks in macOS and Linux. I'm not sure about Windows or other types of links like hard links, macOS aliases, etc. So far, I've seen build tooling use symlinks for these kind of things so addressing that can hopefully solve the majority of issues out there.

@jll63
Copy link

jll63 commented Mar 25, 2022

This has not been fixed yet? This is so annoying.

@GeekyMonkey
Copy link

Getting this onto the Backlog milestone is not much to celebrate. There are currently over 2900 tickets in that milestone. How do we get this bumped to a milestone that will actually happen?

@misha-antonenko
Copy link

misha-antonenko commented Nov 13, 2022

Hello, how would one approach fixing this? Is this time-consuming?

@ludakhris
Copy link

Any updates?

@bpasero bpasero added the *out-of-scope Posted issue is not in scope of VS Code label Dec 6, 2022
@vscodenpa
Copy link

We closed this issue because we don't plan to address it in the foreseeable future. If you disagree and feel that this issue is crucial: we are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding, and happy coding!

@vscodenpa vscodenpa closed this as not planned Won't fix, can't repro, duplicate, stale Dec 6, 2022
@avkonst
Copy link

avkonst commented Dec 6, 2022

I think the issue is crucial. For environments / folders with lots of links, the usability of VS Code is below average.

@bpasero
Copy link
Member

bpasero commented Dec 7, 2022

Please see #167999 for our plan this month to clean up and "groom" all opened issues. This issue was closed as part of that initiative.

@ipkiss42
Copy link

ipkiss42 commented Dec 8, 2022

I also think this issue is crucial. Let me explain why.

Even though not expanding symlinks to a canonical path may look benign, the impact is much bigger than it seems. In fact, this symlink issue is at the core of multiple bugs with very different symptoms, in VS Code itself as well as in multiple extensions. Here are just a few examples:

The variety of these symptoms "dilutes" the votes from affected users, because most of the time users didn't get directed to a central tracking issue. The same applies to all the other issues I haven't mentioned above. And to duplicates of those issues. And to symlink-related issues whose root cause was never identified. This amounts to a large user impact, unfortunately very hard to track by votes alone due to the nature of the issue.

IIUC, resolving symlinks to their canonical path was the previous behavior of VS Code. I don't think this decision should be reverted, because it unblocked multiple use cases (see e.g. #18837), which shouldn't be broken again.

Expecting the entire ecosystem to fix these issues might work in the very long-term, but if issues keep getting resolved as "out-of scope" (like microsoft/vscode-java-test#630 or #74104), this won't be happening anytime soon. In the meantime, users have to pay attention to always using canonical paths when opening files and folders... assuming they are even aware of the issue.

I like @koenlek's proposal in this issue for a few reasons:

  • It is opt-in, so by default it won't reintroduce the issues (e.g. Do not fs.realpath workspace paths so that symlinks are left intact #18837).
  • It is not opinionated regarding what an ideal long-term solution might be.
  • It makes troubleshooting easier ("does the problem go away if you set openFilesAtCanonicalPath to true?").
  • It allows avoiding the issue for many users at once. At my workplace, the default environment setup uses multiple symbolic links, so every developer can be impacted by the side-effects of this issue. A setting would allow for central remediation (e.g. at install time).
  • It sounds easy to implement (just guessing here, I haven't looked at the actual code).

I sincerely hope you will reconsider the decision to close this issue. But regardless of what you decide, I also want to thank you for all the amazing work on VS Code!

@dewilcox
Copy link

dewilcox commented Dec 8, 2022

I think this is a crucial issue too. I was going to explain my reasons, but @ipkiss42 said it better.

@bpasero
Copy link
Member

bpasero commented Dec 8, 2022

@ipkiss42 that is a great summary, thanks! However, here the ask is to follow symlinks of files that get opened, though you list issues about opening a folder that is a symlink. If I understand the original ask here then it is to resolve symlinks when opening an editor, i.e. always check if it is a link or not. But this issue is not about resolving the possible symlink when opening a folder? Or would the setting apply to both scenarios?

@ipkiss42
Copy link

ipkiss42 commented Dec 9, 2022

It should apply to both scenarios. And also to opening a workspace file, if this is not covered by the regular way of opening a file.

My mental model is that all these issues, at their core, boil down to a file path comparison done somewhere. For example, the logic to decide between reusing a tab vs. creating a new one checks if there is already a tab open for the given path. If not, it opens a new one. When faced with two representations of the same path (e.g. a canonical one and one with symlinks), the comparison treats them as different and we end up with two tabs.

You can easily reproduce it:

  • Open a folder (Ctrl-K Ctrl-O) via a non-canonical path.
  • Open a file in that folder, via the explorer.
  • Open the same file via its canonical path in the UI (Ctrl-O).
  • You now have two different tabs for the same file. You can hover over each tab to see the corresponding absolute paths.

In this example, the path of the first file was constructed from the non-canonical folder path and from the relative path to the root, so it was different from the canonical path of the second file.

Conclusion: the setting is effective only if it applies to every "entry point" from where we get absolute paths.

This brings an interesting question: why do we end up with different representations of file paths today, since the UI never expands them to canonical paths? Shouldn't they all be non-canonical, thus avoiding any issue when comparing paths?
Obviously, some places in the code are making paths canonical, or they are introducing canonical paths themselves (behaving as additional "entry points"). Language servers, linters, and more generally all the tools integrating with VS Code and its extensions may provide paths which end up being compared.

Unfortunately, there is no reason why tools should all agree on using canonical or non-canonical paths. As a result:

  • For tools emitting relative paths, canonical paths, or paths constructed from other paths provided by VS Code, the new setting should work just fine.
  • For tools emitting non-canonical absolute paths, the new setting may introduce new problems (with similar symptoms as today).

I believe this is still better than the current situation, because:

  • Tools are much more likely to go from a non-canonical path to a canonical one (a simple API call in most languages) than the opposite. IMHO, this is why the issue is so widespread today.
  • VS Code (and extensions) could canonicalize paths coming from tools when the setting is active. Or the tools themselves could respect the setting.

So instead of having tools deciding independently whether or not to canonicalize paths, the setting provides a path forward for eventually aligning every tool in the ecosystem, either natively (in the tool itself) or in the integration layer.

@ipkiss42
Copy link

ipkiss42 commented Dec 9, 2022

BTW, another way to look at it is that the setting would buy us time to figure out a better approach.

Maybe paths should be made canonical in all the places where path comparisons are done? I don't know, but it may take time to decide and act on it. I'd rather be unblocked via the setting in the meantime.

@bpasero
Copy link
Member

bpasero commented Dec 9, 2022

Just a quick reply without going deep (yet): VS Code internally uses and identifies everything via URI class which is essentially a path with a scheme. We have a canonical form of it which is simply whoever first introduces a URI it becomes the canonical form (code pointer). This helped us avoid issues on case insensitive platforms such as macOS and Windows to not open the same file with different casing.

However, things are more complicate than that: not only can a file be expressed by different cases and still point to the same file on disk, symbolic links can as well. However, symbolic links cannot be resolved synchronous, you need to ask the OS for its resolved form, which is a long running call. All our URI handling however requires synchronous execution.

Bottom line: it would probably be relatively easy to resolve a symbolic link when it comes to opening a folder because that happens very early on startup and I think a setting for that could be added. However, resolving a link as part of opening an editor will be more tricky because essentially this requires us to make canonical URI handling asynchronous.

@ipkiss42
Copy link

ipkiss42 commented Dec 9, 2022

IIUC, it wouldn't be necessary to do it when opening an editor, if the editor is opened via the explorer or via any other mechanism which relies on the (already resolved) folder path. It would be needed only when opening a file "from nothing", e.g. from the UI, in which case doing it synchronously might be acceptable.

@bpasero
Copy link
Member

bpasero commented Dec 9, 2022

I would argue that in 99% of all cases the editor is opened from a component or piece of code that does NOT resolve the symbolic link if there is one. Not to speak about extensions who can also open any URI as editor where we do not know if the file is resolved or not.

@bpasero
Copy link
Member

bpasero commented Dec 10, 2022

My mental model is that all these issues, at their core, boil down to a file path comparison done somewhere.
Shouldn't they all be non-canonical, thus avoiding any issue when comparing paths?
This brings an interesting question: why do we end up with different representations of file paths today, since the UI never expands them to canonical paths?

Yes, the issue is comparing paths, or in our case Uri. In many places where we deal with identity of a Uri we use a class called ResourceMap:

export class ResourceMap<T> implements Map<URI, T> {

When deciding if a Uri already exists in a component, we check this map which in the end will do a path and scheme comparison:

private static readonly defaultToKey = (resource: URI) => resource.toString();

For editors to decide if an editor is already opened we do something similar:

return isEqual(otherInput.resource, this.resource);

The key thing here is that when a Uri enters our system, we try to use its canonical form:

  • if its the first time we see a Uri we just use that form
  • if the Uri exists with different casing but same value, we use the form that was used first (except for Linux)

We cannot make any assumptions about the form of the Uri when it enters VS Code: it can come from extensions, it can be the user clicking a file in the explorer, it can be any code that constructs a Uri from an existing path or by path concatenation.

In order to support symbolic links, we essentially need to resolve the symbolic link and then use the resolved value as canonical form, not the symbolic link one. The issues at hand are:

  • this is a long running file system operation, this requires Promise based execution
  • we currently do not allow link resolution in our file system providers (File Provider: support symbolic link operations #71204) so it would only work for local files but not in remote scenarios

Happy to discuss this further. Again, I think resolving a link when opening VS Code would be straight forward but it does not seem to address all the issues.

@bpasero bpasero reopened this Dec 12, 2022
@bpasero bpasero added keep Issues we should not close as out of scope file-io File I/O and removed *out-of-scope Posted issue is not in scope of VS Code labels Dec 12, 2022
@bpasero
Copy link
Member

bpasero commented Dec 12, 2022

Since there is a good amount of discussion and value in this issue (esp. #130082 (comment)), I am good to reopen but would still not make any promises as to when and if ever this issue is being addressed. There are blockers and questions on the way:

  • what does it mean for when an extension brings in thousands of paths (such as diagnostic markers), do we resolve links all at once or somehow deferred only when opening an editor?
  • what does it mean when a (symbolic link) path for opening suddenly turns into a different (resolved) path, we need to make the URIIdentityService fit to compare all these paths, so we need a Map of paths to target?
  • do we know if fs.readlink is correct to be used, or should it be fs.realpath or even fs.native.realpath and how do we bring this into the file system providers?

@dschwen
Copy link

dschwen commented Nov 19, 2023

This issue is the bane of my existence. I work in a large codebase that - as part of its build system - symlinks all headers into a single path, to keep the include path list managable. Compile errors in the terminal list the symlinks as the error location, so when clicking those errors the editor opens the symlink. However the actual file is likely to be open as well due to teh header/source switching plugin we use. Both symlink and file open leads to frequent corruption of header files when enabling format on save (C++). These errors are sometimes not obvious and if not caught right away will mess up your day when you continue editing the files.

@kamil-wojcicki
Copy link

kamil-wojcicki commented Jul 16, 2024

This is a frequent issue during remote debug on servers with environments that use symlinks e.g. to streamline paths.

Fyi, PyCharm's approach was to allow the user to define path mappings:
https://www.jetbrains.com/help/pycharm/edit-project-path-mappings-dialog.html

Note, the case there was different: they work with local and remote copies of the code, while VSCode works only with a remote copy. However, using a mapping approach would allow the user to configure with prefix path differences to "ignore" and hence avoid opening a separate copy of the same file under a different path.

For example: /home/user/code could be mapped to underlying path /mounts/<deep_and_complex_path_structure>/user/code avoiding the issues.

@gedalia
Copy link

gedalia commented Aug 15, 2024

It looks like part of this (Bazel sandbox python files open into a window) was turned into a issue here: microsoft/vscode-python#17002 we (Boston Dynamics) are using debugpy and it really gets annoying that this doesn't work better, its there any way to get this improved, @brettcannon I'm happy to get you 5 upvotes if that's what it takes.

@brettcannon
Copy link
Member

@gedalia voting is different per repo (if they use voting at all). But if there's a specific Python issue you can open it on the Python extension.

@gedalia
Copy link

gedalia commented Aug 15, 2024

@brettcannon it looks like you opened it and then closed it. Should I just create a new request with the same contents? or is this a issue with https://github.com/microsoft/debugpy hard for me to tell.

@gedalia
Copy link

gedalia commented Aug 15, 2024

looks like it might have been done in: microsoft/debugpy#743 I'll have to test that.

@brettcannon
Copy link
Member

@gedalia if the debugpy fix doesn't work, open a new issue on the Python debugger repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality file-io File I/O keep Issues we should not close as out of scope workbench-editors Managing of editor widgets in workbench window
Projects
None yet
Development

No branches or pull requests

15 participants