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

ghidra: refactor #60664

Closed

Conversation

deliciouslytyped
Copy link
Contributor

@deliciouslytyped deliciouslytyped commented May 1, 2019

Added a plugins system, launcher generator, eclipse plugin.

Things done
  • Tested using sandboxing
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@deliciouslytyped
Copy link
Contributor Author

deliciouslytyped commented May 1, 2019

I'd like to think this is well organized and commented, but in any case, it's probably best to start reading through the code from default.nix and follow the references.

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.
Copy link
Member

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
Copy link
Member

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.

Copy link
Contributor Author

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
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Member

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
Copy link
Member

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?

Copy link
Contributor Author

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?

@deliciouslytyped
Copy link
Contributor Author

deliciouslytyped commented May 2, 2019

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.

@deliciouslytyped
Copy link
Contributor Author

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?

@deliciouslytyped
Copy link
Contributor Author

deliciouslytyped commented May 13, 2019

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.

@deliciouslytyped
Copy link
Contributor Author

deliciouslytyped commented May 13, 2019

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 name attribute with -bin, and fixed a typo in headless-01.nix ./nixpkgs -> ./nixpkgs.nix from the previous force push. (I should run tests automatically before pushing...)

Added a plugins system, launcher generator, eclipse plugin package.
@deliciouslytyped
Copy link
Contributor Author

deliciouslytyped commented Jun 11, 2019

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

@deliciouslytyped deliciouslytyped mentioned this pull request Jun 28, 2019
2 tasks
@deliciouslytyped
Copy link
Contributor Author

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.

@nomeata
Copy link
Contributor

nomeata commented Feb 24, 2020

According to NationalSecurityAgency/ghidra#1 the way to configure Ghidra for high-res displays is to edit

/nix/store/fn9y7vc900hwq9qhmjj0zb7kxx5swydd-ghidra-9.1/lib/ghidra/support/launch.properties

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?

@deliciouslytyped
Copy link
Contributor Author

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.

@stale
Copy link

stale bot commented Aug 22, 2020

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.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 22, 2020
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 3, 2020
@stale
Copy link

stale bot commented Jun 6, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 6, 2021
@AndersonTorres
Copy link
Member

Closing as dead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants