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

Configuration Resolver: figure out process.env behaviour #72075

Closed
bpasero opened this issue Apr 10, 2019 · 5 comments
Closed

Configuration Resolver: figure out process.env behaviour #72075

bpasero opened this issue Apr 10, 2019 · 5 comments
Assignees
Labels
debt Code quality issues

Comments

@bpasero
Copy link
Member

bpasero commented Apr 10, 2019

It looks like #72029 was triggered by using IWindowConfiguration.userEnv instead of process.env which I did accidentally by my refactorings to bring the configuration resolver service into browser namespace.

@jrieken @joaomoreno can you remind me again when to use process.env vs process.lazyEnv? Should process.env never be used?

My theory is that the resolver service was accessing IWindowConfiguration.userEnv early enough before the process environment was fully resolved.

//cc @alexr00

@bpasero bpasero added the debt Code quality issues label Apr 10, 2019
bpasero added a commit that referenced this issue Apr 10, 2019
@jrieken
Copy link
Member

jrieken commented Apr 10, 2019

Yes, always use process.lazyEnv

@jrieken
Copy link
Member

jrieken commented Apr 10, 2019

For more detail, until process.lazyEnv isn't resolved process.env isn't complete, e.g on mac/linux we are still resolving bashrc and friends

@bpasero
Copy link
Member Author

bpasero commented Apr 10, 2019

It also seems that process.env has more values initially compared to IWindowConfiguration.userEnv, I wonder if process.env always does a roundtrip to the main side to get most up to date values 🤔 ?

@bpasero
Copy link
Member Author

bpasero commented Apr 13, 2019

Ok, this is unrelated to process.env vs process.lazyEnv because the code that gets executed is within a constructor and not top level. I now separated src/vs/workbench/services/configurationResolver/electron-browser/configurationResolverService.ts and src/vs/workbench/services/configurationResolver/browser/configurationResolverService.ts where the one is still using process.env and the other is using IWindowConfiguration.userEnv.

@bpasero bpasero closed this as completed Apr 13, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues
Projects
None yet
Development

No branches or pull requests

3 participants