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

ProjInfo sets environment variables for the current process. #191

Open
safesparrow opened this issue Jun 21, 2023 · 2 comments
Open

ProjInfo sets environment variables for the current process. #191

safesparrow opened this issue Jun 21, 2023 · 2 comments

Comments

@safesparrow
Copy link
Contributor

safesparrow commented Jun 21, 2023

Here environment variables are set for the current process.

This means that spawning an unrelated child process after using ProjInfo is affected by those.
In my case, I have to reset them after, otherwise child dotnet processes that I run myself don't work as expected.

Ideally ProjInfo would not change current process's environment, and if it does, it should at least revert any changes it makes (which doesn't help if other things are using the Environment in parallel, but is better than the current state).

If the environment is only set for the benefit of child processes, could ProcessStartInfo be utilised to set them for child processes only?

@baronfel
Copy link
Collaborator

Instead of environment variables, we could return a collection of global properties, but then callers would be forced to use those global properties at all times or else something might be incorrect. The SDK and MSBuild tooling rely on the values of those properties being set.

@safesparrow
Copy link
Contributor Author

safesparrow commented Jun 21, 2023

It's fine to use environment variables, but instead of changing the current environment, they should only be applied to child processes. Otherwise the user is forced to figure out what they need to undo manually, which is not user-friendly.

One way to do this would be for functions like Init.init to return toolsPath * EnvironmentChanges, and things like WorkspaceLoader.Create accept such a tuple.

A backwards-compatible way would be for these functions to have overloads that do that on top of current versions.

Personally I don't depend on ProjInfo setting those variables - I don't need them as long as ProjInfo can set them wherever it needs - and in my case the only top-level API I use is WorkspaceLoader.Create.

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

2 participants