How far can external contribution to Ghidra actually go?

[UnsleptArch]

What's up, I got a question for the maintainers and anyone else who's been doing this a while.

I've spent a chunk of the last few days going through the PR history trying to understand how contribution actually works here, mainly to make my own PR's better and to be as beneficial to ghidra as possible (as well as out of sheer boredom at my classes). A big few things came to light out of that, that got me curious about the actual ceiling on external contribution:

• 392 people show up in the GitHub contributor graph, Accepted external PRs get pulled onto an internal GP-####_<maintainer>_PR-<n>_<you>_<slug> branch and land in master with your name and email intact in the git author field, closing your PR with a grey closed icon instead of a merge badge. So the real acceptance rate when you also count that, is 55%, not the ~40%ish you'd get from only counting GitHub-merged PRs.
• Some processor areas (SuperH, tricore, PowerPC, x86, ARM) sit at 70-88% acceptance for outside contributors. Decompiler is the outlier at 36%.
• Two or three devs seem to be in charge of the PR's accounting for ~70% in total

None of that seems to indicate whether there's an actual path from "external contributor whose PRs keep being accepted" to a larger general role in Ghidra, however i don't know the inner happenings of ghidra so i may be mistaken. Commit access, getting looped into design discussions before code exists, an invite into whatever the internal review process looks like, or even just being able to send large PR's without the multi week / month timeline. Or is it just staying an "external" contributor forever no matter the volume or quality or impact of the work you do. Is there a known case of someone crossing that line without an NSA employment offer? Or is "keep sending good PRs to the two people who ingest them and wait for long periods of time while they have to comb through hundreds per day" actually the ceiling, and that's just how it works here in ghidra? I also realize that this may seem like a criticism in the Ghidra PR process, and I want to assure you that it's not it, i highly respect all of ghidra's devs and their decisions and also realize that they are much more experienced then me in this field, Im just curious on the current state of external help for ghidra.

-Best regards Unslept

[dragonmacher]

I will speak on how I personally view the current PR support.

• In general, unsolicited PRs are unlikely to get any real attention
• We have a history of making exceptions for very small PRs for minor bug fixes or documentation issues
• Some larger PRs have been accepted, but typically will be heavily rewritten
• We have not yet developed any mechanism for creating a list of 'vetted' public contributors whose PRs would be more likely to be approved
• We have a few devs on the team that stay on top of Github issues and PRs. We also have devs on the team that rarely check on Github items

In my experience most public contributors do not have the experience with out code to build solutions that can be accepted without major changes. It is easier for us to make the changes directly, rather than going through the review process.

Admittedly, handling PRs in this way is not allowing us to tap into a sea of valuable brain power. Further, I image that most gitbub projects are much better at managing contributions.

[0xFE4415]

Got it. One follow-up: are there plans to have GitHub Actions run the test suite as well?
It currently seems to only run the build.

I'm wondering if there's a plan for GH Actions to eventually cover broader CI validation, so contributors and maintainers can see whether the changes are fine directly on GitHub.

[ryanmkurtz]

If we had some relatively quick running tests, I'd be in favor of a GitHub Action that runs on all PR's. We do not currently have those though.

[jobermayr]

I have a question about this commit.

[...] If you have a need for an updated library, please submit an issue with your request instead of a pull request.
[...]
- Please do not include the issue number that is being fixed in the pull request title field. 
   Instead, use "Fixes #xxx" somewhere in the pull request description text box.

I assume esp. AI reads it without the if clause and "instead" in all cases. Maybe it should be reworded to avoid duplicates (issue and following PR from same "person" a few seconds later).

[0xFE4415]

If we had some relatively quick running tests, I'd be in favor of a GitHub Action that runs on all PR's. We do not currently have those though.

I have some proposals then:

1. As outlined by OP (emphasis mine): "Some processor areas [...] sit at 70-88% acceptance for outside contributors. Decompiler is the outlier at 36%." and it has a quick test suite, decomp_test_dbg/decomp_test_opt .
2. The DevGuide separates the tests into unit tests and "more complex integration tests." Perhaps the gradle unitTestReport subset would be quick enough to be viable for running in the GitHub Actions pipeline?
3. Add an explicit note that GitHub Actions does not run the full test pipeline and that those tests are run internally by the Ghidra team. This would help manage expectations, i.e., GH Actions pipeline pass does not imply full test suite pass.

[UnsleptArch]

@0xFE4415 Yeah that sounds like a great idea, I also think a more robust dev guide in general would be good to deliver better PR's from the community.

[ABratovic]

• Decompiler is the outlier at 36%.

waiting until the blackhole turns into a Quasar to spit out all requested improvements

This has been an illuminating discussion. One thing I've observed about @caheckman is that any decompiler crashes (eg #9537) are resolved within 2-4 weeks of being reported which I think is an awesome response time.