Skip to content

Conversation

@lei9444
Copy link
Contributor

@lei9444 lei9444 commented Jun 20, 2025

Summary of the Pull Request

Root cause:

FileVersionInfo.GetVersionInfo(Assembly.Location) is not compatible with NativeAOT

Fix:

Replaced the usage of FileVersionInfo with static reflection GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version. This uses static reflection to read version metadata directly from the assembly, which is fully supported in NativeAOT. It does not require file system access and is trimming/AOT-safe.

PR Checklist

  • Closes: #40151
  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copy link
Contributor

@moooyo moooyo left a comment

Choose a reason for hiding this comment

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

LGTM

@moooyo
Copy link
Contributor

moooyo commented Jun 20, 2025

Anyone have any concern?

@lei9444
Copy link
Contributor Author

lei9444 commented Jun 20, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lei9444
Copy link
Contributor Author

lei9444 commented Jun 20, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lei9444
Copy link
Contributor Author

lei9444 commented Jul 1, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moooyo moooyo merged commit 73841f6 into main Jul 1, 2025
16 of 19 checks passed
@yeelam-gordon yeelam-gordon added the Area-Logging Issues regarding the PowerToys logging facility label Jul 30, 2025
@yeelam-gordon yeelam-gordon added this to the PowerToys 0.93 milestone Jul 30, 2025
@yeelam-gordon yeelam-gordon requested a review from Copilot August 5, 2025 05:05
Copy link
Contributor

Copilot AI left a 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 updates the logger implementation to support NativeAOT by replacing FileVersionInfo.GetVersionInfo(Assembly.Location) with a static reflection approach using GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version.

  • Removes NativeAOT-incompatible file system access for version information
  • Replaces FileVersionInfo usage with assembly attribute reflection
  • Eliminates the need for pragma warnings related to single-file publishing

#pragma warning disable IL3000 // Avoid accessing Assembly file path when publishing as a single file
private static readonly string Version = FileVersionInfo.GetVersionInfo(Assembly.Location).ProductVersion;
#pragma warning restore IL3000 // Avoid accessing Assembly file path when publishing as a single file
private static readonly string Version = Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version ?? "Unknown";
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

[nitpick] The fallback value "Unknown" may not provide sufficient information for debugging purposes. Consider using a more descriptive fallback like "Version not available" or including additional context about why the version could not be determined.

Suggested change
private static readonly string Version = Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version ?? "Unknown";
private static readonly string Version = Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version ?? "Version not available";

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Logging Issues regarding the PowerToys logging facility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants