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

Environment variables are eaten by zypak-helper when ELECTRON_RUN_AS_NODE=1 #19

Closed
catsout opened this issue Dec 8, 2021 · 9 comments
Closed

Comments

@catsout
Copy link

catsout commented Dec 8, 2021

The problem appears when run electron with ELECTRON_RUN_AS_NODE=1
It seems that it will start node cli first without using of chrome-sandbox, and then spawn(vscode) it self to run with chrome

If I use zypak-wrapper at first with CHROME_WRAPPER, that spawning will be overridden to zypak-helper exec-latest $CHROME_WRAPPER ...
Here zypak-helper exec-latest will spawn a new sandbox environment(I'm not sure, as I don't known how SpawnLatest works)
As a result, the environment variables will not be inherited, which will cause some problems(linked below), as the first run_as_node may set some environment variables used by electron
flathub/com.visualstudio.code#261
flathub/com.visualstudio.code#259
flathub/com.visualstudio.code#237

I think maybe do execvp without zypak-helper exec-latest when ZYPAK_DISABLE_SANDBOX=1 with CHROME_WRAPPER to solve this problem
Need unset ZYPAK_DISABLE_SANDBOX in CHROME_WRAPPER script or in the overridden execvp

@refi64
Copy link
Owner

refi64 commented Dec 11, 2021

For some context: the whole reason SpawnLatest needs to exist is for stuff like chrome://restart to work. It's expected that chrome://restart would re-open Chromium to the latest version installed, meaning we need to ask Flatpak to start the application in a new sandbox containing the latest Chromium...but when it restarts it, the environment is now cleared out.

In theory, Zypak should just be saving the original environment to more closely follow the original behavior, then this would work (I think). I'll just need to fiddle with it a bit to make sure it doesn't break anything.

@gasinvein
Copy link

From what I can tell, the only issue with vscode and zypak which can't be worked around is that --wait doesn't work. Is it somehow related to the fact that zypak looses environment variables?

@catsout
Copy link
Author

catsout commented Feb 3, 2022

Is it somehow related to the fact that zypak looses environment variables?

https://github.com/refi64/zypak/blob/main/src/preload/host/exec_zypak_sandbox.cc#L44
https://github.com/refi64/zypak/blob/main/src/helper/spawn_latest.cc#L25

execvp is preloaded, as long as the program want to restart itself, a new spwan env will be involved, which will loss the environment variables.

When ELECTRON_RUN_AS_NODE=1 specified, the starting progress like:
zypak electron --> run as node and set some env and unset ELECTRON_RUN_AS_NODE --> spwan self with zypak preload execvp --> env lost, run as electron --> app started

The lost env causes that --wait doesn't work

@gasinvein
Copy link

Is there some particular env var responsible for --wait? It seems like the lock file created for wait is passed on cli.

@catsout
Copy link
Author

catsout commented Feb 3, 2022

I got it wrong, It's not about the environment variables on --wait.
The point is that zypak-helper host-latest|exec-latest will not wait for the processes started by it.

From the code

// When running with --wait, we want to continue running CLI process
// until either:
// - the wait marker file has been deleted (e.g. when closing the editor)
// - the launched process terminates (e.g. due to a crash)

@gasinvein
Copy link

Seems like a distinct issue, then.

@refi64 refi64 closed this as completed in 8424c6b Mar 7, 2022
@refi64
Copy link
Owner

refi64 commented Mar 7, 2022

Please try the main branch with ZYPAK_SPAWN_LATEST_ON_REEXEC=0.

@bytzo
Copy link

bytzo commented Mar 7, 2022

Using ZYPAK_SPAWN_LATEST_ON_REEXEC=0 with 8424c6b seems to fix the issues in the VSCode Flatpak. Additionally, the --wait flag seems to function properly again.

@gasinvein
Copy link

Yep, seems to work for Code-OSS as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants