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

Use System.Reflection.Metadata to read assembly info #237

Merged
merged 3 commits into from Nov 2, 2017

Conversation

Projects
None yet
3 participants
@zvirja
Contributor

zvirja commented Oct 27, 2017

Fixes #229. This is a rework of #235.

Instead of loading assembly to AppDomain to retrieve its references and assembly attributes I use the System.Reflection.Metadata package as advised here.

Changes applied in this PR:

  • Added AssemblyMetadataReader to fetch assembly metadata without assembly load. Also I've introduced AssemblyMetadataParser to encapsulate all the low-level work with System.Reflection.Metadata package.
  • Moved metadata sorting to the AssemblyMetadata class.
  • Now Full Name and Strong Name metadata entries are always placed at the top. That allows to quickly see assembly information. Other attributes are placed after it.

I've tested the app and it works fine with all the packages I saw.
NLog 4.5 beta 2:
image

@304NotModified @onovotny Please take a look and share your feedback.

zvirja added some commits Oct 27, 2017

Use Reflection.Metadata package to read assembly info
The previous approach when we tried to load assembly doesn't work
well for the different target frameworks as dependencies might be
missing.
@zvirja

This comment has been minimized.

Show comment
Hide comment
@zvirja

zvirja Nov 2, 2017

Contributor

@304NotModified @onovotny Any chance you will have time in the nearby future to review this? 😞

Contributor

zvirja commented Nov 2, 2017

@304NotModified @onovotny Any chance you will have time in the nearby future to review this? 😞

@onovotny onovotny merged commit 1861f0b into NuGetPackageExplorer:master Nov 2, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@zvirja zvirja deleted the zvirja:use-reflection-lib-to-load-assembly-info branch Nov 2, 2017

@zvirja

This comment has been minimized.

Show comment
Hide comment
@zvirja

zvirja Nov 2, 2017

Contributor

@onovotny Thank you for the review!

Contributor

zvirja commented Nov 2, 2017

@onovotny Thank you for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment