Safeguards loading of assemblies during load script generation. #3098
Conversation
| try | ||
| (AssemblyDefinition.ReadAssembly path, FileInfo(path)) |> Some | ||
| with | ||
| | _ -> None |
There was a problem hiding this comment.
I don't like that pattern, can we at least catch only the relevant exception or otherwise log the error as warning?
| try | ||
| (AssemblyDefinition.ReadAssembly (f.FullName:string), f) |> Some | ||
| with | ||
| | _ -> None) |
There was a problem hiding this comment.
Yes, sure. Good point.
I was "copying" from https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/Installation/InstallProcess.fs#L300 :)
There was a problem hiding this comment.
Yes, getting rid of them is hard, because you do not know what you have catched previously ;).
A PR or adding the other case is welcome as well ;)
|
Thanks for taking care of this :) |
|
My pleasure! |
| @@ -98,7 99,13 @@ type DependencyCache (lockFile:LockFile) = | |||
| installModel | |||
| |> InstallModel.getLegacyReferences (TargetProfile.SinglePlatform framework) | |||
| |> Seq.map (fun l -> l.Path) | |||
| |> Seq.map (fun path -> AssemblyDefinition.ReadAssembly path, FileInfo(path)) | |||
| |> Seq.map (fun path -> | |||
There was a problem hiding this comment.
One more question: What path is it here? Why is someone putting invalid references into the /lib folder?
There was a problem hiding this comment.
They are invalid in the sense that they are native x64 or x86 dlls that the assembly loader might not like to load.
And yes, the direct choose is obviously a better way.
| @@ -121,7 128,11 @@ type DependencyCache (lockFile:LockFile) = | |||
|
|
|||
| let assemblyFilePerAssemblyDef = | |||
| libs |> Seq.map (fun (f:FileInfo) -> | |||
There was a problem hiding this comment.
why not choose/pick directly here?
There was a problem hiding this comment.
Obviously here as well.
|
thanks |
This is a super naive approach to the problem of loading native assembies that will fail with a
BadImageFormatException.Fixes #3097 .
I confirmed, that this fixes the error for https://github.com/ChrSteinert/Paket-3097