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.

Sign up for GitHub

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

Jump to bottom

Environment changes are only picked up upon full restart #7239

Closed
vsviridov opened this issue Aug 10, 2020 · 2 comments · Fixed by #7243
Closed

Environment changes are only picked up upon full restart #7239

vsviridov opened this issue Aug 10, 2020 · 2 comments · Fixed by #7243
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@vsviridov
Copy link

Environment

Windows build number: Microsoft Windows NT 10.0.18362.0
Windows Terminal version (if applicable): 1.1.2021.0

Steps to reproduce

  • Launch Windows terminal
  • Modify environment (e.g. via Win+Pause -> Advanced System Settings -> Environment variables)
  • Open a new tab in the existing instance

Expected behavior

  • Should see environment change in the new tab

Actual behavior

  • Environment change only becomes available if the entire Terminal instance is restarted completely
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 10, 2020
DHowett added a commit that referenced this issue Aug 11, 2020
@DHowett
Always create a new environment block before we spawn a process
043ae03 
This commit ensures that we always furnish a new process with the
cleanest, most up-to-date environment variables we can. There is a minor
cost here in that WT will no longer pass environment variables that it
itself inherited to its child processes.

This could be considered a reasonable sacrifice. It will also remove
somebody else's TERM, TERM_PROGRAM and TERM_PROGRAM_VERSION from the
environment, which could be considered a win.

Related to #1125 (but it does not close it or resolve any of the other
issues it calls out.)

Fixes #7239
Fixes #7204 ("App Paths" value creeping into wt's environment)
@DHowett DHowett mentioned this issue Aug 11, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Aug 11, 2020
@DHowett
Copy link
Member

DHowett commented Aug 11, 2020

Thanks for the report! This is a duplicate of #1125, but I decided to just leave it open in PR #7243.

@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Product-Terminal The new Windows Terminal. Priority-2 A description (P2) and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 11, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 11, 2020
@ghost ghost closed this as completed in #7243 Aug 11, 2020
ghost pushed a commit that referenced this issue Aug 11, 2020
@DHowett
Always create a new environment block before we spawn a process (#7243)
849243a 
This commit ensures that we always furnish a new process with the
cleanest, most up-to-date environment variables we can. There is a minor
cost here in that WT will no longer pass environment variables that it
itself inherited to its child processes.

This could be considered a reasonable sacrifice. It will also remove
somebody else's TERM, TERM_PROGRAM and TERM_PROGRAM_VERSION from the
environment, which could be considered a win.

I validated  that GetCurrentProcessToken returns a token we're
_technically able_ to use with this API; it is roughly equivalent to
OpenProcessToken(GetCurrentProcess) in that it returns the current
active _access token_ (which is what CreateEnvironmentBlock wants.)

There's been discussion about doing a 3-way merge between WT's
environment and the new one. This will be complicated and I'd like to
scream test the 0-way merge first ;P

Related to #1125 (but it does not close it or resolve any of the other
issues it calls out.)

Fixes #7239
Fixes #7204 ("App Paths" value creeping into wt's environment)
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Aug 11, 2020
@ghost
Copy link

ghost commented Aug 26, 2020

🎉This issue was addressed in #7243, which has now been successfully released as Windows Terminal Preview v1.3.2382.0.:tada:

Handy links:

@zadjii-msft zadjii-msft mentioned this issue Mar 4, 2021
Closed
6 tasks
@DHowett DHowett mentioned this issue Jun 1, 2023
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Always create a new environment block before we spawn a process microsoft/terminal
3 participants
@DHowett @vsviridov @zadjii-msft