Rizin as subproject #4679

amibranch · 2024-10-19T13:16:52Z

Your checklist for this pull request

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the rizin book with the relevant information (if needed)

Detailed description

Added some minor fixes that are relevant, if rizin is used as subproject:

Added extern "C" linkage specification to some public headers of rizin (not specifically relevant for meson, but became necessary in a used project)
Replaced add_global_link_arguments with add_project_link_arguments (for the same reason, as why add_*_arguments have been replaced)
Changed the meson_git_wrapper.py-script to explicitly specify the output-path, as currently the pwd is chosen (which is always the top-build-dir. This leads to several problems, if used in subprojects (where the top rizin-build-dir is NOT the top build-dir)
Made the RIZIN_BUILD_PATH conditional in the integration-test meson-build, as this makes problems in subprojects that do not configure the cli-option

Some minor fixes to comply with CI

Removed a native-arg that is not allowed and should not have any effect in declare_dependency
Exchanged the c_std default options for libzip, rzar and rzwinkd to be gnu99, as otherwise compile-errors occurred with glibc

Test plan

TODO (mainly CI)
...

Closing issues

...

XVilka · 2024-10-19T16:07:25Z

@amibranch it breaks Windows builds:

FAILED: subprojects/libzip-1.9.2/liblibzip.a.p/meson-generated_.._zip_err_str.c.obj 
"cl" "-Isubprojects\libzip-1.9.2\liblibzip.a.p" "-Isubprojects\libzip-1.9.2" "-I..\subprojects\libzip-1.9.2" "-I..\subprojects\libzip-1.9.2\lib" "-Isubprojects\zlib-1.2.13" "-I..\subprojects\zlib-1.2.13" "/MT" "/nologo" "/showIncludes" "/utf-8" "/W2" "/O2" "/Gw" "/Fdsubprojects\libzip-1.9.2\liblibzip.a.p\meson-generated_.._zip_err_str.c.pdb" /Fosubprojects/libzip-1.9.2/liblibzip.a.p/meson-generated_.._zip_err_str.c.obj "/c" subprojects/libzip-1.9.2/zip_err_str.c
c:\projects\rizin\build\subprojects\libzip-1.9.2\zipconf.h(18): fatal error C1017: invalid integer constant expression
[200/1973] Compiling C object subprojects/xz-5.4.3/src/liblzma/liblzma.a.p/lzma_lzma_decoder.c.obj
[201/1973] Compiling C object subprojects/libzip-1.9.2/liblibzip.a.p/lib_zip_add.c.obj
FAILED: subprojects/libzip-1.9.2/liblibzip.a.p/lib_zip_add.c.obj 
"cl" "-Isubprojects\libzip-1.9.2\liblibzip.a.p" "-Isubprojects\libzip-1.9.2" "-I..\subprojects\libzip-1.9.2" "-I..\subprojects\libzip-1.9.2\lib" "-Isubprojects\zlib-1.2.13" "-I..\subprojects\zlib-1.2.13" "/MT" "/nologo" "/showIncludes" "/utf-8" "/W2" "/O2" "/Gw" "/Fdsubprojects\libzip-1.9.2\liblibzip.a.p\lib_zip_add.c.pdb" /Fosubprojects/libzip-1.9.2/liblibzip.a.p/lib_zip_add.c.obj "/c" ../subprojects/libzip-1.9.2/lib/zip_add.c
c:\projects\rizin\build\subprojects\libzip-1.9.2\zipconf.h(18): fatal error C1017: invalid integer constant expression
ninja: build stopped: subcommand failed.

XVilka · 2024-10-19T16:11:35Z

Also pylint complains:

Run export PATH=${HOME}/Library/Python/3.9/bin:${HOME}/Library/Python/3.10/bin:${HOME}/.local/bin:${PATH}
pylint: Command line or configuration file:1: UserWarning: 'BaseException' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.BaseException' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.
pylint: Command line or configuration file:1: UserWarning: 'Exception' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.Exception' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.
************* Module gdbserver
test/scripts/gdbserver.py:25:8: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)
************* Module fuzz_rz_asm
test/fuzz/scripts/fuzz_rz_asm.py:137:11: E0606: Possibly using variable 'sys' before assignment (possibly-used-before-assignment)
************* Module meson_git_wrapper
sys/meson_git_wrapper.py:61:8: E1124: Argument 'output_path' passed by position and keyword in function call (redundant-keyword-arg)
sys/meson_git_wrapper.py:61:47: E0602: Undefined variable 'gittip_dir' (undefined-variable)
************* Module rzshell_which
librz/core/cmd_descs/rzshell_which.py:16:4: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)

-----------------------------------
Your code has been rated at 9.80/10

amibranch · 2024-10-19T16:32:06Z

Also pylint complains:

Run export PATH=${HOME}/Library/Python/3.9/bin:${HOME}/Library/Python/3.10/bin:${HOME}/.local/bin:${PATH}
pylint: Command line or configuration file:1: UserWarning: 'BaseException' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.BaseException' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.
pylint: Command line or configuration file:1: UserWarning: 'Exception' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.Exception' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.
************* Module gdbserver
test/scripts/gdbserver.py:25:8: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)
************* Module fuzz_rz_asm
test/fuzz/scripts/fuzz_rz_asm.py:137:11: E0606: Possibly using variable 'sys' before assignment (possibly-used-before-assignment)
************* Module meson_git_wrapper
sys/meson_git_wrapper.py:61:8: E1124: Argument 'output_path' passed by position and keyword in function call (redundant-keyword-arg)
sys/meson_git_wrapper.py:61:47: E0602: Undefined variable 'gittip_dir' (undefined-variable)
************* Module rzshell_which
librz/core/cmd_descs/rzshell_which.py:16:4: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)

-----------------------------------
Your code has been rated at 9.80/10

I guess this was introduced by one of the rebase-actions by myself. Will be fixed in a next push to this PR: #4680 (I rather want to work on a rebased state, to avoid any regressions). However, I just want to wait for all CI-Results - just in case

wargio · 2024-10-20T03:07:23Z

meson.build

@@ -564,12 +564,13 @@ if r.returncode() == 1 and get_option('subprojects_check')
  error('Subprojects are not updated. Please run `git clean -dxff subprojects/` to delete all local subprojects directories. If you want to compile against current subprojects then set option `subprojects_check=false`.')
 endif

-libzip_dep = dependency('libzip', required: get_option('use_sys_libzip'), static: is_static_build)
+libzip_dep = dependency('libzip', required: get_option('use_sys_libzip'), static: is_static_build, default_options: [ 'c_std=gnu99' ])


Suggested change

libzip_dep = dependency('libzip', required: get_option('use_sys_libzip'), static: is_static_build, default_options: [ 'c_std=gnu99' ])

libzip_dep = dependency('libzip', required: get_option('use_sys_libzip'), static: is_static_build, default_options: [ 'c_std=gnu99,c99' ])

Add also c99 as fallback, otherwise it will not compile on MSVC

needed to make it conditional, as muon would throw an error:

/home/runner/work/rizin/rizin/meson.build:186:21: error ''gnu99,c99'' is not one of ['none', 'c89', 'c99', 'c11', 'c17', 'c18', 'c2x', 'gnu89', 'gnu99', 'gnu11', 'gnu17', 'gnu18', 'gnu2x'] 186 | capstone_proj = subproject('capstone-' + capstone_version, default_options: ['default_library=static', 'c_std=gnu99,c99']) ^_________________________________________________________________________________________________________

wargio · 2024-10-20T03:07:55Z

subprojects/rzar/meson.build

@@ -2,7 +2,7 @@ project('rzar', 'c',
  license: 'LGPL-3.0-only',
  meson_version: '>=0.55.0',
  default_options: [
-    'c_std=c99',
+    'c_std=gnu99',


Suggested change

'c_std=gnu99',

'c_std=gnu99,c99',

as fallback

wargio

Please replace all those c_std=gnu99 with c_std=gnu99,c99 for compatibility with MSVC.

amibranch · 2024-10-20T08:18:36Z

The libzip-1.9.2-error in windows-builds seem to be fixed upstream in dev already with 08904d2

@XVilka either rebase #3706 on current dev (and I will rebase this PR), or we continue with #4680 (and close #3706 and this PR)

wargio · 2024-10-20T11:55:55Z

we need to fix it in another PR, because usually we need some artifacts for a fallback repo.

Also removed an arg that is not allowed in muon and ignored by meson

wargio · 2024-10-21T02:53:53Z

meson.build

+  lang_opt = ['c_std=gnu99']
 elif cc.has_argument('--std=c99')
  add_project_arguments('--std=c99', language: ['c', 'cpp'])
+  lang_opt = ['c_std=c99']
+else
+  lang_opt = []


better to not do this. i prefer the c_std flags to be directly defined in subproject also create a PR with a branch name containing the word dist (required to run all the CIs) and close this and the other rebase pr. you can rebase in one PR directly.

PR with new branch name has been created: #4684

wargio · 2024-10-21T02:54:29Z

subprojects/rzwinkd/meson.build

-  default_options: [
-    'c_std=c99',
-  ]


define this instead of using add_project_arguments

AFAIK, this cannot be designed conditional. However, the MSVC-Compiler could get problems with these flags, therefore we need to check cc.has_argument('--std=*'. (It was missed here in the else-branch. However, in https://github.com/rizinorg/rizin/pull/4680/files#diff-d1ca507ad4335da90daae84cf012d657f3698491ecf5298a3e230442cf8f4ae1 it has been done as intended)

you can always override c_std like you do when you invoke default_options, if instead you use add_project_arguments then is more problematic.

XVilka · 2024-10-21T10:36:35Z

For Muon, could you please open an issue there too?

The libzip-1.9.2-error in windows-builds seem to be fixed upstream in dev already with 08904d2

@XVilka either rebase #3706 on current dev (and I will rebase this PR), or we continue with #4680 (and close #3706 and this PR)

I rebased my PR, lets see if it helps.

amibranch · 2024-10-22T16:33:41Z

As suggested in review, PR is replaced by: #4684

XVilka added 2 commits August 1, 2023 13:46

meson: do not use add_global_arguments()

1907372

meson: set GNU99/C99 standard for subprojects

99671e9

amibranch requested review from yossizap, wargio, XVilka, kazarmy, thestr4ng3r and ret2libc as code owners October 19, 2024 13:16

github-actions bot added rz-test API buildsystem WinDbg labels Oct 19, 2024

amibranch mentioned this pull request Oct 19, 2024

Rizin as subproject [rebased] #4680

Closed

5 tasks

wargio reviewed Oct 20, 2024

View reviewed changes

wargio requested changes Oct 20, 2024

View reviewed changes

Meson-build: Trying to achieve compatibility to use rizin as subproject

ee20a0e

amibranch force-pushed the rizin_as_subproject branch from 11487d4 to d43f53e Compare October 20, 2024 06:41

amibranch force-pushed the rizin_as_subproject branch 2 times, most recently from 14793b8 to d1b1bf2 Compare October 20, 2024 19:25

Fixups for the c_std-options

7c09434

Also removed an arg that is not allowed in muon and ignored by meson

amibranch force-pushed the rizin_as_subproject branch from d1b1bf2 to 7c09434 Compare October 20, 2024 19:49

wargio reviewed Oct 21, 2024

View reviewed changes

XVilka force-pushed the dist-meson-subproject-fixes branch from 99671e9 to 8ada452 Compare October 21, 2024 10:36

amibranch mentioned this pull request Oct 22, 2024

Rizin as subproject dist #4684

Merged

5 tasks

amibranch closed this Oct 22, 2024

amibranch deleted the rizin_as_subproject branch November 3, 2024 19:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rizin as subproject #4679

Rizin as subproject #4679

amibranch commented Oct 19, 2024

XVilka commented Oct 19, 2024 •

edited

Loading

XVilka commented Oct 19, 2024

amibranch commented Oct 19, 2024

wargio Oct 20, 2024 •

edited

Loading

amibranch Oct 20, 2024

wargio Oct 20, 2024

wargio left a comment

amibranch commented Oct 20, 2024 •

edited

Loading

wargio commented Oct 20, 2024

wargio Oct 21, 2024

amibranch Oct 22, 2024

wargio Oct 21, 2024

amibranch Oct 21, 2024

wargio Oct 21, 2024 •

edited

Loading

XVilka commented Oct 21, 2024

amibranch commented Oct 22, 2024

	libzip_dep = dependency('libzip', required: get_option('use_sys_libzip'), static: is_static_build, default_options: [ 'c_std=gnu99' ])
	libzip_dep = dependency('libzip', required: get_option('use_sys_libzip'), static: is_static_build, default_options: [ 'c_std=gnu99,c99' ])

Rizin as subproject #4679

Rizin as subproject #4679

Conversation

amibranch commented Oct 19, 2024

XVilka commented Oct 19, 2024 • edited Loading

XVilka commented Oct 19, 2024

amibranch commented Oct 19, 2024

wargio Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

amibranch Oct 20, 2024

Choose a reason for hiding this comment

wargio Oct 20, 2024

Choose a reason for hiding this comment

wargio left a comment

Choose a reason for hiding this comment

amibranch commented Oct 20, 2024 • edited Loading

wargio commented Oct 20, 2024

wargio Oct 21, 2024

Choose a reason for hiding this comment

amibranch Oct 22, 2024

Choose a reason for hiding this comment

wargio Oct 21, 2024

Choose a reason for hiding this comment

amibranch Oct 21, 2024

Choose a reason for hiding this comment

wargio Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

XVilka commented Oct 21, 2024

amibranch commented Oct 22, 2024

XVilka commented Oct 19, 2024 •

edited

Loading

wargio Oct 20, 2024 •

edited

Loading

amibranch commented Oct 20, 2024 •

edited

Loading

wargio Oct 21, 2024 •

edited

Loading