[AOT] Refactor Logger function and mark managedCommon as AOT compatible #36327
Conversation
moooyo
added
the
Needs-Review
|
When we publish application with AOT enable. Some reflection usage will be forbidden. So, we need to make this change. |
|
old log format: [05:29:23.8888162] [Info] Program::Main |
crutkas
added
In for .88
Don't merge - Hold for release
src/common/ManagedCommon/Logger.cs
Outdated
| throw; | ||
| #endif | ||
| } | ||
| string fileName = Path.GetFileName(sourceFilePath); |
There was a problem hiding this comment.
May be Path.GetFileNameWithoutExtension?
Looks like this will throw exception as well, although I don't see why. Just to be safe in case.
There was a problem hiding this comment.
I believe with this way, the original strange code for "async method" should be not only gone, but much more reliable, we can put as part of benefit for this change after testing.
| } | ||
|
|
||
| private static MethodBase GetCallerMethod(StackTrace stackTrace) | ||
| private static string GetCallerInfo(string memberName, string sourceFilePath, int sourceLineNumber) |
There was a problem hiding this comment.
For sourceLineNumber.
Given originally they don't have, we need to ask for PMs, that any parser of the log it will break this.
If Yes, we should still keep the original "TypeName::MethodName" format first.
Last but not least, do we know how to view PowerToys log? If we can parrse the log and show a summary of before after, it will give confidence of this changes.
| { | ||
| Log(string.Empty, TraceFlag); | ||
| Log(string.Empty, TraceFlag, memberName, sourceFilePath, sourceLineNumber); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] |
There was a problem hiding this comment.
May be we don't need this one anymore.
| @@ -53,18 +52,37 @@ public static void InitializeLogger(string applicationLogPath, bool isLocalLow = | |||
| Trace.AutoFlush = true; | |||
| } | |||
|
|
|||
| public static string GetVersion() | |||
There was a problem hiding this comment.
Can we just ask the caller to pass in its own class type?
From Github copilot, the following code should be AOT compatible:
var versionInfo = FileVersionInfo.GetVersionInfo(typeof(Program).Assembly.Location);
Console.WriteLine($"Version: {versionInfo.FileVersion}
jaimecbernardo
removed
the
Don't merge - Hold for release
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed