-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
ghidra: refactor #60664
ghidra: refactor #60664
Conversation
I'd like to think this is well organized I'll add additional documentation to the manual later, I just wanted to get this out the door. There's tests in the doc folder that can serve as usage examples in the interim. |
Overriding | ||
---------- | ||
For the moment, the API documentation is the tests. | ||
Documentation will be added in at most a few weeks. |
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.
Famous last words.
# should stop in a breakpoint in the plugin source code | ||
# and display the appropriate source location. | ||
|
||
#TODO not yet implemented |
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.
I'd say either implement this right away or do not check in this stubbed test for now.
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.
Giving up on this for the moment, possible starting points:
https://testing.bredex.de/test-machine-setup.html https://www.eclipse.org/rcptt/
I have no idea how to work any of this stuff.
]; | ||
|
||
propagatedBuildInputs = [ | ||
jdk |
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.
Should jdk really be propagated here?
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.
Shouldn't it? I think I wrote this when I wasn't so good at nix.
I guess at the time this was providing me with the setup hook, which I now call manually in jdkWrapper, making this unnecessary?
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.
Yes. I don't see why derivations that receive ghidra as build input also need access to the same jdk binary. The wrapper ensures that java can be found at runtime.
jdkWrapper = src: dst: '' | ||
makeWrapper "$out/${src}" "$out/${dst}" \ | ||
--prefix PATH : '${self.pkgs.lib.makeBinPath [ self.jdk ]}' \ | ||
--run '. ${self.jdk}/nix-support/setup-hook' #set JAVA_HOME |
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.
We not setting JAVA_HOME directly?
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.
I wasn't really sure what to do about this. It makes some kind of sense to me to delegate setting whatever environment stuff a dependency implies, by relying on the dependency's scripts?
Yeah, I've been meaning to update, and do the build from source, but I haven't had the time / spare cycles to think about it. Thanks for the nudge, I'll try to up the priority on that. |
So I just tried to throw together a minimal build from source and the first thing I ran into was some problem about missing Jython. This is going to be fun... (Maybe I did something wrong with the Gradle init config thing...). Maybe it's easier to build the full package... I know basically nothing about Gradle and only want to learn just enough to get this to work. @artemist I don't suppose you've tried any source builds? |
5947575
to
f56a269
Compare
I just did a force push switching the "with inherit" expressions in the tests to have the inherit inside the "let"s, as well as fixing the nixpkgs import path in the tests. Edit: looks like I missed one. |
f56a269
to
ff62ba7
Compare
Edit: did the force push after the comment Force pushed update to 9.0.2. TODO: I have not thoroughly checked for breakage and at least mkRunline probably needs to be updated to support the new (ip address + port) debug config scheme as opposed to just the port. I have also suffixed the |
Added a plugins system, launcher generator, eclipse plugin package.
ff62ba7
to
a1f6aac
Compare
Has anyone else actually tried using this? On a sidenote, I have some WIP infrastructure at https://github.com/deliciouslytyped/nix-ghidra-wip using a factored (WIP) out library: https://github.com/deliciouslytyped/nix-rootedoverlay |
I've updated both repositories. Things seem to be functioning sanely. I'm still working on cleaning up rootedoverlay, but the main thing remaining for rootedoverlay is really just documentation and tests. The core functionality is there and it appears to work. It's purpose is to abstract out functionality for handling plugin sets. The next step for the Ghidra stuff would probably (still) be a source build. I don't see how I could decrease the amount of code / simplify further without removing functionality. |
According to NationalSecurityAgency/ghidra#1 the way to configure Ghidra for high-res displays is to edit
Now, on nix I surely shouldn’t edit that fiile… does this PR add support for overriding these properties, or should I open a separate issue for that? |
I haven't had time to work on this for a while. Off the top of my head, I don't think I had anything for that. |
Hello, I'm a bot and I thank you in the name of the community for your contributions. Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human. If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do: If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list. If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past. If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments. Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel. |
I marked this as stale due to inactivity. → More info |
Closing as dead. |
Added a plugins system, launcher generator, eclipse plugin.
Things done
nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)