-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update to Ghidra 10.1.3 #18
Conversation
Started test build 82596 |
I can hazard a guess and say that it won't build. If you have tested the patch locally though, I would be grateful if you could file a PR for it upstream so it gets in the next point release, which we could then pick up. |
I've tested that locally. And it works. Really. |
I'll find another way to resolve this network issue. |
Ideally gradle support would be added to the flatpak-builder-tools: |
Build 82596 failed |
Ok, so I thought that removing We could download all dependences that Gradle needs and put them in the cache. But this is a very hacky method. |
Started test build 83473 |
I did found a more pleasant way to download dependencies using Flatpak Builder and then build with |
Build 83473 successful
|
Sounds like the build works fine! |
It will need reviewing. As a quick review, given the number of changes, the changes would need to be split up in as many commits as it makes sense, first adding the json files with the dependencies (and explaining how they were generated), updating the build system, adding the missing releases, etc. all in small, trivial, commits so that the review is easier. Thanks for working on that! |
Ok. I'll try to make that. |
Started test build 83525 |
Build 83525 successful
|
I split this in multiples commits. Just ping me if you don't understand something. |
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.
Quite a few nits to fix before this can be merged, but apart from "how to generate the gradle deps", it's just git surgery. Good job.
Started test build 84486 |
Forgot to mention NationalSecurityAgency/ghidra#4107 |
Build 84486 successful
|
Started test build 84488 |
Build 84488 successful
|
@hadess This has been a long time. What's happening? Do I need to do something else? |
I've been waiting for your updates on my review comments since the beginning of April, in particular, this one: Once you've reviewed all the comments, and as you fix them, please make sure to mark the threads as "resolved" along with a comment explaining how you fixed it (eg. whether you modified an existing commit, dropped a patch, or something else). |
@hadess Oh, ok. I did forget to mark them as resolved. Sorry, I'm not an expert in Git. I did resolve them all. Also, you should know that I will still be there after this merge. So in case of any future problem with dependencies I can help. You just need to contact me. Tell me if there is something else to do. |
I'll look at this on Monday, but I'm still not happy with the way |
I don't think there is any better way to generate this. No one is providing such script: |
Started test build 90953 |
Build 90953 successful
|
Those are dependencies that wouldn't be automatically downloaded by running Gradle. They're listed at: https://github.com/NationalSecurityAgency/ghidra/blob/master/DevGuide.md#manual-download-instructions
File was generated by using flatpak-gradle-generator.py in: flatpak/flatpak-builder-tools#276 And generate-deps.sh
Instead of repackaging binaries. This will allow us to add patches as necessary. Build instructions are listed at: https://github.com/NationalSecurityAgency/ghidra/blob/master/DevGuide.md#building-ghidra
Use cp instead of install. Since we are building Ghidra from source, only Linux platform files are generated, so we can copy the entire directory.
Started test build 90981 |
Started test build 90983 |
I've reworked the commit messages and contents to my liking. I'll wait for the last 10.1.3 build to finish and will run a quick test. If all that passes, I'll push all this. Thanks very much for your help in getting all this updated. |
Build 90981 successful
|
Build 90983 successful
|
Stops it from clobbering the stable release if installed.
Started test build 90988 |
Build 90988 successful
|
Thanks |
After spending some time updating this Flatpak, I've managed to make it work with the latest Ghidra version.
This should solve #6, #12, #14, #15, #16, and also NationalSecurityAgency/ghidra#3726.
I changed many things: For exemple Ghidra is installed in
/app/ghidra
. I removedghidra.properties
options. Also I usecp
instead ofinstall
. Fell free to revert those changes. I've just cleaned all those things to make it work in a first place. But now that it works...