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

Allow using system pugixml for build #106

Open
mxkrsv opened this issue Jan 6, 2023 · 4 comments
Open

Allow using system pugixml for build #106

mxkrsv opened this issue Jan 6, 2023 · 4 comments

Comments

@mxkrsv
Copy link

mxkrsv commented Jan 6, 2023

I ended up patching meson.build by myself:

From 971fdf012c6c48e43e00d91727698ace4b0c67a6 Mon Sep 17 00:00:00 2001
From: Maxim Karasev <[email protected]>
Date: Fri, 23 Dec 2022 22:27:24 +0300
Subject: [PATCH] Use system pugixml

---
 meson.build | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/meson.build b/meson.build
index e35d4f7..a5788c9 100644
--- a/meson.build
+++ b/meson.build
@@ -7,10 +7,6 @@ version : '5.8.0',
 default_options : ['c_std=c11', 'cpp_std=c++11']
 )
 
-pugixml_sources = [
-  'third-party/pugixml/src/pugixml.cpp'
-]
-
 r2ghidra_sources = [
   'src/ArchMap.cpp',
   'src/CodeXMLParse.cpp',
@@ -28,7 +24,6 @@ r2ghidra_sources = [
 
 incdirs = [
   'src',
-  'third-party/pugixml/src/',
   'ghidra-native/src/decompiler/',
 ]
 
@@ -178,7 +173,6 @@ ghidra_decompiler_sources = [
 
 r2ghidra_core_sources = [
   r2ghidra_sources,
-  pugixml_sources,
   ghidra_decompiler_sources,
   'src/anal_ghidra_plugin.c',
   'src/anal_ghidra.cpp',
@@ -188,16 +182,17 @@ r2ghidra_core_sources = [
 
 sleighc_sources = [
   r2ghidra_sources,
-  pugixml_sources,
   'ghidra-native/src/decompiler/slgh_compile.cc',
   'ghidra-native/src/decompiler/slghparse.cc',
   'ghidra-native/src/decompiler/slghscan.cc',
   ghidra_decompiler_sources,
 ]
 
+pugixml = dependency('pugixml')
+
 r2ghidra_core_plugin = library('core_r2ghidra',
   r2ghidra_core_sources,
-  dependencies: [r_core],
+  dependencies: [r_core, pugixml],
   override_options : ['c_std=c11', 'cpp_std=c++11'],
   include_directories: r2ghidra_incdirs,
   install: true,
@@ -207,6 +202,6 @@ r2ghidra_core_plugin = library('core_r2ghidra',
 sleighc_exe = executable('sleighc', sleighc_sources,
   include_directories: r2ghidra_incdirs,
   override_options : ['c_std=c11', 'cpp_std=c++11'],
-  dependencies: [r_core],
+  dependencies: [r_core, pugixml],
   install: true
 )
-- 
2.39.0

Would be nice to have such ability as an option in upstream.

@eli-schwartz
Copy link

eli-schwartz commented Jan 6, 2023

Using subproject wraps this is actually really easy and Meson provides all the tools and UX for making this an option without you doing anything. Just, in addition to the above patch, create the directory subprojects/ and run meson wrap install pugixml.

@trufae
Copy link
Contributor

trufae commented Jan 6, 2023

What i find out is that the pugixml dependency doesnt work even if it builds it fails at runtime because the apis changed (thats why its not using the last version of pugixml). Ideally i would prefer to depend on r2's xml parser instead. So we dont depend on external things that eventually break

so im not a big fan of adding the pption to use system wide pugixml

@eli-schwartz
Copy link

Just add an exact version of pugixml as a dependency. If the system version is too new, it will not be found and the bundled one is forced.

@trufae
Copy link
Contributor

trufae commented Jan 9, 2023

To me that looks like an extra moving piece that makes the maintainance harder and opens the doors to new bugs. But if thats not enabled by default and distros are happy with it im fine to ship it as a temporal fix. But the proper fix would be to use r2xml instead of external code

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

No branches or pull requests

3 participants