Skip to content

Version.txt should use file version from assembly custom attributes not FileVersionInfo #4505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
r0ss88 opened this issue Nov 27, 2019 · 4 comments
Labels
bug Something isn't working loadsave Bugs related loading and saving data or models P2 Priority of the issue for triage purpose: Needs to be fixed at some point.

Comments

@r0ss88
Copy link
Contributor

r0ss88 commented Nov 27, 2019

The changes made in issue #3132 have introduced a bug due to relying on a physical location for the assembly. If the Microsoft.ML.Core assembly is loaded from memory the assembly.location will be empty. Using FileVersionInfo.GetVersionInfo relies on a physical path - so an argument exception is thrown inside System.IO.Path since the supplied path is empty.

var versionInfo = FileVersionInfo.GetVersionInfo(typeof(RepositoryWriter).Assembly.Location);
using (var ent = rep.CreateEntry(DirTrainingInfo, "Version.txt"))
using (var writer = Utils.OpenWriter(ent.Stream))
writer.WriteLine(versionInfo.ProductVersion);

Instead of using FileVersionInfo.GetVersionInfo the assembly custom attributes should be used, for example:
var productVersion = assembly.CustomAttributes.FirstOrDefault(a => a.AttributeType == typeof(AssemblyFileVersionAttribute)).ConstructorArguments.First();

This will return the same product string version as the FileVersionInfo.GetVersionInfo does.

r0ss88 added a commit to r0ss88/machinelearning that referenced this issue Nov 29, 2019
…from FileVersionInfo and replace with using assembly custom attributes (dotnet#4505)
@codemzs
Copy link
Member

codemzs commented Nov 30, 2019

thanks @r0ss88 . This seems like a good find but in what scenarios can we be loading this assembly from memory and it will not be present on disk?

@r0ss88
Copy link
Contributor Author

r0ss88 commented Dec 2, 2019

thanks @r0ss88 . This seems like a good find but in what scenarios can we be loading this assembly from memory and it will not be present on disk?

Happy to contribute. In my scenario I am working with a system where assemblies can be loaded from disk or stored in a database. Though in this case they are loaded from disk the assembly is read as bytes then loaded - hence location is blank.

@r0ss88
Copy link
Contributor Author

r0ss88 commented Jan 2, 2020

@codemzs happy new year, any update?

@sharwell
Copy link
Contributor

sharwell commented Jan 2, 2020

@codemzs FileVersionInfo is not correct either way. This should be using the AssemblyInformationalVersion attribute (when present). If you use GitVersioning to generate the assembly attributes, it will produce an internal static class that you can reference directly for any information you need. Other version generation tools may do something similar. Otherwise, an approach like I used for ANTLR's code generator can be used:

https://github.com/tunnelvisionlabs/antlr4cs/blob/12f3271157c2df1984eac7009d71c929fd070f20/runtime/CSharp/Antlr4.Tool/AntlrTool.cs#L39-L45

@gvashishtha gvashishtha added the P1 Priority of the issue for triage purpose: Needs to be fixed soon. label Jan 9, 2020
@Lynx1820 Lynx1820 added the bug Something isn't working label Jan 9, 2020
@harishsk harishsk added the loadsave Bugs related loading and saving data or models label Apr 29, 2020
@frank-dong-ms-zz frank-dong-ms-zz self-assigned this May 21, 2020
@frank-dong-ms-zz frank-dong-ms-zz added P2 Priority of the issue for triage purpose: Needs to be fixed at some point. and removed P1 Priority of the issue for triage purpose: Needs to be fixed soon. labels Jun 9, 2020
@frank-dong-ms-zz frank-dong-ms-zz removed their assignment Jun 25, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working loadsave Bugs related loading and saving data or models P2 Priority of the issue for triage purpose: Needs to be fixed at some point.
Projects
None yet
Development

No branches or pull requests

7 participants