-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Make dotnet --info print all DOTNET_* environment variables #117797
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR enhances the dotnet --info
command to display all environment variables with DOTNET_
or COREHOST_
prefixes, rather than only showing DOTNET_ROOT
related variables. The implementation adds cross-platform environment variable enumeration functionality and updates the output formatting to accommodate longer variable names.
Key changes:
- Adds platform-specific environment variable enumeration functions
- Updates environment variable filtering and display logic to include all DOTNET_* and COREHOST_* variables
- Replaces existing targeted test with comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/native/corehost/hostmisc/pal.h | Adds function declaration for enumerate_environment_variables with functional header include |
src/native/corehost/hostmisc/pal.windows.cpp | Implements Windows-specific environment variable enumeration using GetEnvironmentStringsW |
src/native/corehost/hostmisc/pal.unix.cpp | Implements Unix-specific environment variable enumeration using extern environ |
src/native/corehost/fxr/install_info.cpp | Refactors environment variable display logic to enumerate, filter, sort, and display all DOTNET_/COREHOST_ variables |
src/installer/tests/HostActivation.Tests/InstallLocation.cs | Removes old test that only tested DOTNET_ROOT variables |
src/installer/tests/HostActivation.Tests/HostCommands.cs | Adds comprehensive test for new environment variable enumeration functionality |
@@ -957,6 +957,23 @@ bool pal::getenv(const pal::char_t* name, pal::string_t* recv) | |||
return (recv->length() > 0); | |||
} | |||
|
|||
extern char **environ; |
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.
The extern declaration should be moved to the top of the file with other includes, or use the standard #include <unistd.h> which declares environ on most systems.
extern char **environ; |
Copilot uses AI. Check for mistakes.
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov |
Should it show COMPlus_ environment variables? Should the prefix match be case-insensitive on Windows? |
The COMPlus names are legacy that we’d like to phase out. I don’t think we should add support for them in new features. |
Can we somehow just show what is used instead of just showing all environment variables a simple shell script could do? I would guess the answer is no, as multiple layers look at different env vars... |
Do you mean which were honored at runtime? That seems more like the responsibility of a tracing system. Do you have a specific use case? |
Co-authored-by: Aaron Robinson <[email protected]>
In the same vein, introducing a |
Good idea @am11: #117797 (comment) Yes, let's make this feature specific to As Andy suggests, this make it clear that they are our only supported ENVs. It's our public namespace. LLMs are going to make the assumption that |
There are a number of variables that begin with
I don't see how this makes it clear. What is clear that the |
My take is that teams can move to use |
The rationale behind introducing the |
Supporting DOTNET variations of the ICorProfiler env vars seems like a good thing to do at some point. I filed #117902 to track it. |
dotnet --info
currently only printsDOTNET_ROOT
/DOTNET_ROOT_<ARCH>
if they are set. Expand it to all environment variables with aDOTNET_
orCOREHOST_
prefix.Example:
Resolves #117732
cc @dotnet/appmodel @AaronRobinsonMSFT @richlander