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

Don't set HOME environment variable #1923

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Conversation

nagilson
Copy link
Member

@nagilson nagilson commented Aug 16, 2024

On Activation our Extension sets the HOME variable causing Git to no longer function.

Resolves #1795
Resolves #1400
Resolves #1686
Resolves #1707
Resolves #1916

This import statement in our code on WebRequestWorker.ts sets HOME.

const get_proxy_settings_1 = require("get-proxy-settings");

How do I know? I used the SysInternals Process Explorer to read the environment for every vscode sub process and went through the decompiled JS line by line in a binary search to see which line of code causes HOME to change 🥱

image

HOME is no longer changed.

This is definitely one of the weirdest bugs Ive had to solve.... seriously. We checked the source code this library and they dont edit HOME either, probably their 1 of dependencies does. But it doesn't really matter.

@nagilson nagilson requested review from a team and smitpatel August 16, 2024 19:56
@nagilson nagilson enabled auto-merge (squash) August 16, 2024 20:02
Co-authored-by: Smit Patel <smitpatel@users.noreply.github.com>
@nagilson nagilson merged commit 524b14e into dotnet:main Aug 16, 2024
7 checks passed
@pbl-pw
Copy link

pbl-pw commented Aug 17, 2024

I searched the node_modules directory and found that npm-conf was writing the HOME environment variable.

@dpalmetzvbg
Copy link

Thanks for investing your time and fixing it. Seems it was a difficult one... :-)

@nagilson
Copy link
Member Author

nagilson commented Aug 19, 2024

Thanks for reporting it :) and @pbl-pw for helping as well with the detailed report. It was more confusing/weird and time-consuming than it was difficult... I would not expect a library import to do this 🙄 '
I will probably write a bug report over on their side

@nagilson
Copy link
Member Author

There has been an issue on their repo since 2018 kevva/npm-conf#13 💀 Yes it is npm-conf

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