-
Notifications
You must be signed in to change notification settings - Fork 316
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
fix: Enviroment Variables are not correctly inherited from system on … #6164
Conversation
…MacOS Use CONSOLE ParentEnvironmentType instead of SYSTEM CONSOLE is the default value initialized in GeneralCommandLineso it was used before. Now, if we want to inherit the system envs we use SYSTEM, so in both cases it differs from what was there before. This PR just brings the old behavior in case a user wants to inherit enviroment from the system. https://github.com/JetBrains/intellij-community/blob/9838665ba816fc8ce06e4071aa5a906a35fbb22e/platform/platform-util-io/src/com/intellij/execution/configurations/GeneralCommandLine.java#L85C1-L86C1 fixes bazelbuild#6163
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the swift change!
Oh, does it mean the ParentEnvironmentType.SYSTEM was not intentional? That was my only concern |
ok if so, I think we can merge and publish a hotfix @mai93 |
I would like it to be SYSTEM if the user ticks the "inherit" box, but I agree that right now there's a bug and the this gets people unblocked. I'm happy to iterate on making it work properly later. |
Ideally, we'd toggle between SYSTEM and CONSOLE, depending on whether the toggle was clicked or not. |
Let's merge and hot fix, I'll iterate in the morning |
…MacOS (#6164) Use CONSOLE ParentEnvironmentType instead of SYSTEM CONSOLE is the default value initialized in GeneralCommandLineso it was used before. Now, if we want to inherit the system envs we use SYSTEM, so in both cases it differs from what was there before. This PR just brings the old behavior in case a user wants to inherit enviroment from the system. https://github.com/JetBrains/intellij-community/blob/9838665ba816fc8ce06e4071aa5a906a35fbb22e/platform/platform-util-io/src/com/intellij/execution/configurations/GeneralCommandLine.java#L85C1-L86C1 fixes #6163
…MacOS (#6164) Use CONSOLE ParentEnvironmentType instead of SYSTEM CONSOLE is the default value initialized in GeneralCommandLineso it was used before. Now, if we want to inherit the system envs we use SYSTEM, so in both cases it differs from what was there before. This PR just brings the old behavior in case a user wants to inherit enviroment from the system. https://github.com/JetBrains/intellij-community/blob/9838665ba816fc8ce06e4071aa5a906a35fbb22e/platform/platform-util-io/src/com/intellij/execution/configurations/GeneralCommandLine.java#L85C1-L86C1 fixes #6163
Sorry, folks, just a question from an outsider here: The issue that was addressed in this PR has made the plugin completely unusable for many developers in my company. Have you considered covering it with some form of tests to ensure it's not reintroduced in the future? |
Yep definitely we should :( |
…MacOS
Use CONSOLE ParentEnvironmentType instead of SYSTEM
CONSOLE is the default value initialized in GeneralCommandLineso it was used before. Now, if we want to inherit the system envs we use SYSTEM, so in both cases it differs from what was there before. This PR just brings the old behavior in case a user wants to inherit enviroment from the system.
https://github.com/JetBrains/intellij-community/blob/9838665ba816fc8ce06e4071aa5a906a35fbb22e/platform/platform-util-io/src/com/intellij/execution/configurations/GeneralCommandLine.java#L85C1-L86C1
fixes #6163
Checklist
Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.
Discussion thread for this change
Issue number:
<please reference the issue number or url here>
Description of this change