Re: emacs-29 e2ac0d416b9 1/5: ; Merge from origin/emacs-28

Re: emacs-29 e2ac0d416b9 1/5: ; Merge from origin/emacs-28

[Eli Zaretskii]

> branch: emacs-29
> commit e2ac0d416b9864d7ae80b3541d94060eebcd615f
> Merge: 068b53500e2 e339926272a
> Author: Stefan Kangas <stefankangas@gmail.com>
> Commit: Stefan Kangas <stefankangas@gmail.com>
> 
>     ; Merge from origin/emacs-28
>     
>     The following commits were skipped:
>     
>     e339926272a Fix etags local command injection vulnerability
>     5d05ea803e9 Fixed ctags local command execute vulnerability
>     22fb5ff5126 Fix ruby-mode.el local command injection vulnerability (b...
>     807d2d5b3a7 Fix htmlfontify.el command injection vulnerability.
>     ae9bfed50db Fix storing email into nnmail by Gnus

Stefan, why did you merge from emacs-28 to emacs-29?  I think it's a
mistake: who knows what this brought to the release branch.

If the reason was etc/HISTORY, I'd prefer making that change in
emacs-29 by hand instead.  Are there any other reasons?

Re: emacs-29 e2ac0d416b9 1/5: ; Merge from origin/emacs-28

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>>     ; Merge from origin/emacs-28
>>
>>     The following commits were skipped:
>>
>>     e339926272a Fix etags local command injection vulnerability
>>     5d05ea803e9 Fixed ctags local command execute vulnerability
>>     22fb5ff5126 Fix ruby-mode.el local command injection vulnerability (b...
>>     807d2d5b3a7 Fix htmlfontify.el command injection vulnerability.
>>     ae9bfed50db Fix storing email into nnmail by Gnus
>
> Stefan, why did you merge from emacs-28 to emacs-29?  I think it's a
> mistake: who knows what this brought to the release branch.

We know what this brought though?  You can see the full list of changes
with "git diff", e.g.:

    git diff 068b53500e24b7b..ad6c6a3a11569c4

> If the reason was etc/HISTORY, I'd prefer making that change in
> emacs-29 by hand instead.  Are there any other reasons?

Yes, ChangeLog.3, and AUTHORS too.  Of course we can copy the files and
commit them manually, but AFAIK that's what merge is intended for.  It
also preserves history and lets us use the same tags on master as on the
release branch.

What am I missing?

Re: emacs-29 e2ac0d416b9 1/5: ; Merge from origin/emacs-28

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 19 Feb 2023 09:31:08 -0800
> Cc: emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >>     ; Merge from origin/emacs-28
> >>
> >>     The following commits were skipped:
> >>
> >>     e339926272a Fix etags local command injection vulnerability
> >>     5d05ea803e9 Fixed ctags local command execute vulnerability
> >>     22fb5ff5126 Fix ruby-mode.el local command injection vulnerability (b...
> >>     807d2d5b3a7 Fix htmlfontify.el command injection vulnerability.
> >>     ae9bfed50db Fix storing email into nnmail by Gnus
> >
> > Stefan, why did you merge from emacs-28 to emacs-29?  I think it's a
> > mistake: who knows what this brought to the release branch.
> 
> We know what this brought though?  You can see the full list of changes
> with "git diff", e.g.:
> 
>     git diff 068b53500e24b7b..ad6c6a3a11569c4

Why would we need to eyeball all those changes now?  It's a wasted
effort.  We never merge to the release branch, never.

> > If the reason was etc/HISTORY, I'd prefer making that change in
> > emacs-29 by hand instead.  Are there any other reasons?
> 
> Yes, ChangeLog.3, and AUTHORS too.

You will recreate those as part of tarring Emacs 29 anyway, so why
merge them?  And HISTORY change is a single line.

> What am I missing?

It is simply unnecessary risk, and something we never do, for very
good reasons.  I'd sleep better if you'd reverted those changes on
emacs-29, and made the single change in HISTORY by hand.

Re: emacs-29 e2ac0d416b9 1/5: ; Merge from origin/emacs-28

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>>     git diff 068b53500e24b7b..ad6c6a3a11569c4
>
> Why would we need to eyeball all those changes now?  It's a wasted
> effort.  We never merge to the release branch, never.

I already eyeballed all of those changes, so there's no need for you to
do it too.  Here's a summary of the changes:

$ git diff -b --stat 068b53500e24b7b..ad6c6a3a11569c4
 ChangeLog.3 | 430 +++++++++++++++++++++++++++++++++++++-
 etc/AUTHORS |  24 +--
 etc/HISTORY |   2 +
 3 files changed, 443 insertions(+), 13 deletions(-)

> You will recreate those as part of tarring Emacs 29 anyway, so why
> merge them?

You left out the part of my message where I explained this, but a merge
preserves history and tags.  That's generally why you merge instead of
simply copying files.  It also happens to be more convenient.

> It is simply unnecessary risk, and something we never do, for very
> good reasons.  I'd sleep better if you'd reverted those changes on
> emacs-29, and made the single change in HISTORY by hand.

The risk is minimal, since we're only changing documentation files and
not code.  There is also a big gain in simplicity.  These are standard
git operations, after all.

Re: emacs-29 e2ac0d416b9 1/5: ; Merge from origin/emacs-28

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 20 Feb 2023 08:50:10 -0800
> Cc: emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >>     git diff 068b53500e24b7b..ad6c6a3a11569c4
> >
> > Why would we need to eyeball all those changes now?  It's a wasted
> > effort.  We never merge to the release branch, never.
> 
> I already eyeballed all of those changes

You could miss something or make a mistake, couldn't you?  It's
possible.

> $ git diff -b --stat 068b53500e24b7b..ad6c6a3a11569c4
>  ChangeLog.3 | 430 +++++++++++++++++++++++++++++++++++++-
>  etc/AUTHORS |  24 +--
>  etc/HISTORY |   2 +
>  3 files changed, 443 insertions(+), 13 deletions(-)

That's not what I saw.  I saw many more changes brought be the merge.

And again, the changes to AUTHORS and ChangeLog.3 are not needed on
the emacs-29 branch, since those files are generated as part of
preparing the release.

> You left out the part of my message where I explained this, but a merge
> preserves history and tags.

We don't need to preserve history on the emacs-29 branch.  The history
and tags are preserved by the master branch, and when we cut a release
branch, it inherits that.  But merging an old release branch to a
newer release branch is pointless, as it doesn't preserve anything.
An old release branch is a dead end, and any changes on it are not
interesting, since they are at best no-ops.

> > It is simply unnecessary risk, and something we never do, for very
> > good reasons.  I'd sleep better if you'd reverted those changes on
> > emacs-29, and made the single change in HISTORY by hand.
> 
> The risk is minimal

Famous last words.

Why should we have _any_ risk at all?  There's no gain here,
absolutely none!  Risk with no gain makes no sense...

Re: emacs-29 e2ac0d416b9 1/5: ; Merge from origin/emacs-28

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> You could miss something or make a mistake, couldn't you?  It's
> possible.

Yes, of course.  That's why I prefer using standard git operations (as
opposed to merely copying); it is a way to mitigate that risk.

> I saw many more changes brought be the merge.

Please try again, but use the -b flag.  Otherwise, you will include
removal of trailing whitespace in three files.  (Our git hooks blocked
merging in the presence of such files.)

>> The risk is minimal
>
> Famous last words.
>
> Why should we have _any_ risk at all?  There's no gain here,
> absolutely none!  Risk with no gain makes no sense...

The gain is workflow efficiency, which is of course non-zero.  Merging,
in this case, is also a gain in simplicity and convenience.

When I said "minimal risk", that was is in reference to that we are
discussing documentation (non-code) files.  Sorry for being imprecise.
Meanwhile, copying file contents as juvenile commits is generally high
risk due to the limited traceability.