[Swan-dev] On re-applying "pluto: warn if loaded connection ended up unoriented" et.al.

[Andrew Cagney]

On Tue, 23 Jan 2024 at 10:44, Paul Wouters <paul at nohats.ca> wrote:
>
> On Mon, 22 Jan 2024, Andrew Cagney wrote:
>
> >> Also, please use separate commits for code and test cases in the future.
> >
> > Except this wasn't my mess.
> >
> > I was dealing with a commit that, once it became clear was broken,
> > should have been quickly reverted, followed by an incomplete trickle
> > of test changes.
>
> I don't see why. Whoever is working on the bug can work on 2af2e7f6^

I'm not sure how to read this.

Perhaps you mean other developers.

Even if code is good based on the commit before a breaking change
(e888dd156  c0d4e4f1a a7b680693
3be6424fb 8116a4939) that doesn't mean it is good when it is merged.
So either other developers wander off and go skiing or merge
regardless.  The latter has the real risk of causing further
regressions.  Neither is productive.

Perhaps you mean the original developer.

That's what branches are for - creating branches is cheap so create
one and ask for help.  Hopefully one of our many volunteers will step
in and lend a hand.    Admittedly when this project was started it
used CVS so branching and reverting were both a PITA, but that was
long ago.  Now-a-days branching, merging, reverting, and keeping
mainline stable are all considered best practices.

> Removing it just risks the bug remaining and the issue being unsolved
> and appearing later in less clear circumstances only for some users
> in a released version, causing us to issue a CVE. I'd rather see half
> the test cases explode and us focussing on resolving the bug.

I wonder if there's a rule that says the moment anyone uses CVE to
defend their position the debate over.

We have a testsuite (it just hit 1000 tests),
We have a bug database,
We have a release tracker.
We have weekly discussions to review bugs and decide what still needs
to be fixed.

So we'd have to really screw up to drop the ball on this one.  We're
not exactly going to attract and keep new developers if they find that
they keep being voluntold.

> Regardless, the code commit and the test case changes could still
> appear in two separate commits.
>
> > there's one test and one code change _and_ the pair score a 100% clean
> > bill of health from a test run:
>
> It's a "clean bill of health" that actually brushed the real problem of
> abuse of RC_LOG_SERIOUS side effects under the carpet. How many CVEs
> could that lead to if we shipped with the RC_LOG_SERIOUS -> RC_LOG
> change? How long until another RC_LOG_SERIOUS crasher is accidentally
> introduced and not caught by a testcase for another CVE?

Except:

- this isn't a crasher
- this change is only nice-to-have not must-have
- if it had been logged at the correct level - RC_LOG, or merged with
the "IKEv. connection added" line, we'd not even be having this
discussion