search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
161.04k stars 28.26k forks source link

Build does not wait for files to save #139764

Open yecril71pl opened 2 years ago

yecril71pl commented 2 years ago

Issue Type: Bug

In order to be able to run my code, I have to modify file:///proc/sys/fs/inotify/max_user_instances first. So I did and I told VS Code to run.
This file, however, requires privilege escalation to be saved. Here is the order of events:

  1. Ask me whether to escalate.
  2. Build my code.
  3. Run my code.
  4. Get root password.
  5. Save the file.

Here is the expected order of events:

  1. Ask me whether to escalate.
  2. Get root password.
  3. Save the file.
  4. Build my code.
  5. Run my code.

VS Code version: Code 1.63.2 (899d46d82c4c95423fb7e10e68eba52050e30ba3, 2021-12-15T09:39:46.686Z) OS version: Linux x64 5.15.8-1-default Restricted Mode: No

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM)2 Quad CPU Q6600 @ 2.40GHz (4 x 2393)| |GPU Status|2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
opengl: enabled_on
rasterization: disabled_software
skia_renderer: enabled_on
video_decode: disabled_software
vulkan: disabled_off
webgl: enabled
webgl2: enabled| |Load (avg)|3, 4, 4| |Memory (System)|5.66GB (0.12GB free)| |Process Argv|--unity-launch --crash-reporter-id 2a03cb52-9f4b-48bf-93d3-5c0537d534ee| |Screen Reader|no| |VM|0%| |DESKTOP_SESSION|/usr/share/xsessions/plasma5| |XDG_CURRENT_DESKTOP|KDE| |XDG_SESSION_DESKTOP|KDE| |XDG_SESSION_TYPE|x11|
Extensions (26) Extension|Author (truncated)|Version ---|---|--- doxdocgen|csc|1.3.2 vscode-resx-editor|Dom|0.0.5 vscode-pull-request-github|Git|0.34.2 Ionide-fsharp|Ion|5.10.2 better-cpp-syntax|jef|1.15.10 vscode-language-pack-pl|MS-|1.63.3 csharp|ms-|1.23.17 dotnet-interactive-vscode|ms-|1.0.2606010 vscode-dotnet-pack|ms-|1.0.4 python|ms-|2021.12.1559732655 vscode-pylance|ms-|2021.12.2 jupyter|ms-|2021.11.1001550889 jupyter-keymap|ms-|1.0.0 jupyter-renderers|ms-|1.0.4 remote-containers|ms-|0.209.6 remote-ssh|ms-|0.70.0 remote-ssh-edit|ms-|0.70.0 remote-wsl|ms-|0.63.13 cmake-tools|ms-|1.9.2 cpptools|ms-|1.7.1 cpptools-extension-pack|ms-|1.1.0 powershell|ms-|2021.12.0 vscode-markdown-notebook|ms-|0.0.26 vsliveshare|ms-|1.0.5200 vsliveshare-audio|ms-|0.1.91 cmake|twx|0.0.17 (1 theme extensions excluded)
A/B Experiments ``` vsliv368:30146709 vsreu685:30147344 python383:30185418 vspor879:30202332 vspor708:30202333 vspor363:30204092 pythontb:30283811 pythonvspyt551cf:30345471 pythonptprofiler:30281270 vshan820:30294714 vstes263:30335439 pythondataviewer:30285071 vscod805:30301674 pythonvspyt200:30340761 binariesv615:30325510 bridge0708:30335490 bridge0723:30353136 vsaa593cf:30376535 pythonvs932:30410667 vscop804cf:30404767 vscop940:30404999 vsrem710:30416614 ```
bpasero commented 2 years ago

Well I think "build/debug" is waiting for files to save, but since the save fails (requiring admin password) it ignores that and continues to run. Maybe the user needs to be asked in this case.

roblourens commented 2 years ago

Sorry I don't understand. The OP says that build/run happened before saving? Does this resolve before the escalation process is done? https://github.com/microsoft/vscode/blob/e7b3724e0cdc9095d73a06b47203baa2c1777ea8/src/vs/workbench/contrib/debug/common/debugUtils.ts#L315

yecril71pl commented 2 years ago

The launcher should check the return value.

https://github.com/microsoft/vscode/blob/e7b3724e0cdc9095d73a06b47203baa2c1777ea8/src/vs/workbench/services/editor/common/editorService.ts#L273

roblourens commented 2 years ago

I see that but I would expect that it doesn't resolve while the user is typing the password and then resolves true when the password is correct.

roblourens commented 2 years ago

Oh, I suppose it bails when it shows the notification to escalate

yecril71pl commented 2 years ago

Whether the implementation of this API makes sense is secondary to the fact that the client ignores the contract.

bpasero commented 2 years ago

Looks like a save error is swallowed from here:

https://github.com/microsoft/vscode/blob/c8e2fd7baeaa8cd9dc43d3051db66a7428309c97/src/vs/workbench/services/textfile/common/textFileEditorModel.ts#L711

and

https://github.com/microsoft/vscode/blob/c8e2fd7baeaa8cd9dc43d3051db66a7428309c97/src/vs/workbench/services/textfile/common/textFileEditorModel.ts#L864-L866

Will see if I can return false in case of an error so that saveAll would return false too.

yecril71pl commented 2 years ago

A function that does not return anything returns undefined, which is just fine. There is no need to explicitly return false.

bpasero commented 2 years ago

With https://github.com/microsoft/vscode/commit/42a43ebbd9a15cdf17a7ad577750981829a46490, a call to IEditorService.save or IEditorService.saveAll will now correctly return false if any editor that is saved failed to save.

roblourens commented 2 years ago

I think the original request is that we would wait and start debugging after "Retry as Sudo" is clicked. We don't have a way to wait on this and I wouldn't want to block debugging on a non-modal dialog anyway, so does it make sense to show a message like "Save failed, start debugging anyway?" which would show up at the same time as the "Failed to save" notification.

bpasero commented 2 years ago

Fair, but since this is not a modal dialog but just a notification the save operation at that point has just failed and the editor remains dirty until the user tries again. There is no way currently to join that via any API.

yecril71pl commented 2 years ago

Please read the description once more. There is no need to try again.