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

Fix: Could not load System.CodeDom exception with xRetry.Reqnroll plugin #316

Merged
merged 5 commits into from
Nov 6, 2024

Conversation

gasparnagy
Copy link
Contributor

@gasparnagy gasparnagy commented Nov 5, 2024

🤔 What's changed?

When the code-behind generation runs on .NET Framework (e.g. when the project is built from Visual Studio), the assembly versions are verified strictly, so if a dependency of the Reqnroll generator is updated (e.g. System.CodeDom or Gherkin), the plugins that were compiled for an earlier version fail, although they would be compatible with the new dependency.

The solution is to hook on the assembly resolve event and if the same assembly has been loaded already, we use the loaded one even if the version number is different.

⚡️ What's your motivation?

Fixes #310: Could not load System.CodeDom exception with xRetry.Reqnroll plugin.

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

n/a

📋 Checklist:

  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • Users should know about my change
    • I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

@clrudolphi
Copy link
Contributor

Doesn't this open Reqnroll to some risk that such easy acceptance of prior versions will result in hard-to-diagnose bugs when backward compatibility isn't perfect?
Do we need to now enhance our testing to include prior versions of common dependencies?
I suppose the relevant question is: what are the alternatives here?

  • Could we require plugins to use version ranges on their dependencies that Reqnroll might also load?
  • Could we limit Reqnroll's prior release acceptance to Microsoft-issued assemblies (such as those in the System.* namespace) under the assumption that those are most likely to be backward compatible while protecting Reqnroll from conflicts caused by 3rd party assemblies?

@gasparnagy
Copy link
Contributor Author

@clrudolphi when you build with dotnet build, these dependency version differences are anyway ignored by the framework, so the issue is really about building from Visual Studio.

In general this is more of an issue with commonly used external plugins, becase we anyway release a new version of our own plugins with a new Reqnroll version.

I don't think we can restrict the dependency compatibility with version number ranges. The System.CodeDom jumped from v4 to v8 and it seems to be fully backward compatible. Also we recently upgraded Gherkin v29 to v30 which is also backwards compatible, but would still cause a similar issue with xRetry (and similar generator plugins).

I think we should do what you say. For these well-known external plugins we should have a test to be able to get notified about compatibility issues. This PR adds such a test for xRetry.Reqnroll v1.0.0. We can follow this pattern for any others as they become known.

@gasparnagy gasparnagy self-assigned this Nov 5, 2024
@gasparnagy gasparnagy requested a review from clrudolphi November 6, 2024 08:58
@gasparnagy gasparnagy merged commit bd11612 into main Nov 6, 2024
4 checks passed
@gasparnagy gasparnagy deleted the fix_xretry_compatibility branch November 6, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could not load file or assembly 'System.CodeDom, Version=4.0.0.0, Culture=neutral
2 participants