Hi all,
On 14.10.22 03:53, Anastasia Klimchuk wrote:
A quick answer first: We are closer to the release, and the state of the master is better than a half year ago.
hmmm, I may not have made myself clear with my question. I didn't mean to ask for an overall state but wrt. stability (iow. absence of code flaws) in particular. And if/how we can trust that there are no or few enough flaws.
More details. In general, the progress is visible here https://ticket.coreboot.org/issues/353
Yes, this is the progress of tickets. But how to put this... I know the state as far as we measure it. What I'm concerned about is what we don't measure.
Maybe this helps to understand the idea of the release process better: In the Development Guidelines[1] it says
"Branching for a new release can happen at any point in time when a commit (branch point) on master seems to be in good shape and was reasonably tested after previous invasive changes."
The last part tries to protect us from very new regressions / issues that nobody found (measured) yet. And this was written in a time when flashrom builds from the master branch were much more tested. We could tell people that it's safe to use the master branch (which it isn't anymore). And even the people who used releases implicitly tested a state of the code for us that was much closer to master.
Also, what led to the long list in #353 was a development process that created issues faster than they were fixed. Many of the issues were only made visible by a re-review. Can we say that the ratio of creating and fixing issues is more balanced now? How can we trust that if we'd re-review again, we wouldn't end up with an even longer list?
There is one issue that came up around May https://ticket.coreboot.org/issues/390 . This one is new. It does not prevent from using any functionality, and also if the user does not specify `--progress` they won’t even notice.
Can the latter be proved? I'd say yes, but it seems like more effort then re-writing the whole patch. I never fully reviewed the patch but in the few places I looked into I saw wrong calculations and overflows. And those run independently from the `--progress` switch. And please remember we are talking about C code. To say that it doesn't affect existing functionality would require a proof of defined behavior. Because if there is undefined behavior, flashrom could just crash.
Also worth to remember: flashrom often runs with root privileges. So every coding mistake is a security issue unless we can prove that it isn't.
Lots of code improvements: for example we have way less global state than it used to be 1/2yr ago, and we have more unit tests of all types.
The former is in the category of "invasive changes" that I'd like to avoid before a release. I see a lot of refactorings in the name of less global state. IMO that's a nice long-term goal, but shouldn't stall the project. Just very recently, there was a patch train that unnecessarily introduced a lot of NULL pointers (I commented how to do it without and that we'd have to be careful otherwise). Things were merged (without being careful), reverted, fixed up (I warned about NULL pointers again), merged again (again without looking for NULL pointers left), fixed up again. How can we know now that it's settled? How to trust such a development process?
(This case might actually be why I started to wonder how the measured progress relates to the actual progress on the master branch.)
If you ask me to pick what’s very important to fix I would say https://ticket.coreboot.org/issues/370 I remember several patches on that, from different people. Potentially, the bug is [technically] fixed, but we need to coordinate who is rebasing on the top of whom, and complete the review.
The story is much more complex. Every time somebody looks into it, more even older issues pop up. I'd say revert to the state of the last release, and then after a new release, fix remaining issues and only then add new things and the refactorings again. IMO this driver was patched to a dead end, or at least a state where the easiest way for- ward is to go a few steps back first. Last time Edward drew attention to it, I really tried to find a solution. But even comparing the state right after the problematic patch to the current state was too frus- trating: Due to all the refactorings I couldn't see (within reasonable time) what changed in terms of functionality.
Maybe we should have a rule for this? "If there are any known issues within a file or area of the code, no refactoring must take place until they are fixed."?
Nico
[1] https://www.flashrom.org/Development_Guidelines#Release_branches_(e.g._1.0.x)