On Sun, Jan 26, 2020 at 10:40:02PM +0000, William Dauchy wrote:
> On Sun, Jan 26, 2020 at 11:06 PM Lukas Tribus <lu...@ltri.eu> wrote:
> > Just because commit 456 fixes something that has been introduced with
> > commit 123 DOES NOT mean we backport 456 to all the branches that 123
> > was committed to - instead we backport 456 to a certain branch if it
> > *actually* makes sense to do so.
> > This is not something a VCS will figure out for us, but is a human
> > decision. The author should be suggesting it's intention regarding the
> > backport in the commit message, because it's the one person that spend
> > the most time with the issue and probably has the best understanding
> > of its impacts as well as the complexity and necessity of a backport.
> > That does not mean the author needs to tests the fix with those
> > branches, send branch-specific diff's, or do other research regarding
> > the backport, but *the intention needs to be stated* (based on the
> > research already conducted for the matter at hand).

I wouldn't have said it better. That's exactly the point, and is
the reason why there are a lot of "should be backported" or "seems
to be affected" in our commit messages. It's important to keep in
mind that for one commit to master, there are sometimes up to 4-5
ones replicated to older branches, done by different people, who
usually don't know much (if at all) about the area being fixed, and
doing this effort each an every time the fix gets backported is
painful and counter-productive because it adds to the error rate.
So any hint the person fixing the bug can give in human language,
including "in version 1.6 the function has a different name", or
"in version 1.8 an additional null check will be needed" are very
helpful.

> yes, that's why I was more talking about a semi automatic process,
> which only suggests to do a backport with this tag.

For now (sadly), the code is still not isolated enough to warrant a
semi automatic process. Usually in the 3 months that follow a major
release, all cherry-picks from master to this release work without
any effort. But then, between refactoring, code moves, name changes,
API changes etc, even if the patch applies it doesn't imply that it
will work, so we need to read the change in its context (and obviously
to perform basic testing).

> I am thinking about examples like on netdev linux kernel subsystem, a
> "Fixes:" tag suggests that it is a possible candidate for stable
> branches, but then it's up to the maintainer to decide whether to
> backport it or not.

Many times I've been wondering if we should start to add a Fixes tag,
but very often I figured it would not map well. Often we don't fix a
bug introduced by one patch but *revealed* by one patch while it was
already there. This is related to the long history of this code being
extremely monolithic in the past and progressively becoming more
modular. While in Linux all drivers are independent, each network
protocol is independent, and bugs area really isolated, it's still
not the case for us eventhough it's progressively starting to appear.
So for now, placing a Fixes tag would not bring much value. Also,
having to issue "git describe --contains" and/or "git log --grep $ID"
for each of them is more time consuming than simply skipping the patch
being reviewed based on its commit message. In Linux, the Fixes tag
started to become really useful to me when adding "# 3.4, 3.10 ..."
after it because it was a convenient way to indicate "drop that". 

I know our process is far from being perfect, but I'm trying to keep
it optimally efficient for those doing the work so that we can continue
to deliver timely fixes for all supported branches.

> Anyway, that being said, I have nothing against adding those messages,
> it was a suggestion.

Thanks for the suggestion. It's sometimes useful to question whether
what's been done for a while is still valid or not.

Willy

Reply via email to