GNU bug report logs - #78813
feature/igc: [PATCH] Make the array face_attr_sym readonly

Previous Next

Package: emacs;

Reported by: Helmut Eller <eller.helmut <at> gmail.com>

Date: Tue, 17 Jun 2025 08:50:02 UTC

Severity: normal

Tags: patch

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


Message #38 received at 78813 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78813 <at> debbugs.gnu.org, eller.helmut <at> gmail.com
Subject: Re: bug#78813: feature/igc: [PATCH] Make the array face_attr_sym
 readonly
Date: Sat, 21 Jun 2025 13:54:38 +0300
> Date: Sat, 21 Jun 2025 10:07:43 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: eller.helmut <at> gmail.com, 78813 <at> debbugs.gnu.org
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> Sorry I'm asking for clarification, but my impression was the igc branch
> >> was to change very little in the !HAVE_MPS case, because general GC
> >> cleanups such as this one make more sense on the main branch: fewer
> >> merge conflicts, we'll see the benefits before merging the branch (or
> >> even if for some reason we never do), a better git history, better test
> >> coverage for pre-push testing.
> >
> > That's true in general, but if some change in the non-MPS case makes
> > the overall code cleaner, there should be no reason not to make it on
> > the branch.
> 
> I can't parse that statement.  If a change affects only the non-MPS
> case, which branch should we make it on?  The master branch would make
> sense to me, and the feature/igc branch doesn't make sense.

That's not the situation I alluded to.  Your question was about more
general situations: you never said that you are talking about changes
that affect _only_ the non-MPS parts of the code.  I replied with the
situation in mind where some change affects _both_ MPS and non-MPS
parts.  Changes that affect only the non-MPS parts should preferably
installed on master, indeed, but even in that case this is not a
dogma, and we could make exceptions if there are good reasons.

> > As for merge conflicts, I'm not afraid of that, especially if we make
> > changes in an area that doesn't see too many changes (initializations
> > are generally of that nature).
> 
> Well, you did just push a change which causes a merge conflict here.

I didn't install any changes on the igc branch, so I'm not sure how
this is relevant.  I did exactly what you suggested: making any such
changes on master.  What am I missing?

> Helmut, as this discussion shows, I see no way to apply changes such as
> this one right now; they won't be accepted in master, apparently, and
> applying them to feature/igc will produce merge conflicts, as we've just
> seen.

What have we just seen, may I ask??

> >> If we decide not to reduce the number of static objects at this time and
> >> focus on getting things to work, I can live with that and continue
> >> working on the branch, but I can't speak for Helmut, of course.
> >>
> >> Diverting all work that's even slightly related to GC to the feature/igc
> >> branch and keeping master in its current state seems like a very risky
> >> proposal to me.
> >
> > Maybe I'm missing something, but what does this change have to do with
> > GC?  This particular static object is never modified and is not
> > exposed to Lisp.  So it shouldn't cause us any GC-related efforts.
> 
> This confuses me.  The unusual thing here is precisely that we have a
> static object which is not staticpro'd but which we know not to cause GC
> problems.  Helmut said so clearly when he posted the patch.  "Never
> modified and not exposed to Lisp" is irrelevant to the question of
> whether GC needs to know about the object.

Well, the initialization aspects are still relevant.  Let's please try
to keep the initialization of Lisp values in the syms_of_FOO and
init_FOO functions, as we always did.  If this requires GC protection,
it's fine by me.

> This single change isn't critical, of course, but feature/igc already
> contains too many changes which should be merged to master before we
> ever consider merging the branch as a whole.  If the strategy is to
> increase that number further, that makes a sensible merge of feature/igc
> much less likely.

Like I said: merge conflicts are a non-issue with modern dVCSes.
Let's not let that particular tail wag the dog.




This bug report was last modified 25 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.