Log CurrentUICulture in binlog.#5426
Conversation
|
|
|
If we are going to have |
|
Honestly I don’t know enough about this to know what’s the difference. Do you have a specific usage in mind where knowing CurrentCulture is required? |
|
Same as yours but what happens if the
|
|
In my opinion we should be logging enough and just enough. Logging needs a purpose. We've been aggressively trying to log less to reduce binlog sizes and cognitive load. I realize it's a small thing, but I'd say let's add this when we need it. For now I need the |
|
So, will you have |
|
I'm sorry, perhaps I'm misunderstanding. I have no plans to use CurrentCulture. I only need CurrentUICulture for my purposes. If you feel like I'm missing your point could you please explain in more detail? What scenario will not work as desired? |
|
What I'm asking is, Will |
|
Yes, if the logged culture is not among those supported by MSBuild I will fall back to en-US, because I assume that's what MSBuild would do. |
|
And yes, the CurrentUICulture updates based on your OS language, and MSBuild uses it to retrieve the resources in that locale: |
|
The That's (mostly) why I've asked for the |
|
I think maybe you're misunderstanding what I'm logging this property for. I don't need to know what CurrentCulture was set to in the MSBuild process. I only need to know what CurrentUICulture was set to, if anything. I'm not interested in any fallbacks. I'm not going to show any UI myself. The only purpose that I need this information for is what language the strings are in the .binlog. They can only be in one of the languages supported by MSBuild and that's what I care about. |
|
I see. Then what about opening the binlogs in systems where the specified UI language resource was not available? MSBuild will return the default language which would be |
|
MSBuild always ships with a fixed set of languages, on all systems. It doesn't matter on which OS the log viewer is running, it will always have all languages supported by MSBuild available, for example for |
|
We have an internal fork which has |
Forgind
left a comment
Forgind
left a comment
There was a problem hiding this comment.
I assume you've tested the behavior if CurrentUICulture isn't defined? I'm fine with defaulting to either en or CurrentCulture, but it shouldn't crash.
|
@Forgind I'm not sure how to test it but according to https://referencesource.microsoft.com/#mscorlib/system/globalization/cultureinfo.cs,811 it can't be null. |
|
I was thinking of trying to set it to a language MSBuild doesn't recognize and seeing what happens, but that's convincing enough for me. |
rainersigwald
left a comment
rainersigwald
left a comment
There was a problem hiding this comment.
LGTM. #5439 should fix the tests; we'll rerun after merging it.
rainersigwald
added
the
merge-when-branch-open
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ghost
added this to the
Second part Part 1 of not checking byte parent d86a1e168bdf295aa777d47ee1a4b988b8913889 author Nathan Mytelka <Forgind@users.noreply.github.com> 1591730709 -0700 committer Nathan Mytelka <Forgind@users.noreply.github.com> 1593461702 -0700 Remove outdated comment Part 2 of unmasking first byte Part 3 Part 4 Part 5 Part 6 Reenabled administrator privilege and cleanup Add test Improve RemoveDependenciesFromEntryIfMissing (dotnet#5392) Fixes dotnet#5180 The fix is a straightforward cache of files that have been detected to exist already, preventing duplicate file system checks on files we already know exist. There will be a future issue and PR addressing ConstructDependencyTable mentioned in a comment on the original issue. Testing on my machine shows an improvement in RemoveDependenciesFromEntryIfMissing of ~850ms down to 30~35ms. Spruce up ObjectModelHelpers assertions These fired while I was writing a new test but didn't have much useful information. Keep AssertItems from throwing an ArgumentOutOfRangeException on mismatched lengths, and give a clue or two about mismatched lengths in AssertItemHasMetadata. Regression tests for dotnet#5445 Ensure that Update and Remove operations done at evaluation time that use item functions pay attention to the item function and don't apply to all items of the same type. Respect item functions in lazy Update/Remove Fixes dotnet#5445 by checking to see if an item function is invoked (the captured expression has subcaptures) before optimizing operations on same-item captures. Rename short-circuit-lazy-item-update check method The question this method answers is 'can I just remove/update every item in the group, or do I need to expand the value to match against existing items?' Renamed it for a bit more clarity there. Log CurrentUICulture in binlog (dotnet#5426) This will be useful to be able to open localized binlogs. If we know the culture of the log we can fetch the right resources from the MSBuild .dlls. Properly parse version Part 7
* First step Second part Part 1 of not checking byte parent d86a1e168bdf295aa777d47ee1a4b988b8913889 author Nathan Mytelka <Forgind@users.noreply.github.com> 1591730709 -0700 committer Nathan Mytelka <Forgind@users.noreply.github.com> 1593461702 -0700 Remove outdated comment Part 2 of unmasking first byte Part 3 Part 4 Part 5 Part 6 Reenabled administrator privilege and cleanup Add test Improve RemoveDependenciesFromEntryIfMissing (#5392) Fixes #5180 The fix is a straightforward cache of files that have been detected to exist already, preventing duplicate file system checks on files we already know exist. There will be a future issue and PR addressing ConstructDependencyTable mentioned in a comment on the original issue. Testing on my machine shows an improvement in RemoveDependenciesFromEntryIfMissing of ~850ms down to 30~35ms. Spruce up ObjectModelHelpers assertions These fired while I was writing a new test but didn't have much useful information. Keep AssertItems from throwing an ArgumentOutOfRangeException on mismatched lengths, and give a clue or two about mismatched lengths in AssertItemHasMetadata. Regression tests for #5445 Ensure that Update and Remove operations done at evaluation time that use item functions pay attention to the item function and don't apply to all items of the same type. Respect item functions in lazy Update/Remove Fixes #5445 by checking to see if an item function is invoked (the captured expression has subcaptures) before optimizing operations on same-item captures. Rename short-circuit-lazy-item-update check method The question this method answers is 'can I just remove/update every item in the group, or do I need to expand the value to match against existing items?' Renamed it for a bit more clarity there. Log CurrentUICulture in binlog (#5426) This will be useful to be able to open localized binlogs. If we know the culture of the log we can fetch the right resources from the MSBuild .dlls. Properly parse version Part 7 * Moved user check * Moved Handshake * Fixed build * Refactoring * Move fire byte calculation into loop * Save before committing 😃 * Catch uncaught exception * PR feedback * Correct off-by-one error
* First step Second part Part 1 of not checking byte parent d86a1e168bdf295aa777d47ee1a4b988b8913889 author Nathan Mytelka <Forgind@users.noreply.github.com> 1591730709 -0700 committer Nathan Mytelka <Forgind@users.noreply.github.com> 1593461702 -0700 Remove outdated comment Part 2 of unmasking first byte Part 3 Part 4 Part 5 Part 6 Reenabled administrator privilege and cleanup Add test Improve RemoveDependenciesFromEntryIfMissing (#5392) Fixes #5180 The fix is a straightforward cache of files that have been detected to exist already, preventing duplicate file system checks on files we already know exist. There will be a future issue and PR addressing ConstructDependencyTable mentioned in a comment on the original issue. Testing on my machine shows an improvement in RemoveDependenciesFromEntryIfMissing of ~850ms down to 30~35ms. Spruce up ObjectModelHelpers assertions These fired while I was writing a new test but didn't have much useful information. Keep AssertItems from throwing an ArgumentOutOfRangeException on mismatched lengths, and give a clue or two about mismatched lengths in AssertItemHasMetadata. Regression tests for #5445 Ensure that Update and Remove operations done at evaluation time that use item functions pay attention to the item function and don't apply to all items of the same type. Respect item functions in lazy Update/Remove Fixes #5445 by checking to see if an item function is invoked (the captured expression has subcaptures) before optimizing operations on same-item captures. Rename short-circuit-lazy-item-update check method The question this method answers is 'can I just remove/update every item in the group, or do I need to expand the value to match against existing items?' Renamed it for a bit more clarity there. Log CurrentUICulture in binlog (#5426) This will be useful to be able to open localized binlogs. If we know the culture of the log we can fetch the right resources from the MSBuild .dlls. Properly parse version Part 7 * Moved user check * Moved Handshake * Fixed build * Refactoring * Move fire byte calculation into loop * Save before committing 😃 * Catch uncaught exception * PR feedback * Correct off-by-one error * Make handshake version explicit * PR comments
4 participants
This will be useful to be able to open localized binlogs. If we know the culture of the log we can fetch the right resources from the MSBuild .dlls.