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

Files do not restore their preferred resource path casing when reloading window #102627

Closed
haxiomic opened this issue Jul 15, 2020 · 17 comments
Closed
Assignees
Labels
debt Code quality issues *out-of-scope Posted issue is not in scope of VS Code workbench-editors Managing of editor widgets in workbench window

Comments

@haxiomic
Copy link

haxiomic commented Jul 15, 2020

  • VSCode Version: 1.48.0-insider and 1.47.1
  • OS Version: Darwin x64 18.7.0 / macOS 10.14.6

If I change the casing of a filename in vscode on macOS, the old file name casing is forever stored in ~/Library/Application Support/Code/workspaceStorage, this causes problems with extensions that require case-sensitive file paths

Steps to Reproduce:

  1. In vscode, create a file named EXAMPLE.txt
  2. Rename this file to Example.txt
  3. Right click on the filename in the top bar to see the case hasn't changed everywhere in vscode
    Screenshot 2020-07-15 at 20 05 19
  4. Extensions that require case-sensitive file paths will no longer work for this file. This persists through restarts and cache clearing. The only way to restore functionality is by deleting the workspace storage file in ~/Library/Application Support/Code/workspaceStorage

This has come up when using the haxe language extension – after changing a file's case, the old file case given by vscode is sent to the haxe compiler, breaking autocomplete for this file (haxe nightly build 4.2)

Does this issue occur when all extensions are disabled?: Technically Yes (but it's probably not an issue without extensions)

@bpasero
Copy link
Member

bpasero commented Jul 16, 2020

@haxiomic isn't it enough to close the file and open it again? Having a hard time to understand how the casing of the file is persisted even when you close the file. What storage key do you still see the old casing persist?

@bpasero bpasero added the info-needed Issue requires more information from poster label Jul 16, 2020
@haxiomic
Copy link
Author

haxiomic commented Jul 16, 2020

@bpasero closing the file and opening has no affect, I’ve also tried

  • restarting vscode
  • deleting the file and recreating it with the same name (with any variation of casing)
  • clearing editor history
  • deleting vscode cache files manually

The extension still receives the original incorrect name

The problem is original name is always kept somewhere inside the workspaceStorage folder, I assume whatever checks are used when updating this state ignore file name casing so it never changes. The only workaround I’ve found is to delete this folder manually

@bpasero
Copy link
Member

bpasero commented Jul 16, 2020

@haxiomic without knowing the exact storage key that the extension is trying to access it is hard to understand the issue. It is possible that maybe the extension stores the URI?

@haxiomic
Copy link
Author

haxiomic commented Jul 16, 2020

Looks to be in codelens/cache2

I ran through the repro steps, renaming EXAMPLE.hx to Example.hx and in state.vscdb, the name is Example.hx everywhere except codelens/cache2:

{
    "file:///Users/geo/Downloads/haxe-name-bug/EXAMPLE.hx": {
        "lineCount": 9,
        "lines": []
    }
}

I don't know a lot about codelens at this point but this is the jsonrpc object pass in the vshaxe extension for autocomplete:

{
    "jsonrpc": "2.0",
    "id": 17,
    "method": "display/completion",
    "params": {
        "file": "/Users/geo/Downloads/haxe-name-bug/EXAMPLE.hx",
        "contents": "class Example {\n\n\tstatic var x:Int;\n\n\tstatic public function main() {\nxx\n\t}\n\n}",
        "offset": 72,
        "wasAutoTriggered": false,
        "meta": [":deprecated"]
    }
}

@bpasero bpasero assigned jrieken and unassigned bpasero Jul 16, 2020
@bpasero bpasero removed the info-needed Issue requires more information from poster label Jul 16, 2020
@bpasero
Copy link
Member

bpasero commented Jul 16, 2020

Thanks for finding out about the key.

@jrieken jrieken added the info-needed Issue requires more information from poster label Jul 16, 2020
@jrieken
Copy link
Member

jrieken commented Jul 16, 2020

This doesn't make sense. The codelens/cache2 cache is to persist info about where code lenses have been so that you don't see flicker after restarts or when changing files. However, is doesn't define the path of the model so it cannot be cause such issues.

@jrieken jrieken removed the info-needed Issue requires more information from poster label Jul 16, 2020
@jrieken jrieken assigned bpasero and unassigned jrieken Jul 16, 2020
@jrieken
Copy link
Member

jrieken commented Jul 16, 2020

Looks like the editor input serialised and restores the wrong uri

Screenshot 2020-07-16 at 17 52 34

@bpasero
Copy link
Member

bpasero commented Jul 17, 2020

I cannot reproduce an issue here. My steps are the following:

  • open a file super.ts
  • notice how codelens caches super.ts because it uses the text model URI
  • rename super.ts to SUPER.ts
  • since we cache canonical URIs in memory, for as long as VSCode is open, the URI will be super.ts
  • in addition, since the editor input was created with the canonical URI, we store and restore this URI, the other one is just used as label information

To get out of this you will have to:

  • close the editor (needed so that the next time you open the editor it will use the new URI)
  • reload the window (needed because we persist canonical URIs in memory)
  • open the editor and the URI is now correct and codelens cache is also correct

I am thinking this just works as designed and is a consequence of all the work we did to fix #12448

@jrieken

Looks like the editor input serialised and restores the wrong uri

Well, not sure I would agree? You store canonical URIs in memory, so when the editor input gets created, even when the user changed the URI, the input will get your canonical form. I will store and restore only this form because for the editor input that is the only correct URI.

@bpasero bpasero added the under-discussion Issue is under discussion for relevance, priority, approach label Jul 17, 2020
@jrieken
Copy link
Member

jrieken commented Jul 17, 2020

Has nothing to do with code lens. Do the following

  • have an extension that does this: vscode.workspace.onDidOpenTextDocument(d => console.log(d.uri.toString()))
  • open sample.txt in an editor, notice how its path is logged to the console
  • rename the file to sAMple.txt, the editor title updates, keep the editor open
  • reload the window
  • 🐛 the extension logs the old name

Yes, we have defined a canonical URI but not so that it survives a reload of the window/process. The form on disk should be the actual form and it seems that the serialised editor input defines the "wrong" canonical uri here.

@bpasero bpasero added debt Code quality issues and removed under-discussion Issue is under discussion for relevance, priority, approach labels Jul 17, 2020
@bpasero bpasero added this to the July 2020 milestone Jul 17, 2020
@bpasero
Copy link
Member

bpasero commented Jul 17, 2020

File editor inputs now keep track of their preferredResource. This is the resource that was originally used by clients when wanting to create an input but it might be different from the resource given our canonical form. However, when the file editor input is being recreated from storage, it will use preferredResource and thus has a chance of being created with the "correct" URI.

This still requires a restart of VSCode but you no longer have to close the editor to get the "correct" URI.

@bpasero
Copy link
Member

bpasero commented Jul 17, 2020

Cannot do this, I see data loss when the file is dirty:

  • open a file
  • make it dirty
  • rename the file to different case
  • reload window
  • => data gone

@bpasero bpasero removed this from the July 2020 milestone Jul 17, 2020
@bpasero bpasero changed the title macOS file case-sensitivity issue: wrong filename stored in workspaceStorage/ Files do not restore their preferred resource when reloading window Jul 17, 2020
@bpasero bpasero added the workbench-editors Managing of editor widgets in workbench window label Jul 17, 2020
@bpasero
Copy link
Member

bpasero commented Jul 17, 2020

Very tricky, not having a good solution for now. The truth is that backup identifiers are the hash of a file path, so when an editor opens and gets dirty, the backup is a hash of its file path (preserving the casing). When the file is renamed, the hash does not change because we no longer change the underlying model, we just change the label.

If we now simply go ahead and change the model URI on restart because we pick the preferred URI now, the backup can no longer be found and the data is lost. Somehow the backup service would also need to be aware of a preferred URI vs initial URI.

The workaround for now is as I stated before:

  • close the editor
  • reload window
  • open the editor

@jrieken
Copy link
Member

jrieken commented Jul 17, 2020

Would it make sense to validate the persisted URI before making it the canonical one? Maybe not the whole path but just the basename?

@bpasero
Copy link
Member

bpasero commented Jul 17, 2020

@jrieken can you clarify what that would mean?

@jrieken
Copy link
Member

jrieken commented Jul 17, 2020

When restoring an editor input from a case-insensitive file system, check for its actual casing before proceeding to open.

@bpasero
Copy link
Member

bpasero commented Jul 17, 2020

Unfortunately there is no straightforward solution to know what casing a file has on disk other than doing readdir for each folder segment of a path and then using the name that is returned to build up the path.

node.js recently added fs.realpath.native but from my knowledge, this e.g. would convert a Windows subst path to its real path from C: drive, which is not really what we want.

And even if we decide to figure out the real path as it is on disk and then proceeding to create a model with that path, backups would still be stored under the previous case and fail to be looked up.

I think it is not impossible to fix this, e.g. we have a method to iterate over all backups and could then use the proper IURIIdentityService.extUri to compare for a backup instead of relying on the strict equalness. But since I leave on vac soon, I would only do this change when I am back.

Bottom line: I think the backup service should take the OS path case into account when answering the question wether a backup exists for a resource or not.

@bpasero bpasero changed the title Files do not restore their preferred resource when reloading window Files do not restore their preferred resource path casing when reloading window Jul 21, 2020
@bpasero bpasero added the *out-of-scope Posted issue is not in scope of VS Code label Nov 3, 2020
@bpasero bpasero closed this as completed Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Code quality issues *out-of-scope Posted issue is not in scope of VS Code workbench-editors Managing of editor widgets in workbench window
Projects
None yet
Development

No branches or pull requests

5 participants
@bpasero @jrieken @isidorn @haxiomic and others