-
Notifications
You must be signed in to change notification settings - Fork 459
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
Upgrade the .NET bindings to .NET Standard 2.0. #410
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# Changelog of the .NET bindings for Keystone | ||
|
||
**Version 0.9.1.1: May 18th, 2019**: | ||
|
||
- Update to .NET Standard 2.0. | ||
- Load dynamically the dynamic-link library that corresponds to the architecture of the running process (32 or 64-bit). | ||
|
||
|
||
**Version 0.9.1.0: August 2nd, 2018**: | ||
|
||
- Refactor the bindings to target .NET Standard 1.1. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,36 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>netstandard1.1</TargetFramework> | ||
<TargetFramework>netstandard2.0</TargetFramework> | ||
<RootNamespace>Keystone</RootNamespace> | ||
|
||
<Version>1.1.0</Version> | ||
<AssemblyVersion>$(Version)</AssemblyVersion> | ||
<FileVersion>$(Version).0</FileVersion> | ||
<Version>0.9.1.1</Version> | ||
<AssemblyVersion>0.9.1.1</AssemblyVersion> | ||
<FileVersion>0.9.1.1</FileVersion> | ||
|
||
<Description>.NET bindings to the Keystone Engine.</Description> | ||
<Authors>Grégoire Geis</Authors> | ||
<Description>Keystone is a lightweight multi-platform, multi-architecture assembler framework. This package corresponds to the csharp bindings in the official git repository.</Description> | ||
<Authors>Nguyen Anh Quynh, Marco Fornaro, Grégoire Geis, Jämes Ménétrey</Authors> | ||
|
||
<PackageId>Keystone.Net</PackageId> | ||
<PackageId>keystoneengine.csharp</PackageId> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why renaming to However this does pose a problem: How do we decrease the current version? (I'm not sure NuGet allows this). |
||
<PackageVersion>$(Version)</PackageVersion> | ||
<PackageRequireLicenseAcceptance>False</PackageRequireLicenseAcceptance> | ||
<PackageReleaseNotes>- First release.</PackageReleaseNotes> | ||
<PackageTags>assembler x86 arm keystone</PackageTags> | ||
<PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance> | ||
<PackageReleaseNotes>Release notes can be found at: https://github.com/keystone-engine/keystone/blob/0.9.1/ChangeLog</PackageReleaseNotes> | ||
<PackageTags>assembler x86 x64 arm keystone llvm</PackageTags> | ||
|
||
<PackageProjectUrl>https://github.com/keystone-engine/keystone</PackageProjectUrl> | ||
<PackageLicenseUrl>$(PackageProjectUrl)/blob/master/COPYING</PackageLicenseUrl> | ||
<PackageProjectUrl>http://www.keystone-engine.org</PackageProjectUrl> | ||
<PackageLicenseUrl>https://github.com/keystone-engine/keystone#license</PackageLicenseUrl> | ||
<PackageIconUrl>http://www.keystone-engine.org/images/keystone.png</PackageIconUrl> | ||
|
||
<RepositoryUrl>$(PackageProjectUrl).git</RepositoryUrl> | ||
<RepositoryUrl>https://github.com/keystone-engine/keystone</RepositoryUrl> | ||
<RepositoryType>git</RepositoryType> | ||
<Company /> | ||
<Copyright>Nguyen Anh Quynh</Copyright> | ||
<GeneratePackageOnBuild>true</GeneratePackageOnBuild> | ||
<Product>Keystone Engine - .NET Bindings</Product> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'"> | ||
<GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
</PropertyGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
using System; | ||
using System.IO; | ||
using System.Runtime.InteropServices; | ||
|
||
namespace Keystone | ||
|
@@ -8,29 +9,34 @@ namespace Keystone | |
/// </summary> | ||
internal class NativeInterop | ||
{ | ||
// This shouldn't be needed, even on Windows | ||
// /// <summary> | ||
// /// Taken from: http://stackoverflow.com/questions/10852634/using-a-32bit-or-64bit-dll-in-c-sharp-dllimport | ||
// /// </summary> | ||
// static NativeInterop() | ||
// { | ||
// var myPath = new Uri(typeof(NativeInterop).Assembly.CodeBase).LocalPath; | ||
// var myFolder = Path.GetDirectoryName(myPath); | ||
/// <summary> | ||
/// Load the appropriate dynamic-link library, according the architecture of the running application. | ||
/// </summary> | ||
/// <remarks> | ||
/// Taken from: http://stackoverflow.com/questions/10852634/using-a-32bit-or-64bit-dll-in-c-sharp-dllimport | ||
/// </remarks> | ||
static NativeInterop() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm kinda curious about this: Is it ever needed? I removed it from the original commit because after tests in different environments I never ended up needing this. |
||
{ | ||
var libPath = Path.GetDirectoryName(new Uri(typeof(NativeInterop).Assembly.CodeBase).LocalPath); | ||
var is64 = IntPtr.Size == 8; | ||
var subfolder = is64 ? "x64" : "x86"; | ||
|
||
// var is64 = IntPtr.Size == 8; | ||
// var subfolder = is64 ? "\\win64\\" : "\\win32\\"; | ||
if (!string.IsNullOrEmpty(libPath)) | ||
{ | ||
var dllPosition = Path.Combine(libPath, subfolder, "keystone.dll"); | ||
|
||
// string dllPosition = myFolder + subfolder + "keystone.dll"; | ||
// If this file exist, load it. | ||
// Otherwise let the marshaller load the appropriate file. | ||
if (File.Exists(dllPosition)) | ||
{ | ||
LoadLibrary(dllPosition); | ||
} | ||
} | ||
} | ||
|
||
// // If this file exist, load it. | ||
// // Otherwise let the marshaller load the appropriate file. | ||
// if (File.Exists(dllPosition)) | ||
// LoadLibrary(dllPosition); | ||
// } | ||
[DllImport("kernel32.dll")] | ||
private static extern IntPtr LoadLibrary(string dllToLoad); | ||
|
||
// [DllImport("kernel32.dll")] | ||
// private static extern IntPtr LoadLibrary(string dllToLoad); | ||
|
||
[DllImport("keystone", CallingConvention = CallingConvention.Cdecl, EntryPoint = "ks_version" )] | ||
internal static extern uint Version(ref uint major, ref uint minor); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
# Keystone.Net | ||
.NET Standard bindings for Keystone. | ||
# Keystone.Net | ||
.NET bindings for Keystone (.NET Standard 2.0), written in C#. | ||
|
||
## Usage | ||
```csharp | ||
|
@@ -30,3 +30,36 @@ using (Engine keystone = new Engine(Architecture.X86, Mode.X32) { ThrowOnError = | |
|
||
For those who already used the bindings before their last update, many things have changed. | ||
You can migrate your existing code easily using the [migration guide](./MIGRATON.md). | ||
|
||
## NuGet package | ||
The NuGet package `keystoneengine.csharp` is maintained to reflect the latest version of the bindings and the library. It can either be downloaded using Visual Studio or by [browsing NuGet directly](https://www.nuget.org/packages/keystoneengine.csharp/). The package already embeds the 32/64-bit native dynamic-link libraries of Keystone. | ||
|
||
## Found an issue or bug ? | ||
Feel free to open a GitHub issue on [the official repository of Keystone](https://github.com/keystone-engine/keystone/issues) and ping the contributors. | ||
|
||
|
||
## Contributors | ||
Authors: | ||
|
||
- Grégoire Geis ([https://github.com/71](@71)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Link and text are swapped. |
||
- Jämes Ménétrey ([@ZenLulz](https://github.com/ZenLulz/)) | ||
- Marco Fornaro ([@chaplin89](https://github.com/chaplin89)) | ||
|
||
### Want to contribute ? | ||
Hey you! Your help is more than welcome! Things to keep in mind when working on the .NET bindings for Keystone: | ||
|
||
- Think about the backward compatibility; while code refactoring is a good practice, changing entirely the API *may* result in struggles. | ||
- Elaborate the unit tests that prove your code is working. Test all the paths of the newly added functions/classes. Keep the code coverage high! | ||
- Please; write the required *XML Documentation Comments*, so every developer has the chance to understand your code. | ||
- Update the changelog with a summary of your changes. | ||
|
||
#### Version notation | ||
The version of the .NET bindings for Keystone is indicated in the Visual Studio project file. The major, minor and incremental versions (w.x.y) match the version of the library Keystone that the bindings are developed with. The build number (the .z in w.x.y.z) is incremented for each newer version of the .NET bindings. Please, don't forget to increment this version when you submit a pull request. | ||
|
||
On the last commit for a pull request, please create a tag called `csharp-bindings-w.x.y.z`. | ||
|
||
#### Pull request submission | ||
Ping the contributors of the .NET bindings when submitting a pull request, so your changes can be peer reviewed. | ||
|
||
#### NuGet package update | ||
Once your pull request has been accepted, please contact [@ZenLulz](https://github.com/ZenLulz/) so either he updates the library for you or add you to the project as a contributor on nuget.org. An example of *nupkg* is provided in this folder, as it requires to have a very specific configuration, because it embeds unmanaged libraries. The picture *nuget-package-config.png* details the structure and content of a NuGet package, ready to be deployed. Please reuse and test the package before pushing it onto nuget.org, as there is no possible roll back. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we end up keeping the existing package name, let's remember to modify this line. |
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.
Since we're not using features only available in .NET Standard 2.0 (AFAIK), I think we should try targeting multiple frameworks by replacing
by