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-extensions.ret-sync: init at unstable-2024-05-29 #315672

Merged
merged 4 commits into from
Aug 9, 2024

Conversation

vringar
Copy link
Contributor

@vringar vringar commented May 29, 2024

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@vringar
Copy link
Contributor Author

vringar commented May 29, 2024

In addition to installing the extension, it also has to be enabled on a per project level but idk where to document that.

@vringar
Copy link
Contributor Author

vringar commented Jun 4, 2024

@NixOS/nixpkgs-merge-bot merge

@nixpkgs-merge-bot
Copy link
Contributor

@vringar merge not permitted (#305350):
pkgs/tools/security/ghidra/extensions.nix is not in pkgs/by-name/
pkgs/tools/security/ghidra/extensions/ret-sync/default.nix is not in pkgs/by-name/

@vringar vringar requested a review from emilytrau June 4, 2024 17:59
@vringar
Copy link
Contributor Author

vringar commented Jun 4, 2024

Sorry for requesting you review even though you clearly stated that you didn't want to be a maintainer. I was just hoping that you might have some input on how to make this PR auto-mergeable.
I have a couple of other extensions that I would like to land in the future and thought I could do so independently, but I guess since they are new packages that doesn't work (? assumption on my part). Should I wait for all of them to be ready and get them landed as one PR?

@Mic92
Copy link
Member

Mic92 commented Jun 5, 2024

I don't think we will allow new packages to be auto-mergeable ever because we need to know about the maintainer before we can allow a package to be merged.

@Mic92
Copy link
Member

Mic92 commented Jun 5, 2024

I used ghidra but never with extension. How can I test this?

@vringar
Copy link
Contributor Author

vringar commented Jun 5, 2024

We expose the ghidra.withExtensions function that builds Ghidra and all selected plugins and links them together.
The buildGhidraExtension also always adds me as a maintainer, so that guarantees that all new extensions will have a maintainer.

Copy link
Member

@emilytrau emilytrau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work!

@vringar
Copy link
Contributor Author

vringar commented Aug 1, 2024

Just ran nixpkgs-review pr 315672 and realized that this PR requires #329272 as the build script in its current form is not very usable

@vringar vringar force-pushed the ghidra-ret-sync branch 3 times, most recently from beac81e to b6f1fe7 Compare August 1, 2024 11:41
@vringar
Copy link
Contributor Author

vringar commented Aug 5, 2024

PR is mergable again

@emilytrau
Copy link
Member

I've pushed up some changes

  • Got these extensions to build on darwin with some compatibility changes to buildGhidraExtension
  • Removed use of mitmCache as these extensions do not need to download dependencies

Thanks again, hope to have this merged soon :)

@emilytrau emilytrau merged commit 54cdfe9 into NixOS:master Aug 9, 2024
26 of 27 checks passed
@vringar vringar deleted the ghidra-ret-sync branch August 11, 2024 15:26
@vringar vringar mentioned this pull request Sep 21, 2024
13 tasks
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.

3 participants