Skip to content

fix(record): propagate env of vhs process to record terminal - #572

Merged
caarlos0 merged 1 commit into
charmbracelet:mainfrom
tr3mor:main
Mar 10, 2025
Merged

caarlos0 merged 1 commit into
charmbracelet:mainfrom
tr3mor:main

Conversation

@tr3mor

@tr3mor tr3mor commented Jan 16, 2025 •

Copy link
Copy Markdown
Contributor

Hello,
This pr broke env variables propagation to terminal spawned by record command.
From Env description

	// If Env is nil, the new process uses the current process's
	// environment.

Appending VHS_RECORD makes it not nil, so it resets env. This pr fixes #554.

For example with the latest released vhs

root# echo $HOME
/var/root
root# vhs record
sh-3.2# echo $HOME
sh-3.2# env
VHS_RECORD=true
PWD=/private/var/root
SHLVL=1
_=/usr/bin/env

The simplest solution is to get env of the main process and then append VHS_RECORD
The same example with the build from this PR

root# echo $HOME
/var/root
root# ./vhs record
sh-3.2# echo $HOME
/var/root

sh-3.2# env
TERM=xterm-ghostty
SHELL=/bin/sh
LC_ALL=en_US.UTF-8
USER=root
SUDO_UID=503
SSH_AUTH_SOCK=/private/tmp/com.apple.launchd.4WqxVicPZH/Listeners
__CF_USER_TEXT_ENCODING=0x0:0:0
PATH=/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Users/REDACTED/go/bin:/Users/REDACTED/.local/share/zinit/polaris/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/Applications/Ghostty.app/Contents/MacOS:/Users/REDACTED/Library/Application Support/JetBrains/Toolbox/scripts
MAIL=/var/mail/root
VHS_RECORD=true
PWD=/Users/REDACTED/personal/vhs
LANG=en_US.UTF-8
HOME=/var/root
SUDO_COMMAND=/bin/sh
SHLVL=2
LOGNAME=root
SUDO_GID=20
COLORTERM=truecol

Let me know if you see a better solution.

@tr3mor
tr3mor requested a review from a team as a code owner January 16, 2025 16:08
@tr3mor
tr3mor requested review from csandeep and removed request for a team January 16, 2025 16:08
@tr3mor

tr3mor commented Jan 16, 2025

Copy link
Copy Markdown
Contributor Author

It looks like lint issues are unrelated to my change, so I don't think I need to fix them.

@bashbunni

Copy link
Copy Markdown
Contributor

@tr3mor agreed, will take a look at this. Thanks so much for spotting this issue and coming up with a solution

@fenio

fenio commented Mar 10, 2025

Copy link
Copy Markdown

Can we get this merged? Currently vhs is hardly usable without env inheritance and requires extra steps to init even simple PATH variable.

@caarlos0

Copy link
Copy Markdown
Contributor

I think this makes sense, yes.

@caarlos0
caarlos0 merged commit b461e7b into charmbracelet:main Mar 10, 2025
@caarlos0 caarlos0 added the bug Something isn't working label Mar 10, 2025
@dwmkerr

dwmkerr commented Mar 14, 2025

Copy link
Copy Markdown

Any idea when this version will be released with the fix? Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vhs record unsets $HOME

5 participants

⌂ 换网址