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

Support DOTNET_ prefix #50507

Merged

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Mar 31, 2021

Fixes #47283

This enables support for using the DOTNET_ prefix and is now the preferred first choice. The COMPlus_ is still supported but is the fallback.

A new flag was created to ignore the DOTNET_ prefix in case external users have created a conflicting variable name.

  • DOTNET_EnableDOTNETPrefix=0
  • COMPlus_EnableDOTNETPrefix=0

Add support for EnableDOTNETPrefix to disable respecting the DOTNET_ prefix.
Enable environment variable caching on non-Windows platforms.
@jkotas
Copy link
Member

jkotas commented Mar 31, 2021

A new flag was created to ignore the DOTNET_ prefix in case external users have created a conflicting variable name.

I would not bother with this. If we find that there is a conflicting name, we should just fix it.

@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review April 1, 2021 03:52
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@lambdageek
Copy link
Member

Should we reserve the DOTNET_ prefix for variables that are accepted by both runtimes, and continue using COMPlus_ for CoreCLR (and the BCL) and MONO_ for MonoVM?

@AaronRobinsonMSFT
Copy link
Member Author

@lambdageek That is an interesting split. @stephentoub, who filed the issue, thoughts?

@stephentoub
Copy link
Member

stephentoub commented Apr 1, 2021

Should we reserve the DOTNET_ prefix for variables that are accepted by both runtimes, and continue using COMPlus_ for CoreCLR (and the BCL) and MONO_ for MonoVM?

I would love for us to never utter "COMPlus" again 😄

If we feel we need to differentiate some values based on runtime, I could swallow having some be prefixed with CORECLR_ and some with MONO_, and then DOTNET_ for the general "applies everywhere" case. However, I worry such a split might lead to headaches over time, as settings initially only supported by one runtime shift to being supported everywhere (which we would generally hope to be the case as we further unify and smooth over differences). I also think it'd be potentially confusing for folks to learn about multiple prefixes and remember when to use which. And, there are other splits I don't necessarily want to change the prefix for, e.g. settings that only make sense on one OS and that are simply ignored on others.

@lambdageek
Copy link
Member

Yea I think in the long term it will be bad to have multiple flag prefixes as we continue to converge. I don't, for the record, feel that it's a particularly bad idea to shift CoreCLR over to a DOTNET_ prefix.

I'm just apprehensive bug reports like "Setting DOTNET_GC_XYZ doesn't do anything on Android". But on the other hand that's really no different from "COMPlus_GC_XYZ doesn't do anything on Android" - either way we will need to do some work in Mono, or educate the customer, or both.

@AaronRobinsonMSFT
Copy link
Member Author

either way we will need to do some work in Mono, or educate the customer, or both.

@lambdageek This is spot on. I am going to be writing up some official doc on these configs. The past week has been an amazing journey and this system is interesting to say the least. I am hoping the education can be as simple as "go to <URL> and read".

@AaronRobinsonMSFT
Copy link
Member Author

crossgen2smoke failures are #50539

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit fbdcec9 into dotnet:main Apr 1, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the support_dotnet_prefix branch April 1, 2021 17:20
@ghost ghost locked as resolved and limited conversation to collaborators May 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can we rename COMPlus_ to DOTNET_ for environment variables?
4 participants