5 ms·

> The takeaways here are that there is a real benefit to having an external/independent review and backporting process like the one we perform for our customers

> The takeaways here are that there is a real benefit to having an external/independent review and backporting process like the one we perform for our customers That's great for sales for these people, but I think the real takeaway here is: > when the actual merge of the tree was performed, no mention was made of [Linus's] correction to the [original] fix, and with no specific commit mentioning the correction and fixing it alone, everyone else's processes that depended on cherry-picking specific commits ended up grabbing the bad warning-inducing change. As a further failure, instead of looking at Linus' correct fix (observable by checking out the master tree at the time), the approach employed in the LTS kernels seems to have been to naively silence the warning There are a LOT of process failures in this description of what happened. At the kernel development level, that should be very concerning. It's good that an external auditor found it (hooray open source), but this was almost definitely an easily preventable mistake with a better process. > everyone else's processes that depended on cherry-picking specific commits This alone sounds horrifying. I don't know anything about kernel development, so I don't want to sit here and judge and offer advice out of ignorance, but I really do feel like there's a better process that would work for them that doesn't involve people "cherry-picking specific commits."

Dylan16807 7y ago
> the approach employed in the LTS kernels seems to have been to naively silence the warning I wouldn't even give it that much credit. Putting "line that assigns to var" after "line that uses var" is worse than 'naive'.
richardwhiuk 7y ago
Can you make a recommendation of a process which doesn't involve cherry-pick commits (or more often patch-sets)? Note, your process needs to allow a fix to be developed against V5.1, and back-ported to V4.1. I suspect one of the problems here is a lack of bug tracker, which would have associated the fix to the merge and the fix itself together.
- sdrothrock 7y ago
  > Can you make a recommendation of a process which doesn't involve cherry-pick commits (or more often patch-sets)? Thank you! You just made me realize that there some ambiguity in the original article and thus in my reply; I have nothing against cherry-pick in git and first thought the original article was using the phrase idiomatically, as in "developers picked specific commits with their personal judgment." The rest of this response will be written with that idiomatic reading in mind since that's what I originally intended with my criticism. > I suspect one of the problems here is a lack of bug tracker, which would have associated the fix to the merge and the fix itself together. I agree with this. This would make it immediately obvious that there were multiple fixes for one issue, which should be strange. I also think there should be an overall review process where both: 1. Commits that have been chosen are reviewed by the group 2. Commits that have NOT been chosen should be reviewed by the submitters -- if there's something critical being left out, this is the time to speak up They may already be doing something like this, for all I know, and there may be other reasons that this happened. Additionally, I'm only coming to this conclusion because of the way it was presented in the article; the people involved may have a different view of why this happened entirely. But for me, the way it was presented in the article, it definitely looks like a process failure.
  - Jach 7y ago
    You can only have so much process before you're too far out of the bazaar. This isn't to bring up the relative value tradeoffs of that old split, just to highlight that one shouldn't expect a 'process adjustment' reaction from a fundamentally anti-process collective or for members to even view it primarily through the lens of a process failure. (I'd sooner expect a rant on basic C coding competence even though those days are gone.) That's even if the LKML reflects on this particular timeline of patches much at all; there's no good reason to, really, it's a curiosity. And unlike those of us in a proprietary enterprise no one expects or can insist on an RCA doc with promises to do better backed by planned process tweaks (what issues can't ultimately be recast as process failures?) or new processes.
- the8472 7y ago
  Maybe linus' fix should have been added as separate commit to a branch instead of putting the fix in the merge commit itself? Then the entire branch could be taken and rebased, or at least the range cherry-picked.

Dylan16807 7y ago

> the approach employed in the LTS kernels seems to have been to naively silence the warning I wouldn't even give it that much credit. Putting "line that assigns to var" after "line that uses var" is worse than 'naive'.

richardwhiuk 7y ago

Can you make a recommendation of a process which doesn't involve cherry-pick commits (or more often patch-sets)? Note, your process needs to allow a fix to be developed against V5.1, and back-ported to V4.1. I suspect one of the problems here is a lack of bug tracker, which would have associated the fix to the merge and the fix itself together.

sdrothrock 7y ago
> Can you make a recommendation of a process which doesn't involve cherry-pick commits (or more often patch-sets)? Thank you! You just made me realize that there some ambiguity in the original article and thus in my reply; I have nothing against cherry-pick in git and first thought the original article was using the phrase idiomatically, as in "developers picked specific commits with their personal judgment." The rest of this response will be written with that idiomatic reading in mind since that's what I originally intended with my criticism. > I suspect one of the problems here is a lack of bug tracker, which would have associated the fix to the merge and the fix itself together. I agree with this. This would make it immediately obvious that there were multiple fixes for one issue, which should be strange. I also think there should be an overall review process where both: 1. Commits that have been chosen are reviewed by the group 2. Commits that have NOT been chosen should be reviewed by the submitters -- if there's something critical being left out, this is the time to speak up They may already be doing something like this, for all I know, and there may be other reasons that this happened. Additionally, I'm only coming to this conclusion because of the way it was presented in the article; the people involved may have a different view of why this happened entirely. But for me, the way it was presented in the article, it definitely looks like a process failure.
- Jach 7y ago
  You can only have so much process before you're too far out of the bazaar. This isn't to bring up the relative value tradeoffs of that old split, just to highlight that one shouldn't expect a 'process adjustment' reaction from a fundamentally anti-process collective or for members to even view it primarily through the lens of a process failure. (I'd sooner expect a rant on basic C coding competence even though those days are gone.) That's even if the LKML reflects on this particular timeline of patches much at all; there's no good reason to, really, it's a curiosity. And unlike those of us in a proprietary enterprise no one expects or can insist on an RCA doc with promises to do better backed by planned process tweaks (what issues can't ultimately be recast as process failures?) or new processes.
the8472 7y ago
Maybe linus' fix should have been added as separate commit to a branch instead of putting the fix in the merge commit itself? Then the entire branch could be taken and rebased, or at least the range cherry-picked.