msbuild icon indicating copy to clipboard operation
msbuild copied to clipboard

Exempt our env var Fixes #7922

Open Forgind opened this issue 3 years ago • 3 comments

Fixes #7922

Context

Although it's good to hide environment variables that may contain sensitive information, some variables seldom do and can be quite helpful in diagnosing build issues. This exempts environment variables that start with MSBUILD, COMPLUS_, and DOTNET_ from the normal filtering, even in the absence of MSBUILDLOGALLENVIRONMENTVARIABLES.

Changes Made

Exempted certain environment variables. Also prevented any EnvironmentVariableReadEvents from firing if MSBUILDLOGALLENVIRONMENTVARIABLES is set.

Testing

I built MSBuild.Dev.slnf and checked what its environment node contained.

Notes

This will need a structured log viewer update to make it less confusing.

~Confusing to me, this only almost works. Specifically, it filtered out all environment variables except the intended ones plus OS, LOCALAPPDATA, and USERPROFILE. I don't know what made those special.~

Forgind avatar Aug 31 '22 17:08 Forgind

plus OS, LOCALAPPDATA, and USERPROFILE. I don't know what made those special.

Naively, aren't those properties almost always used in msbuild evaluation❔ For example, UserProfile is normally evaluated to determine LocalAppData, that's normally evaluated to determine $(MSBuildUserExtensionsPath), and that's evaluated to look for default files to import.

dougbu avatar Aug 31 '22 17:08 dougbu

plus OS, LOCALAPPDATA, and USERPROFILE. I don't know what made those special.

Naively, aren't those properties almost always used in msbuild evaluation❔ For example, UserProfile is normally evaluated to determine LocalAppData, that's normally evaluated to determine $(MSBuildUserExtensionsPath), and that's evaluated to look for default files to import.

They are, which is highly suspicious. I'm confused because I wouldn't have expected EnvironmentVariableReadEvents to show up in ProjectStartedEventArgs, since nothing had started by that point...but it also doesn't seem like a big deal if they do, so maybe we shouldn't care?

Forgind avatar Aug 31 '22 18:08 Forgind

I think you're right; this was just the viewer displaying things differently than I'd expected. I tried grabbing a diagnostic text log, and it looks good. I also tried replaying a binlog into a text log, and that looks good, too.

Forgind avatar Sep 01 '22 17:09 Forgind