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

keystone: add new recipe #25968

Merged
merged 31 commits into from
Nov 20, 2024
Merged

keystone: add new recipe #25968

merged 31 commits into from
Nov 20, 2024

Conversation

luadebug
Copy link
Contributor

@luadebug luadebug commented Nov 17, 2024

Summary

Changes to recipe: keystone/0.9.2

Motivation

Keystone assembler framework: Core (Arm, Arm64, Hexagon, Mips, PowerPC, Sparc, SystemZ & X86) + bindings

Details

Adds the keystone library version 0.9.2


@CLAassistant
Copy link

CLAassistant commented Nov 17, 2024

CLA assistant check
All committers have signed the CLA.

@jcar87 jcar87 self-assigned this Nov 17, 2024
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luadebug Thank you for your PR! I did a review over your contribution, please take a look on my suggestions.

A consideration: The keystone library is C++, not C. The application ktool is C. As we mainly are focused on libraries, the C++ settings should be preserved. (Unless the library has C interface only).

[ 65%] Building CXX object llvm/keystone/CMakeFiles/keystone.dir/ks.cpp.o
[ 65%] Building CXX object llvm/keystone/CMakeFiles/keystone.dir/EVMMapping.cpp.o
/home/conan/.conan2/p/b/keyst2d5b9cc91f5ff/b/src/llvm/keystone/ks.cpp: In function 'ks_err ks_option(ks_engine*, ks_opt_type, size_t)':
/home/conan/.conan2/p/b/keyst2d5b9cc91f5ff/b/src/llvm/keystone/ks.cpp:536:38: warning: this statement may fall through [-Wimplicit-fallthrough=]
  536 |                     ks->MAI->setRadix(16);
      |                     ~~~~~~~~~~~~~~~~~^~~~
/home/conan/.conan2/p/b/keyst2d5b9cc91f5ff/b/src/llvm/keystone/ks.cpp:537:17: note: here
  537 |                 case KS_OPT_SYNTAX_NASM:
      |                 ^~~~
/home/conan/.conan2/p/b/keyst2d5b9cc91f5ff/b/src/llvm/keystone/ks.cpp:544:38: warning: this statement may fall through [-Wimplicit-fallthrough=]
  544 |                     ks->MAI->setRadix(16);
      |                     ~~~~~~~~~~~~~~~~~^~~~
/home/conan/.conan2/p/b/keyst2d5b9cc91f5ff/b/src/llvm/keystone/ks.cpp:545:17: note: here
  545 |                 case KS_OPT_SYNTAX_GAS:
      |                 ^~~~
[ 66%] Linking CXX shared library ../lib/libkeystone.so

Plus, consider using the Cmake template recipe for CCI on next time: https://github.com/conan-io/conan-center-index/tree/master/docs/package_templates/cmake_package/

It should save your time for new recipes. The templates are updated to be Conan 2.x compatible only, the current state in CCI.

recipes/keystone/all/conanfile.py Outdated Show resolved Hide resolved
recipes/keystone/all/conanfile.py Outdated Show resolved Hide resolved
recipes/keystone/all/conanfile.py Outdated Show resolved Hide resolved
recipes/keystone/all/conanfile.py Outdated Show resolved Hide resolved
recipes/keystone/all/conanfile.py Show resolved Hide resolved
recipes/keystone/all/test_v1_package/conanfile.py Outdated Show resolved Hide resolved
recipes/keystone/all/test_package/CMakeLists.txt Outdated Show resolved Hide resolved
recipes/keystone/all/test_package/CMakeLists.txt Outdated Show resolved Hide resolved
recipes/keystone/all/conanfile.py Outdated Show resolved Hide resolved
luadebug and others added 13 commits November 19, 2024 13:55
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
@luadebug
Copy link
Contributor Author

Thank you for your help.

luadebug and others added 3 commits November 19, 2024 19:00
@luadebug
Copy link
Contributor Author

Thank you for your support.

@uilianries
Copy link
Member

When building locally I see the follow error:

In file included from /Users/uilian/.conan2/p/b/keystcb8072365bc38/b/src/llvm/lib/MC/MCELFStreamer.cpp:15:
/Users/uilian/.conan2/p/b/keystcb8072365bc38/b/src/llvm/include/llvm/ADT/STLExtras.h:54:34: error: no template named 'binary_function' in namespace 'std'; did you mean '__binary_function'?
   54 | struct greater_ptr : public std::binary_function<Ty, Ty, bool> {

The std::binary_function was removed in C++17: https://en.cppreference.com/w/cpp/utility/functional/binary_function

It results that this recipe only work with cppstd<17

Co-authored-by: Uilian Ries <uilianries@gmail.com>
luadebug and others added 2 commits November 19, 2024 20:10
Co-authored-by: Uilian Ries <uilianries@gmail.com>
@luadebug
Copy link
Contributor Author

Thank you for your improvements!

Co-authored-by: Uilian Ries <uilianries@gmail.com>
@luadebug
Copy link
Contributor Author

Thank you for clarifying upon licenses.

@uilianries
Copy link
Member

The CI runs C++17 by default, so I built locally on Linux and MacOS, to be sure:

https://gist.github.com/uilianries/f9ac65208c96934768f2520e1c18ac1a

Looking good the build logs.

uilianries
uilianries previously approved these changes Nov 20, 2024
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you for your PR!

Co-authored-by: Abril Rincón Blanco <git@rinconblanco.es>
@luadebug
Copy link
Contributor Author

Thank you for making it clean upon licenses.

Co-authored-by: Uilian Ries <uilianries@gmail.com>
@AbrilRBS AbrilRBS merged commit 6264e9e into conan-io:master Nov 20, 2024
7 checks passed
@AbrilRBS
Copy link
Member

Thanks a lot for taking the time to push this PR to the finish line @luadebug, we appreciate it :)

OMGtechy pushed a commit to OMGtechy/conan-center-index that referenced this pull request Dec 31, 2024
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Abril Rincón Blanco <git@rinconblanco.es>
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.

5 participants