Re: [PATCH] Documentation: add platform support policy

From: Kyle Lippincott <spectral@google.com>
Date: Jul 11, 2024, 20:25 UTC
Message-ID: <CAO_smVjpsVZj29s9dKQNy8Jos8HvxmRcc4S25WmngGb2HQB88w@mail.gmail.com>
In-Reply-To: <CAJoAoZmo6ey1RAqgtM098LLLrdKODf2OYyV=YqyFH5VXiSB6RQ@mail.gmail.com>

On Thu, Jul 11, 2024 at 11:37 AM Emily Shaffer <nasamuffin@google.com> wrote:

Show 58 quoted lines

>
> On Wed, Jul 10, 2024 at 12:11 PM Kyle Lippincott <spectral@google.com> wrote:
> >
> > On Tue, Jul 9, 2024 at 3:50 PM Emily Shaffer <emilyshaffer@google.com> wrote:
> > > +* If you rely on some configuration or behavior, add a test for it. You may
> > > +find it easier to add a unit test ensuring the behavior you need than to add an
> > > +integration test; either one works. Untested behavior is subject to breakage at
> > > +any time.
> >
> > Should we state that we reserve the right to reject these tests if
> > they would put what we feel is an excessive burden on the git
> > developers? i.e. a requirement to use C89 would be rejected
> > (obviously). a requirement to support 16-bit platforms would also be
> > rejected. I don't know that we need to list examples for what we'd
> > reject, they could be implied that we're likely to accept anything
> > else.
>
> brian mentioned something similar in their review, to which I proposed
> a minimum requirements field[1]; I think this would work for that too?
> Thoughts?
>
> > > +** Clearly label these tests as necessary for platform compatibility. Add them
> > > +to an isolated compatibility-related test suite, like a new t* file or unit test
> > > +suite, so that they're easy to remove when compatibility is no longer required.
> > > +If the specific compatibility need is gated behind an issue with another
> > > +project, link to documentation of that issue (like a bug or email thread) to
> > > +make it easier to tell when that compatibility need goes away.
> >
> > I think that we likely won't have the ability to investigate whether
> > it's _truly_ gone away ourselves, and there's no guarantee that the
> > person that added these tests will be able to vet it either (maybe
> > they've switched jobs, for example).
> >
> > I think we should take a stance that may be considered hostile, but I
> > can't really think of a better one:
> > - there needs to be a regular (6 month? 12 month? no longer than 12
> > month surely...) reaffirmation by the interested party that this is
> > still a requirement for them. This comes in the form of updating a
> > date in the codebase, not just a message on the list. If this
> > reaffirmation does not happen, we are allowed to assume that this is
> > not needed anymore and remove the test that's binding us to supporting
> > that.
>
> I like the idea of the date in code, because that's "polled" rather
> than "pushed" - when I break the test, I can go look at it, and see a
> condition like "Until July 1st, 2024" and say "well it's July 11th, so
> I don't care, deleted!" - or, more likely, I can see that date expired
> a year and a half ago :)
>
> > We should probably go a step further and intentionally violate
> > the test condition, so that any builds done by the interested parties
> > break immediately (which should be caught by the other processes
> > documented here; if they don't break, then it was correct to remove
> > the restriction).
>
> ...but this seems harder to keep track of. Where are we remembering
> these "due dates" and remembering to break them on purpose? I'm not
> sure that there's a good way to enforce this.

Ah, I was unclear - I was saying when we remove the test, we should intentionally add a use of the thing the test was preventing us from using. So if there's a test saying "you can't use printf()", when removing that test, we should add a use of printf in the same series. I was intentionally being vague about "when" we should remove these tests, and how long we should consider the newly added printf provisional (and possibly going to be rolled back), like in our current test balloons. Just: if you're removing an expired platform-support-policy test, add something to the codebase (possibly just another test) that exercises the previously forbidden aspect, if possible.

This is likely to happen organically: most people aren't going to notice an expired platform-support-policy test and remove it just because. They'll trip over it while attempting to work on a feature, notice it's expired, and remove it as part of the series that makes use of it. But if someone is trying to be helpful and clean up expired platform-support-policy tests without having the goal of immediately using it, they should be encouraged to invert the test so that we don't have potentially months of time difference between "implicitly allowed again [since there's no test preventing its use]" and the actual usage being added.

Show 19 quoted lines

>
> > - _most_ of these restrictions should probably have a limited number
> > of reaffirmations? I feel like this needs to be handled on a
> > case-by-case basis, but I want us to be clear that just because we
> > accepted these restrictions in the past doesn't mean we will accept
> > them indefinitely.
> > - Just because there's a reaffirmation doesn't mean we're guaranteeing
> > we won't delete the test before the affirmation "expires". If there's
> > an urgent security need to do something, and it can't be done without
> > breaking this, we'll choose to break this test. If there's general
> > consensus to do something (adopt a new language standard version, for
> > example), there's no guarantee we'll wait until all the existing
> > affirmations expire.
>
> I honestly like this point more than the previous one. Limiting by a
> number, even one that changes, feels less flexible than allowing
> ourselves to say "enough of this nonsense, it's the Century of the
> Fruitbat, we really want to use <C11 feature> so you can get
> maintenance updates instead now".

Yeah, that's fair. Let's withdraw my suggestion of a limited number of reaffirmations. :) That works well if the goal is to have a temporary restriction while work is actively happening to remove its necessity, but that's less likely to be the case here. If we can't move to C42 because it's not supported on Linux Distro 2050™ yet, users of Linux Distro 2050™ aren't able to promise eventual support of C42 or accelerate that happening.

Show 11 quoted lines

>
> >
> > The thinking here is that this test is imposing a restriction on the
> > git developers that we've agreed to take on as a favor: we are going
> > to restrict ourselves from doing X _for the time being_ not
> > necessarily because it'll break you, but because it's a bad experience
> > for the git developers to create a patch series that lands and then
> > gets backed out when the breakage on $non_standard_platform is
> > detected on seen/next/master.
>
> 1: https://lore.kernel.org/git/CAO_smVjZ7DSPdL+KYCm2mQ=q55XbEH7Vu_jLxkAa5WTcD9rq8A@mail.gmail.com/T/#m9d7656905be500a97ee6f86b030e94c91a79507a