Skip to content
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

Still can't resolve resource assemblies after GH2734 #3226

Closed
mmarinchenko opened this issue Mar 3, 2021 · 0 comments · Fixed by #3227
Closed

Still can't resolve resource assemblies after GH2734 #3226

mmarinchenko opened this issue Mar 3, 2021 · 0 comments · Fixed by #3227
Assignees
Labels
Milestone

Comments

@mmarinchenko
Copy link
Contributor

mmarinchenko commented Mar 3, 2021

This issue is continuation of GH2734.

It's like making 5 mistakes in a 3-letter catch... Seems no one actually tested or reviewed this properly.

  1. There is some mess between using full and short assembly name in ScriptAssemblyResolver. For example Assembly.Load() method expects full name not a short one. Also see point 4 below.
  2. Assembly version is ignored. This leads to possible compatibility issues when different versions of resolved assembly have breaking changes.
  3. try-catch scope is too wide. If .resources assembly can't be loaded the Assembly.Load() throws a FileNotFoundException. Due to .resources assembly name check is inside try block after Assembly.Load() call it is never executed for .resources assembly. Additionally recursive call to AssemblyResolve() must be outside of try block since it handles exceptions on its own.
  4. Finally the actual .resources assembly name check is always false because it is performed against short assembly name which never contains comma separators or spaces :)
@augustoproiete augustoproiete added this to the v1.x Next Candidate milestone Mar 3, 2021
devlead added a commit that referenced this issue May 26, 2021
GH3226: Resolve neutral culture assembly if .resources file is not found.
@devlead devlead modified the milestones: v1.x Next Candidate, v1.2.0 May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants