`workbench.action.files.revert` doesn't update readonly indicator when file mtime becomes earlier

[gjsjohnmurray]

Type: Bug

I work with an unusual environment (InterSystems IRIS) in which source code is stored within the environment.

An InterSystems VS Code extension implements a FileSystemProvider to support editing of this code.

My company has a specialized source control tool (Deltanji) which uses a "reserve-before-edit" paradigm.

A FileSystemProvider stat call on an unreserved file returns the FilePermission.Readonly bit in permissions. This causes VS Code to treat the file as readonly and display the padlock icon on its tab :+1:

When a file is reserved the next stat reports a new mtime and clears the bit. This removes the padlock and enables editing :+1:

If the user then cancels the reservation a workbench.action.files.revert command is run by the source control extension. This correctly loads the original version of the code, but despite stat also setting the readonly bit in permissions the padlock icon does not appear and the code remains editable :bug:

I traced the cause to this code:

https://github.com/microsoft/vscode/blob/603b0ee03b82cc77f90f56a5e0ecfbeee3213de1/src/vs/workbench/services/textfile/common/textFileEditorModel.ts#L1009-L1014

I will submit a PR.

VS Code version: Code - Insiders 1.92.0-insider (82104a3a6cf8713b81e5bbd97960dbf5c82a816a, 2024-07-05T00:00:00.000Z) OS version: Windows_NT x64 10.0.22631 Modes:

[bpasero]

It is a requirement of a VS Code file system provider that mtime advances, when it changes:

https://github.com/microsoft/vscode/blob/b7c13a78e93c2762972be705c032fe8295a87ddb/src/vscode-dts/vscode.d.ts#L8739-L8746

That area that is touched in your PR is quite sensitive to bugs and possible data corruption / race conditions. It is unlikely to be accepted.

[bpasero]

However, I would be less opposed to having a way to update the permissions thing specifically, without altering the mtime. So, leave the mtime and related property as is, but update permissions.

If we do so, note that there is another location as well:

https://github.com/microsoft/vscode/blob/b7c13a78e93c2762972be705c032fe8295a87ddb/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts#L1228-L1249

And finally, instead of doing this from the revert call, why not always do it when forceReadFile is used?

[gjsjohnmurray]

@bpasero I have pushed changes to the PR which I think do what you suggested.

[vs-code-engineering[bot]]

Issue marked as unreleased but unable to locate closing commit in repo history. If this was closed in a separate repo you can add the insiders-released label directly, or comment \closedWith someShaThatWillbeReleasedWhenThisIsRelesed.

[vs-code-engineering[bot]]

        Issue marked as unreleased but unable to locate closing commit in issue timeline. You can manually reference a commit by commenting `\closedWith someCommitSha`, or directly add the `insiders-released` label if you know this has already been releaased

[bpasero]

Latest insiders should include this change: https://code.visualstudio.com/insiders/

[gjsjohnmurray]

/verified