Re: svn commit: r1936137 - subversion/branches/1.15.x

On 15. 7. 2026 12:06, Daniel Sahlberg wrote:
> Den tis 14 juli 2026 kl 17:40 skrev <[email protected]>:
>
>     Author: kotkov
>     Date: Tue Jul 14 15:40:10 2026
>     New Revision: 1936137
>
>     Log:
>     * branches/1.15.x/STATUS: Add separate sections for release
>     blocker candidates
>       and changes that cannot go to a later 1.15.x release (we used
>     this pattern
>       in 1.9.x/STATUS, should be useful here as well).
>
>       Put all current nominations into the "other" section.
>
>     Modified:
>        subversion/branches/1.15.x/STATUS
>
>     Modified: subversion/branches/1.15.x/STATUS
>     ==============================================================================
>     --- subversion/branches/1.15.x/STATUS   Tue Jul 14 15:38:15 2026 
>           (r1936136)
>     +++ subversion/branches/1.15.x/STATUS   Tue Jul 14 15:40:10 2026 
>           (r1936137)
>     @@ -12,8 +12,14 @@ delay a release, etc.
>
>      Status of 1.15.0:
>
>     -Candidate changes:
>     -==================
>     +Release blockers for 1.15.0:
>     +============================
>     +
>     +Candidate changes that cannot go into a later 1.15.x:
>     +=====================================================
>     +
>     +Other candidate changes:
>     +========================
>
>
> These headlines break the backport scripts (libraries 
> /tools/dist/backport/*.py and the associated scripts in /tools/dist/).
>
> More specifically, /tools/dist/backport/status.py (the library 
> responsible for parsing STATUS) is looking for the headline "Candidate 
> changes:". Any unknown headlines are kept as-is, so at least it didn't 
> munch any existing nominations.
>
> I tried this out in the recent 1936154-1936156. When the script added 
> the nomination, it looked for a "Candidate changes:" headline and when 
> one wasn't found, it added one last (see [1]).
>
> I've updated the script in r1936159, changing it to first look for 
> Other candidate changes and, if that fails, look for Candidate changes 
> (adding the header if not found). I think that should work well 
> enough, we can support either the "x.y.0 format" (as it is currently) 
> or "z.y.z with z>0". The script doesn't support adding a nomination as 
> a release blocker or as a "cannot go into a later.." but I think it 
> isn't very high priority.
>
> merge-approved-backports.py seems to work fine, it only looks for 
> "Approved changes" and keep the remaining headers. It doesn't care 
> about the order of the headers. r1936156 was done by 
> running merge-approved-backports.py.
>
> Does this change look ok?


It fails the rule that parsing should be immune to expected content 
changes. The "Approved changes" heading should always be there, STATUS 
file is invalid if it's not; and traditionally we've always had it at 
the end of the file.

Your change adapted to this single specific modification of the section 
headers. The next, slightly different modification will break the script 
again. I really think the safest way is to always insert nominations 
before "Approved changes" and to fail if that heading isn't there. If 
the headings in the file make that insertion dubious, then the heading 
order should be fixed anyway. In the meantime, review of the diff before 
commit is kind of expected.

Yes, I've looked at the script and I'm aware that the parsing code is 
opaque and complex. I didn't say my proposal leads to a simple change. 
But it does make the script more future proof.

-- Brane