feat: bundle sparse package to integrate with windows context menu #151186

deepak1556 · 2022-06-03T02:14:39Z

ESRP signing
Inno install/uninstall
Update Flow

deepak1556 · 2022-06-27T10:44:47Z

build/win32/code.iss

@@ -34,6 +34,7 @@ ShowLanguageDialog=auto
 ArchitecturesAllowed={#ArchitecturesAllowed}
 ArchitecturesInstallIn64BitMode={#ArchitecturesInstallIn64BitMode}
 WizardStyle=modern
+SetupLogging=yes


For setup debugging purpose, will be removed before merging the PR

build/win32/code.iss

deepak1556 · 2022-07-04T17:42:59Z

Status:

Highlighting some interesting corner cases when creating sparse packages with Chromium runtime,

Problem 1 - Fixed Installing the package with ExternalLocation set to the root of the VSCode installation folder leads to a crash on startup when application is installed under %LOCALAPPDATA/Programs (User setup)
- Problem Child process that run with sandbox gets aborted by the runtime as soon as the sandbox phase was initialized after the process launch, currently in VSCode case it would be GPU process and some workbench processes (Process Explorer, DevTools, Issue Reporter, OOPIF). Application launches with Medium IL but the sandbox will change IL to Low IL (GPU) and Untrusted IL (Renderer). Turns out the Appx installation adds two new SID (S-1-15-2-xxx and S-1-15-3-xxx) to the folder marked as ExternalLocation and changes the IL of the folder https://devblogs.microsoft.com/oldnewthing/20220502-00/?p=106550. Based on the exit code of the crashing process, it seemed that process were unable to load DLLs. Both GPU and Renderer load certain DLLs present in the VSCode installation folder as part of the initialization phase and the lower IL of these processes causes the abort.
- Solution Install the sparse package resources from a child folder of the VSCode Installation path so that the root folder permissions remain unchanged.
Problem 2 - Fixed User setup fails to persist data to the default user data path %APPDATA% when launched via the context menu
- Problem Capability unvirtualizedResources added to the package enables filesystem virtualization by default https://docs.microsoft.com/en-us/windows/uwp/packaging/app-capability-declarations
- Solution Disable FileSystemWriteVirtualization
Problem 3 - Fixed Localization of context menu title using a resources file does not work out of box https://docs.microsoft.com/en-us/windows/uwp/app-resources/localize-strings-ui-manifest#loading-strings-in-non-packaged-applications
- Problem ResourceLoader::GetForViewIndependentUse does not pick up the resources file when called from within the sparse DLL. Most Electron apps, don't have control over the runtime code and it is not possible to integrate the resource file for the main application.
- Workaround Read from the localized strings in the registry that were setup as part of the old context menu.
Problem 4 - Pending Integrate sparse package with the Inno updater flow
- Problem During update inno will wait for the application to exit gracefully then replace the files of the installation folder and trigger a restart. Context menu integration loads a DLL as part of the windows explorer which needs to get unloaded (via Remove-AppxPackage cmdlet) before the files are replaced by inno. This requires we make the updater code aware of the sparse package details which is not ideal as we have been keeping the updater isolated from the application logic https://github.com/Microsoft/inno-updater.

deepak1556 · 2022-07-07T16:00:01Z

@joaomoreno Selfhosting build available at

deepak1556 · 2022-07-18T16:44:41Z

Next Steps to land this for preview with Insiders:

Make inno-updater skip appx package folder during update chore: skip sparse package during update inno-updater#17
Separate the context menu into new tasks and isolate them for Insiders flavor
Highlight reinstallation to udpate the appx package in release notes.

deepak1556 · 2022-07-21T11:15:34Z

New insiders build : https://monacotools.visualstudio.com/DefaultCollection/Monaco/_build/results?buildId=178917&view=results

joaomoreno · 2022-07-21T12:35:14Z

New one with updated inno-updater: https://monacotools.visualstudio.com/DefaultCollection/Monaco/_build/results?buildId=178931&view=results

joaomoreno · 2022-07-21T14:07:53Z

@deepak1556 Getting this when running the setup, possibly related to https://github.com/microsoft/vscode/pull/151186/files#diff-8dd373ae4b3c58948172ac6408e9e57ff2451265eca1b3b3c27682b79869bfceR1387

deepak1556 · 2022-07-21T14:29:09Z

Thanks for checking! That condition should not be an issue since it worked in previous builds, I think it is from this newly added condition https://github.com/microsoft/vscode/pull/151186/files#diff-8dd373ae4b3c58948172ac6408e9e57ff2451265eca1b3b3c27682b79869bfceR1416.

deepak1556 · 2022-10-20T07:43:06Z

New Build: https://monacotools.visualstudio.com/Monaco/_build/results?buildId=189784&view=results

Addresses the issues outlined in #151186 (comment)

joaomoreno · 2022-10-20T12:38:33Z

A few more issues:

Garbled menu entry in system setup

Install system setup

🐛 The menu item is garbled

Context menu entry not removed

Install VS Code with context menu enabled
Without uninstalling, reinstall VS Code and this time disable the context menu

🐛 The context menu is still there.

Double menu entry in classic menu after update

I'm fine if we don't fix this, given that it only surfaces in the classic menu and only applies to updates. A fresh installation doesn't hit this issue.

Install previous VS Code version, enable context menu
Use the Developer: Apply Update to update to the new version

🐛 The classic menu will contain both action items

No menu entry in arm64

Install VS Code with context menu enabled, in Windows for ARM64

🐛 No menu entry is present

The AppX seems registered though:

- Remove old context menu entries when updating to newer version - Remove context menu entry when reinstalling without menu action selected.

- Fixes garbled title for system installation

deepak1556 · 2022-10-21T07:22:47Z

New Build: https://monacotools.visualstudio.com/Monaco/_build/results?buildId=189968&view=results

Addresses the following issues from #151186 (comment)

Garbled menu entry in system setup
Context menu entry not removed
Double menu entry in classic menu after update

I haven't setup Windows on arm on my new device yet, will verify the last pending issue after that.

joaomoreno · 2022-10-21T12:35:22Z

Getting there!

More findings:

Error when reinstalling

Install VS Code with context menu entry
Without uninstalling, reinstall VS Code

🐛 There's an error about COM Surrogate:

Choosing Automatically close the applications doesn't work
Choosing Do not close the applications seems to work actually!

Rolling back doesn't remove context menu entry

Install VS code with context menu entry
Use the Developer: Apply Update to update to an old version

🐛 Context menu entry is still there

deepak1556 · 2022-10-21T15:30:07Z

DLLHost.exe aka COM surrogate holds the DLL after context menu is closed for a certain short period of time, I think this has to do with how long the Explorer references the COM object but from my testing it is not an unacceptable long period. But during this period if we perform the update step then we will run into the above error. This is an edge case we cannot solve from our DLL code or the updater. I think this case would be rarely hit by an user, I plan to ignore this issue for now and revisit based on insiders testing.

As for the rollback scenario, we don't plan to support it since it requires changes in our updater to know about the appx package. I am going to merge this PR for insiders testing and follow-up on the arm64 issue.

@joaomoreno thanks a lot for the extensive testing! ❤️

build/azure-pipelines/win32/product-build-win32.yml

build/win32/code.iss

deepak1556 · 2022-10-24T02:50:23Z

@joaomoreno addressed in #164411

deepak1556 marked this pull request as draft June 3, 2022 02:14

deepak1556 added this to the June 2022 milestone Jun 3, 2022

vscodenpa assigned deepak1556 Jun 3, 2022

deepak1556 force-pushed the robo/explorer_sparse_bundle branch from 25b09fa to 3dd2962 Compare June 8, 2022 05:45

deepak1556 force-pushed the robo/explorer_sparse_bundle branch 6 times, most recently from 14ae623 to ed85e85 Compare June 27, 2022 10:40

deepak1556 marked this pull request as ready for review June 27, 2022 10:40

deepak1556 requested a review from joaomoreno June 27, 2022 10:42

deepak1556 commented Jun 27, 2022

View reviewed changes

build/win32/code.iss Outdated Show resolved Hide resolved

deepak1556 commented Jun 27, 2022

View reviewed changes

build/win32/code.iss Outdated Show resolved Hide resolved

deepak1556 commented Jun 27, 2022

View reviewed changes

build/win32/code.iss Show resolved Hide resolved

deepak1556 force-pushed the robo/explorer_sparse_bundle branch from ed85e85 to c89f455 Compare June 27, 2022 12:33

deepak1556 removed this from the June 2022 milestone Jun 30, 2022

deepak1556 force-pushed the robo/explorer_sparse_bundle branch 2 times, most recently from 7aaa945 to d48f65c Compare July 7, 2022 13:29

deepak1556 mentioned this pull request Jul 21, 2022

chore: skip sparse package during update microsoft/inno-updater#17

Merged

deepak1556 force-pushed the robo/explorer_sparse_bundle branch from 3977565 to c2e1cce Compare July 21, 2022 11:08

joaomoreno added this to the July 2022 milestone Jul 21, 2022

deepak1556 and others added 11 commits October 20, 2022 15:09

chore: update vscode-explorer-command@3.0.3

0fde707

fix: guard behind insider quality

6547144

chore: fix merge conflict

b2fdb9a

fix: type mismatch

36f61eb

chore: restrict context menu related actions for win11

3de7211

refactor appx loading/unloading, revert inno updater

d25f42a

update powershell

533be4c

chore: undo inno_updater changes

692530c

fix: appx install script

7098c74

fix: perform install after update step

5ce1ea2

fix: mv appx install to files section

e09f84b

deepak1556 force-pushed the robo/explorer_sparse_bundle branch from c958abe to e09f84b Compare October 20, 2022 07:41

deepak1556 mentioned this pull request Oct 21, 2022

fix: reg path for system installation microsoft/vscode-explorer-command#15

Merged

deepak1556 added 2 commits October 21, 2022 15:35

chore: additional fixes

11a4a05

- Remove old context menu entries when updating to newer version - Remove context menu entry when reinstalling without menu action selected.

chore: update vscode-explorer-command@3.0.4

e980559

- Fixes garbled title for system installation

deepak1556 enabled auto-merge (squash) October 21, 2022 15:30

joaomoreno approved these changes Oct 21, 2022

View reviewed changes

deepak1556 merged commit ebea50a into main Oct 21, 2022

deepak1556 deleted the robo/explorer_sparse_bundle branch October 21, 2022 18:17

joaomoreno reviewed Oct 22, 2022

View reviewed changes

build/azure-pipelines/win32/product-build-win32.yml Show resolved Hide resolved

build/win32/code.iss Show resolved Hide resolved

build/win32/code.iss Show resolved Hide resolved

deepak1556 mentioned this pull request Oct 24, 2022

chore: cleanup inno setup for context menu install #164411

Merged

joaomoreno mentioned this pull request Oct 24, 2022

Windows: Context menu doesn't work in ARM64 #164464

Open

joaomoreno mentioned this pull request Nov 1, 2022

Can't build Windows stable #165142

Closed

github-actions bot locked and limited conversation to collaborators Dec 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: bundle sparse package to integrate with windows context menu #151186

feat: bundle sparse package to integrate with windows context menu #151186

deepak1556 commented Jun 3, 2022 •

edited

Loading

deepak1556 Jun 27, 2022

deepak1556 commented Jul 4, 2022 •

edited by isidorn

Loading

deepak1556 commented Jul 7, 2022

deepak1556 commented Jul 18, 2022 •

edited

Loading

deepak1556 commented Jul 21, 2022

joaomoreno commented Jul 21, 2022

joaomoreno commented Jul 21, 2022

deepak1556 commented Jul 21, 2022

deepak1556 commented Oct 20, 2022

joaomoreno commented Oct 20, 2022

deepak1556 commented Oct 21, 2022

joaomoreno commented Oct 21, 2022 •

edited

Loading

deepak1556 commented Oct 21, 2022

deepak1556 commented Oct 24, 2022

feat: bundle sparse package to integrate with windows context menu #151186

feat: bundle sparse package to integrate with windows context menu #151186

Conversation

deepak1556 commented Jun 3, 2022 • edited Loading

deepak1556 Jun 27, 2022

Choose a reason for hiding this comment

deepak1556 commented Jul 4, 2022 • edited by isidorn Loading

deepak1556 commented Jul 7, 2022

deepak1556 commented Jul 18, 2022 • edited Loading

deepak1556 commented Jul 21, 2022

joaomoreno commented Jul 21, 2022

joaomoreno commented Jul 21, 2022

deepak1556 commented Jul 21, 2022

deepak1556 commented Oct 20, 2022

joaomoreno commented Oct 20, 2022

Garbled menu entry in system setup

Context menu entry not removed

Double menu entry in classic menu after update

No menu entry in arm64

deepak1556 commented Oct 21, 2022

joaomoreno commented Oct 21, 2022 • edited Loading

Error when reinstalling

Rolling back doesn't remove context menu entry

deepak1556 commented Oct 21, 2022

deepak1556 commented Oct 24, 2022

deepak1556 commented Jun 3, 2022 •

edited

Loading

deepak1556 commented Jul 4, 2022 •

edited by isidorn

Loading

deepak1556 commented Jul 18, 2022 •

edited

Loading

joaomoreno commented Oct 21, 2022 •

edited

Loading