-
-
Notifications
You must be signed in to change notification settings - Fork 361
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
Rizin as subproject dist #4684
Rizin as subproject dist #4684
Conversation
3a52010
to
620549e
Compare
620549e
to
f11274e
Compare
beea4e0
to
3eccf1b
Compare
Note:
|
3eccf1b
to
eeb0889
Compare
Same for tree-sitter |
eeb0889
to
2b9d7db
Compare
Same for xz/Libzlma and rzgdb |
3c69d33
to
d7db846
Compare
To any potential reviewer: |
d7db846
to
13aac19
Compare
Done so far. PR is ready for review |
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.
Great job! I am blocking this first to push to the internal branch first so that even more CI jobs are triggered. If they pass - I think it's mergeable almost as is. Please squash your (and mine, if necessary) commits to be self-descriptive and/or self-contained.
@@ -355,7 +359,7 @@ foreach it : ccs | |||
have_pthread = it_th.found() and it_machine.system() != 'windows' | |||
if it_machine.system() == 'sunos' | |||
# workaround for Solaris until https://github.com/mesonbuild/meson/issues/4328 is fixed |
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.
Remove the comment also then.
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.
@thestr4ng3r take a look at this one please
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.
Can't test unfortunately. Seems my solaris disk is not in the server and I have no physical access at the moment. But from manual review, this looks good and likely we can remove this workaround entirely in another patch independently of this pr.
@amibranch all additional CI jobs passed flawlessly. Lets wait for more reviews and it's ready to be merged, IMHO. Just squash your commits. |
13aac19
to
38a719c
Compare
Squashed commits into one - awaiting reviews |
38a719c
to
6018188
Compare
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.
seems ok on linux.
@amibranch please rebase again and make a commit message to follow the common commit convention: https://www.conventionalcommits.org As for the review, now only waiting for @thestr4ng3r to respond about the Solaris changes. After that is sorted out - it's good to merge. |
* Setting GNU99/C99 standard for subprojects * Remove use of `add_global_arguments` * Added extern "C" linkage specification to some public headers of rizin * Changed the meson_git_wrapper.py-script to explicitly specify the output-path * Made the RIZIN_BUILD_PATH conditional in the integration-test meson-build
6018188
to
ef04796
Compare
Linking failure on OpenBSD/sparc64, but this seems unrelated, doesn't it? |
Not sure what changed the behavior of the CI here. Will investigate and rebase if necessary Update: |
Yes, we will merge after #4691 (when it's updated to fix remaining issues) to make CI happy. |
Your checklist for this pull request
Detailed description
PR to replace:
add_global_arguments()
#3706Test plan
CI is green
Closing issues
Partially addresses #3454