Skip to content

IDecompilationService - Use Assembly.GetName().Version rather than FileVersionInfo.GetVersionInfo#82490

Open
MattParkerDev wants to merge 2 commits intodotnet:mainfrom
MattParkerDev:use-assembly-version
Open

IDecompilationService - Use Assembly.GetName().Version rather than FileVersionInfo.GetVersionInfo#82490
MattParkerDev wants to merge 2 commits intodotnet:mainfrom
MattParkerDev:use-assembly-version

Conversation

@MattParkerDev
Copy link
Contributor

FileVersionInfo.GetVersionInfo requires a path to an assembly on disk. This fails in cases where the assembly has been loaded as a stream, rather than from a file.

This replaces it with Assembly.GetName().Version, which I have verified returns the same string, in this case, for ICSharpCode.Decompiler - "9.1.0.7988".

This will also improve performance (probably unnoticeably), as it doesn't need to read the assembly details from disk.

@MattParkerDev MattParkerDev requested a review from a team as a code owner February 21, 2026 16:17
@dotnet-policy-service dotnet-policy-service bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode labels Feb 21, 2026
@jasonmalinowski
Copy link
Member

Does ILSpy guarantee that the assembly version and file version will be roughly the same? I imagine we did this because some assemblies tend to put fairly meaningless stuff in the assembly version (since changing assembly version can cause issues), and the file version is where "interesting" version information is.

@jasonmalinowski jasonmalinowski self-assigned this Feb 23, 2026
@JoeRobich
Copy link
Member

JoeRobich commented Feb 23, 2026

Does ILSpy guarantee that the assembly version and file version will be roughly the same? I imagine we did this because some assemblies tend to put fairly meaningless stuff in the assembly version (since changing assembly version can cause issues), and the file version is where "interesting" version information is.

This just seems to be informational to let the user what version of ilspy was used for decompilation. I am not sure how exact we need to be. I suppose we could make this a fallback for when the assembly does not exist on disk with little consequence.

@jasonmalinowski
Copy link
Member

@JoeRobich yep, it's informational. As long as assembly version is "informational enough" then we're good. But some projects take the philosophy that the assembly version is basically frozen for all time, since changing it breaks stuff. In that case, it's not so informational anymore.

@JoeRobich
Copy link
Member

JoeRobich commented Feb 23, 2026

@jasonmalinowski So it appears that ilspy disables generation of the FileAssembly Version and only sets the AssemblyVersion. Meaning the AssemblyVersion will be used for both.

@jasonmalinowski
Copy link
Member

@jasonmalinowski So it appears that ilspy (disables generation of the FileAssembly Version)[https://github.com/icsharpcode/ILSpy/blob/a1e2a4aa915f5fd745550b6beefafbf62263b239/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj#L26-L28] and only sets the AssemblyVersion. Meaning the AssemblyVersion will be used for both.

Then that's good enough for me if we just put a comment to that effect in this PR that we're going to keep assuming it.

@MattParkerDev
Copy link
Contributor Author

Then that's good enough for me if we just put a comment to that effect in this PR that we're going to keep assuming it.

Done! Feel free to alter the wording of the comment.

private static readonly FileVersionInfo s_decompilerVersion = FileVersionInfo.GetVersionInfo(typeof(CSharpDecompiler).Assembly.Location);
// ICsharpCode.Decompiler disables generation of the AssemblyFileVersion, and sets the AssemblyVersion, meaning that
// typeof(CSharpDecompiler).Assembly.GetName().Version and FileVersionInfo.GetVersionInfo(typeof(CSharpDecompiler).Assembly.Location) return the same version.
private static readonly Version s_decompilerVersion = typeof(CSharpDecompiler).Assembly.GetName().Version ?? throw new ArgumentNullException();
Copy link
Member

Choose a reason for hiding this comment

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

I'd say change this to a string and then substitute out "unknown" in the case of a failure; otherwise if this were to fail then decompilation would be entirely busted. Not worth it for some debugging string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants