Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Add more maintainers #2089

Closed
XVilka opened this issue Jul 18, 2023 · 8 comments
Closed

Add more maintainers #2089

XVilka opened this issue Jul 18, 2023 · 8 comments

Comments

@XVilka
Copy link
Contributor

XVilka commented Jul 18, 2023

It's obviously clear that current maintainers struggle reacting timely to PRs and issues. I propose to extend the team.

@aquynh @kabeor

@thestr4ng3r
Copy link
Contributor

thestr4ng3r commented Jul 18, 2023

Fully agree with this. As it appears, the intended goal of capstone is to provide a full-featured, stable and up to date disassembler library for the most common architectures. But this has been shown to be impossible with the current maintenance strategy.

As a concrete example, in 2018, Apple started to use PAC on iOS and in 2020 switched the primary architecture for macOS to arm64 with all binaries from Apple including the kernel and dyldcache, both highly important to be able to disassemble for security research, full of PAC instructions.
Only until the very recent release of capstone 5, meaning for 5 years, the latest release of capstone disassembled these very common binaries to garbage and we were forced to point to unreleased development branches instead, which naturally no Linux distribution will like to ship. As a consequence, radare2 has switched to the vector35 library for arm and others to zydis for x86 while capstone was bitrotting away.

Adding @kabeor to the team greatly improved the situation, as shown by the aforementioned capstone 5 release. But cs5 still contains major regressions that only appear now that many projects are switching to it, due to a lack of continuous testing, so it is not enough to stop here.
And as the (almost non-existent) review process in #1949 shows, the current team still is not up to the task, albeit for very understandable personal time management reasons. This shows that extending the team is necessary for the project to move forward in any sustainable way.

@kabeor
Copy link
Member

kabeor commented Jul 18, 2023

Hi guys, you know what? I am really do my best to push the whole contribution process. You can see that I wanted to release v5 last year. However, every open source community has its own most carefully things. For example, Capstone cares about stability and compatibility. So how to balance this with new features? The answer is more closer code review. Honestly, I really welcome more brilliant people like you guys can join the maintenance team. But how to control the code quality by multi maintainers? I hope we can talk about this here. Besides, @aquynh has some unique ideas or thoughts. All in all, I am always thinking how to balance these all at the same time? If there's any suggestion, I appreciate a lot.

By the way, I asked @aquynh this morning and he said he will give some feedbacks to #1949 today night. Or I will ask again.

@Rot127
Copy link
Collaborator

Rot127 commented Jul 18, 2023

@kabeor

I am really do my best to push the whole contribution process.

We do very much appreciate this!

The answer is more closer code review. [...] But how to control the code quality by multi maintainers?

I suggest to add more (strict) guidelines and enforce them with the CI. E.g. adding linter checks and proper testing to the CI. This way each contribute can directly see on their own if something breaks the current state. Without any interaction from maintainers.

Reviewers then only must check for things things which require deeper code base knowledge.
If conflicts appear regularly (about maintainer decisions), they can be decided once and added to a CONTRIBUTING.md.

Capstone cares about stability and compatibility. So how to balance this with new features?

The following is a little offtopic. Feel free to move it to another issue.

The auto-sync changes are a move to more stability IMHO. Capstone behaved in many places not like LLVM (see all those "LLVM disassembles it, CS doesn't" or "missing r/w attributes" bugs).
I think this happened because initially CS was build within 7 months and many "edits by hand to get it done" things were made (which is very understandable). Though this backfires now because CS cannot be updated.

auto-sync adds barely any new features but makes changes, so CS can be updated and stay stable after every LLVM release. Additionally we get the ability to add more features easily (e.g. #2045)

Capstone might was build with stability in mind (which is great), but the non-auto-sync way of updating just doesn't allow stability and up-to-dateness in the long run.

But those changes need more maintainers as well. Just to get all archs as quickly as possible in sync with LLVM (if possible).

@XVilka
Copy link
Contributor Author

XVilka commented Aug 9, 2023

Any decision on this yet?

@XVilka
Copy link
Contributor Author

XVilka commented Aug 19, 2023

@kabeor @aquynh ping

@XVilka
Copy link
Contributor Author

XVilka commented Aug 28, 2023

@kabeor @aquynh ping

@XVilka
Copy link
Contributor Author

XVilka commented Sep 4, 2023

@kabeor @aquynh any decision yet?

@Rot127
Copy link
Collaborator

Rot127 commented Jan 3, 2024

@kabeor @aquynh Any news in this matter?

@capstone-engine capstone-engine locked and limited conversation to collaborators Mar 20, 2024
@Rot127 Rot127 converted this issue into discussion #2293 Mar 20, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants