Autoinstall frida devkits #5
Conversation
|
Looks like the CI is failing, because it does the same thing... it downloads and installs the devkits. I recommend dropping that from the CI script. |
|
Thank you for the PR. I have a few issues with this change, though. Firstly, I am not a fan of downloading and installing arbitrary code without user consent during the build process. I would be OK, however, if we required the user to specify an environment variable during Secondly, the code duplication here is a bit unfortunate. Especially since we would now have compatible Frida version numbers in the READMEs and these build scripts which is tedious to update. If possible, see if you can pull the Please let me know what you think and we can get this merged. |
|
Ok. I hear what you are saying about the download. Can I recommend that instead of an environment variable we use a feature flag? This will make it easier to include this crate in other packages? About the code duplication. I totally agree: it stinks. I just wanted to get something working. I will create the frida-build crate as you recommend. |
|
Implemented your suggestions. At the moment I've made the "autodownload" feature flag default. We can change that if you want. Ready to merge from my perspective. |
s1341
force-pushed
the
autoinstall_frida_devkits
branch
from
c4f60ab
to
f8cef76
Compare
frida-sys/frida-core.h
Outdated
| @@ -219,6 +219,7 @@ | |||
| #define g_assertion_message_cmpstrv _frida_g_assertion_message_cmpstrv | |||
| #define g_assertion_message_error _frida_g_assertion_message_error | |||
| #define g_assertion_message_expr _frida_g_assertion_message_expr | |||
| #define g_assertion_set_handler _frida_g_assertion_set_handler | |||
There was a problem hiding this comment.
Please uncheck this file from this PR. I updated Frida recently and this appears to be the old one?
|
I'll get to your request tomorrow. I have some other questions I'd like to ask. Are you available on IRC/discord/twitter somewhere we could chat a bit? |
|
Sure, |
This makes the library really usable in other packages
make sure cd .. happens irrespective of if the download and build line fails
…flag 'autodownload'
s1341
force-pushed
the
autoinstall_frida_devkits
branch
from
a911790
to
a06d7a4
Compare
|
Implemented your suggested changes. |
|
Thanks! |
This PR implements automatic download and installation of the neccessary frida devkits.
At the moment, the download_and_uncompress_devkit is copied to both frida-gum-sys/build.rs and frida-sys/build.rs, so that they are independent.
Tested and working.