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

add keystone #5459

Merged
merged 9 commits into from
Oct 10, 2024
Merged

add keystone #5459

merged 9 commits into from
Oct 10, 2024

Conversation

waruqi
Copy link
Member

@waruqi waruqi commented Oct 9, 2024

  • Before adding new features and new modules, please go to issues to submit the relevant feature description first.
  • Write good commit messages and use the same coding conventions as the rest of the project.
  • Please commit code to dev branch and we will merge into master branch in feature
  • Ensure your edited codes with four spaces instead of TAB.

  • 增加新特性和新模块之前,请先到issues提交相关特性说明,经过讨论评估确认后,再进行相应的代码提交,避免做无用工作。
  • 编写友好可读的提交信息,并使用与工程代码相同的代码规范,代码请用4个空格字符代替tab缩进。
  • 请提交代码到dev分支,如果通过,我们会在特定时间合并到master分支上。
  • 为了规范化提交日志的格式,commit消息,不要用中文,请用英文描述。

@waruqi waruqi mentioned this pull request Oct 9, 2024
@luadebug
Copy link
Contributor

luadebug commented Oct 9, 2024

https://github.com/keystone-engine/keystone/blob/master/docs/COMPILE-WINDOWS.md?plain=1#L27
https://github.com/keystone-engine/keystone/blob/master/docs/COMPILE-WINDOWS-CROSS.md?plain=1#L19
Maybe this will come useful.
Especially their workflows. Like https://ci.appveyor.com/project/aquynh/keystone/history
For example rdbo s own implementation of this https://ci.appveyor.com/project/aquynh/keystone/builds/49914144
rdbo s PR keystone-engine/keystone#582
There could be possibility that it works fine for /MD DLL at least. (Should be good to go for both /MD & /MT I believe)
we have issues due of

kstool -b x64 "mov rax, 1; ret"
error: @programdir\core\sandbox\modules\os.lua:337: cannot exec(kstool -b x64 "mov rax, 1; ret"), No such file or directory

in case of dynamic linked libs for windows using msvc.

# for Windows, do not build kstool if buiding DLL
# TODO: fix this

from src code of cmakelists.txt

end)

on_test(function (package)
if not package:is_cross() then
Copy link
Contributor

@luadebug luadebug Oct 9, 2024

Choose a reason for hiding this comment

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

if not package:is_cross() and not is_mode("shared") then

We do not build it because of CMakeLists.txt (at least in case of windows?)
origin src:

# for Windows, do not build kstool if buiding DLL
# TODO: fix this

@luadebug
Copy link
Contributor

Tencent/ncnn#5592 This might fix NDK builds @waruqi ?

@@ -30,6 +30,9 @@ package("keystone")
package:data_set("build_libs_only", true)
elseif package:is_plat("windows") then
table.insert(configs, "-DKEYSTONE_BUILD_STATIC_RUNTIME=" .. (package:has_runtime("MT", "MTd") and "ON" or "OFF"))
elseif package:is_plat("android") then
Copy link
Contributor

@luadebug luadebug Oct 10, 2024

Choose a reason for hiding this comment

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

Github CI ignored it for some reason?

CMake Warning (dev) at /home/runner/work/xmake-repo/xmake-repo/android-ndk-r27/build/cmake/flags.cmake:46 (if):
Policy CMP0057 is not set: Support new IN_LIST if() operator. Run "cmake
--help-policy CMP0057" for policy details. Use the cmake_policy command to
set the policy and suppress this warning.

IN_LIST will be interpreted as an operator when the policy is set to NEW.
Since the policy is not set the OLD behavior will be used.
Call Stack (most recent call first):
/home/runner/.xmake/packages/c/cmake/3.30.2/924bf70cd2964eea80fdac45c84f12e4/share/cmake-3.30/Modules/Platform/Android-Clang.cmake:23 (include)
/home/runner/.xmake/packages/c/cmake/3.30.2/924bf70cd2964eea80fdac45c84f12e4/share/cmake-3.30/Modules/Platform/Android-Clang-C.cmake:1 (include)
/home/runner/.xmake/packages/c/cmake/3.30.2/924bf70cd2964eea80fdac45c84f12e4/share/cmake-3.30/Modules/CMakeCInformation.cmake:48 (include)
CMakeLists.txt:5 (project)
This warning is for project developers. Use -Wno-dev to suppress it.

CMake Error at /home/runner/work/xmake-repo/xmake-repo/android-ndk-r27/build/cmake/flags.cmake:46 (if):
if given arguments:

"hwaddress" "IN_LIST" "ANDROID_SANITIZE"

Unknown arguments specified
Call Stack (most recent call first):
/home/runner/.xmake/packages/c/cmake/3.30.2/924bf70cd2964eea80fdac45c84f12e4/share/cmake-3.30/Modules/Platform/Android-Clang.cmake:23 (include)
/home/runner/.xmake/packages/c/cmake/3.30.2/924bf70cd2964eea80fdac45c84f12e4/share/cmake-3.30/Modules/Platform/Android-Clang-C.cmake:1 (include)
/home/runner/.xmake/packages/c/cmake/3.30.2/924bf70cd2964eea80fdac45c84f12e4/share/cmake-3.30/Modules/CMakeCInformation.cmake:48 (include)
CMakeLists.txt:5 (project)

https://github.com/search?q=repo%3Axmake-io%2Fxmake-repo%20MAKE_POLICY_DEFAULT_CMP0057&type=code

cmake_policy(SET CMP0057 NEW)

android-NDK-r37.patch

--- CMakeLists.txt.orig 2024-10-10 08:00:00.000000000 +0000
+++ CMakeLists.txt      2024-10-10 08:10:00.000000000 +0000
@@ -19,6 +19,10 @@
   cmake_policy(SET CMP0022 NEW) # automatic when 2.8.12 is required
 endif()

+if (POLICY CMP0057)
+  cmake_policy(SET CMP0057 NEW)
+endif()

 if (POLICY CMP0051)

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@luadebug luadebug Oct 10, 2024

Choose a reason for hiding this comment

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

Well it did not work out.
https://github.com/xmake-io/xmake-repo/actions/runs/11276235247/job/31359534078?pr=5459

error: @programdir\core\sandbox\modules\os.lua:273: -- Android: Targeting API '30' with architecture 'arm64', ABI 'arm64-v8a', and processor 'aarch64'
-- Android: Selected unified Clang toolchain
-- The C compiler identification is Clang 18.0.1
-- The CXX compiler identification is Clang 18.0.1
-- Configuring incomplete, errors occurred!
 older versions.


CMake Warning (dev) at D:/a/xmake-repo/xmake-repo/ndk/android-ndk-r27/build/cmake/flags.cmake:46 (if):
  Policy CMP0057 is not set: Support new IN_LIST if() operator.  Run "cmake
  --help-policy CMP0057" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  IN_LIST will be interpreted as an operator when the policy is set to NEW.
  Since the policy is not set the OLD behavior will be used.
Call Stack (most recent call first):
  C:/Program Files/CMake/share/cmake-3.30/Modules/Platform/Android-Clang.cmake:23 (include)
  C:/Program Files/CMake/share/cmake-3.30/Modules/Platform/Android-Clang-C.cmake:1 (include)
  C:/Program Files/CMake/share/cmake-3.30/Modules/CMakeCInformation.cmake:48 (include)
  CMakeLists.txt:5 (project)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error at D:/a/xmake-repo/xmake-repo/ndk/android-ndk-r27/build/cmake/flags.cmake:46 (if):
  if given arguments:

    "hwaddress" "IN_LIST" "ANDROID_SANITIZE"

  Unknown arguments specified
Call Stack (most recent call first):
  C:/Program Files/CMake/share/cmake-3.30/Modules/Platform/Android-Clang.cmake:23 (include)
  C:/Program Files/CMake/share/cmake-3.30/Modules/Platform/Android-Clang-C.cmake:1 (include)
  C:/Program Files/CMake/share/cmake-3.30/Modules/CMakeCInformation.cmake:48 (include)
  CMakeLists.txt:5 (project)

We could rely over cmake.flags file like this:
https://android-review.googlesource.com/c/platform/ndk/+/3211647/1/build/cmake/flags.cmake
according to:
android/ndk#2032 (comment)
Please note it there old versions are being used:
https://github.com/search?q=repo%3Akeystone-engine%2Fkeystone%20cmake_minimum_required&type=code
Reference: https://cmake.org/cmake/help/latest/policy/CMP0057.html

The policy CMP0057 was introduced in CMake 3.3.

This policy adds support for the IN_LIST operator in if() conditions, which allows you to check if a value exists in a list more easily.

Here’s the relevant information:

    CMake Version: 3.3
    Policy: CMP0057
    Description: This policy governs the use of the IN_LIST operator in if() statements. When the policy is set to NEW, the IN_LIST operator is enabled, allowing list membership checks.

If you're using a version of CMake older than 3.3, the condition if(POLICY CMP0057) will fail because the policy does not exist in those versions. You need to upgrade to CMake 3.3 or later to use this policy.

@luadebug
Copy link
Contributor

luadebug commented Oct 10, 2024

@luadebug
Copy link
Contributor

Tests succeed all checks!Ready to merge @waruqi ! Good job! 💯 👍🏻

@waruqi waruqi merged commit 6ce7aef into dev Oct 10, 2024
67 checks passed
@waruqi waruqi deleted the keystone branch October 10, 2024 22:19
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.

2 participants