when we broke the TLS patch, we didn't notice immediately

[schmonz]

Sometime after 1.08, the TLS patch stopped applying to our tree. It's good we noticed this now so we can repair the situation before our next release. Some options:

1. Prepare a TLS patch rebased onto 1.09
2. Roll back our breaking changes and submit them as new PR(s) that document our known preconditions for merging
3. Coordinate with the TLS patch maintainer to publish a timely new version for notqmail 1.09 when we're ready to release

As usual, I'm extremely not excited about (1). We haven't maintained our rebased patches particularly well, and I don't blame us: they're easy to forget about, and if we do remember, the work isn't any fun. And users who are considering notqmail -- or already using notqmail and trying to track updates -- will much prefer being able to apply the patches they already have and know well. (I'm sure there'll be cases where we can't avoid providing rebased versions of known patches. Still, the fewer the better.)

(3) would be a pretty good outcome: minimal work for us, and users have already been trained to seek official updates to that patch over the years. If I were maintainer of a popular qmail patch, I'm not sure I'd be willing to make the effort to do this. But we can certainly ask. If he is willing, the tradeoff here is that we've made the usability of our release depend on a third party. What if he changes his mind, or simply has no free time when we need him to?

My preference is for (2), especially since our breaking changes in this case appear to be small. For instance, we need to roll back calls to the safe{read,write} generator macros in qmail-{remote,smtpd}.c and restore the open-coded functions so that the TLS patch can modify them.

---

Once we've preserved TLS for 1.09, we need to solve the meta-problem: in our current dev process, important patches can break and we might not know until too late.

@josuah's auto-patcherator is awesome, but we have to remember to look, and it seems to be the case that we haven't remembered. Again, I don't blame us. Our PR flow is already somewhat complicated.

Before we release 1.09, we need an automated way to be notified promptly that we broke a patch we intended not to break, so that we're sure not to release 1.09 with unintended breakage.

The cheapest time to avoid unintentional breakage, of course, is before merging a PR. But we already have to wait a while for the CI builds we've got. If that gets a lot longer, it'll be really tedious. The next-cheapest time to find out we broke something, then, is shortly after the merge.

The proper response to a patch-breakage notification is not necessarily "block the merge" (or "roll it back"), just like it might not be in the present TLS case. My goal here is not that we never decide to break an agreed-important patch. It's that when we make such a decision we're aware of it, we've considered our options, and we think breaking that patch is our smartest available move.

Some ways this notification could be implemented:

• Pre-merge check and/or post-merge "nightly build"
• Running at GitHub and/or on OS package autobuilders (like Gentoo, which is how we discovered the current breakage)
• Email to the dev list

We'll also need to agree on a short list of patches we want to never be surprised about.

[DerDakon]

These are the things that break:

• Makefile: we removed now.o, which causes context errors because this affects the lines just above or below what the patch wants to modify
• hier.c: the patch wants to add it's update_tmprsadh just at the place where the pinq and stuff scripts were that we deleted
• qmail-smtpd.c and qmail-remote.c: the GEN_SAFE_TIMEOUTREAD and GEN_SAFE_TIMEOUTWRITE functions collide with the patch trying to modify the original functions. One would just have to use these functions again, and fix them to use ssize_t and proper error checking
• ssl_timeoutio.c uses datetime_sec, but does not include datetime.h. Before that header was included in now.h, which was removed when we changed now() to return time_t (I already reported that to Frederik)

[DerDakon]

Btw, I will rebase the patch on 1.09 anyway to use it in Gentoo.

[schmonz]

Expecting to need to do the same with tlsonlyremote for pkgsrc. Users of our packages almost certainly don't mind (or know, or care ;-) that we do that for them.

[josuah]

Noticing breakages: A proposition for pre-merge check is running make brokemaster from this repo https://github.com/josuah/notqmail-patch/ (description on the repo). Does it make sense to have a more official https://github.com/notqmail/notqmail-patch/ that anybody can maintain?

Maintaining patch compatiblity: is offering users to send us the patch they need us to maintain an option? This could be easier than having line of code acting like heavy stones through the project due to being pointed at by patches.

TLS patch for qmail: will try to bissect using notqmail-patch

[DerDakon]

The breakage comes e.g. from the pinq removal, see my description of the collisions above.

[josuah]

Even though this new branch is based off master and not notqmail-1.09, it permitted to build a new patch that works on master.

[schmonz]

Got a report from IRC that 1.08 "broke a bunch of patches". Noticed and reported now because (if I'm understanding the report correctly) Gmail seems to have just started enforcing SPF or DKIM policy for incoming plaintext connections, so it's more important now for qmail-remote to negotiate TLS.