-
Notifications
You must be signed in to change notification settings - Fork 483
Restore ability on .NET 9 and later to save dynamic assemblies to disk #718
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
base: master
Are you sure you want to change the base?
Conversation
Note that `catch`-ing strong-naming failures in all cases isn't just a refactoring, but a functional change. It seems appropriate, and more correct than what we had before, becaus creating a dynamic assembly and strong-naming it are really separate concerns.
There is the potential of file collisions when several `PersistentProxy- Builder`s are used concurrently. Better use a random name fragment in- stead of a steadily increasing generation count.
|
@jonorossi, do you think the ability to save dynamic assemblies should be publicly exposed, or should it remain an internal development-only feature? I am undecided whether the feature makes sense in general, given that there is no way to load the saved assemblies back into DynamicProxy's type cache. (That would require de-serialization using |
| Assume.That(assemblyPaths, Is.Empty, "Test did not start with an empty assembly paths list."); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate logic. This has since moved into a [SetUp] method.
| [FixtureLifeCycle(LifeCycle.InstancePerTestCase)] | ||
| public class PersistentProxyBuilderTestCase | ||
| { | ||
| // The above fixture life cycle gives each test its own fresh fixture, such that each test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test. → test (no full-stop)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly extract the net9.0 TFM addition into a separate PR. The net9.0 TFM will be needed for by-ref-like support, too, so it might be nicer to introduce the TFM as a separate preliminary PR for both features, instead of being encased with this feature here.
| using System.Diagnostics.CodeAnalysis; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- should be inside the
namespaceblock - likely superfluous anyway
| var builder = new PersistentProxyBuilder(); | ||
| builder.AssemblySaved += assemblyPaths.Add; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This (and the same code in the other test) could probably go into the [SetUp] method, too, since that one already has dealings with assemblyPaths.
| // for testing purposes | ||
| internal event Action<string>? AssemblySaved; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't particularly like adding such stuff only for internal testing. Perhaps it would be worth thinking about doing this in a way that would make sense for public use, too?
This PR adds a
PersistentProxyBuilderimplementation for .NET 9+ – similar, but not identical, to the one we already have for the .NET Framework. The minimum platform requirement for this new implementation is .NET 9 because it is based on SRE'sPersistedAssemblyBuilder, which was introduced in that version of the runtime. Therefore I am also adding thenet9.0TFM to the library.I have an immediate need for this as an aid in improving by-ref-like types support further (#663), which is quite difficult when one cannot inspect the generated assemblies directly. While the proposed implementation should suffice for that specific purpose, it isn't thoroughly tested yet and it comes with certain caveats (as noted in its XML documentation comment). However, since it is an opt-in feature, it seems reasonably safe to me to publicly expose it.
What is left to be done at a later time is setting up IL verification during test execution on .NET 9 and later, even though that now seems to be within reach.
Closes #697.