CHECK_STRUCTS/dmpstruct.h mechanism is broken.

CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Emacs.

The CHECK_STRUCT/dmpstruct.h mechanism is a very clever way of ensuring
that Emacs cannot be built after amending certain structs, or even the
comments within them.

I have amended such a comment, thus cannot build my Emacs.

This mechanism is broken, since it would appear to be entirely
undocumented.  Or does this documentation exist, somewhere obscure?
There is nothing about it in INSTALL.REPO, for example.

For the sake of my sanity, will whoever it is please tell me what I have
now to do to build my Emacs.  make bootstrap fails.  This is a bug; make
bootstrap should _never_ fail.  I've done make dmpstruct.h.  Again to no
avail.

There's a file dmpstruct.awk involved in this, but it contains no
instructions in its header comments; how is it meant to be called for
example?  What else needs to be run to make it work, for another
example?

And what's it all for?  Why should make bootstrap be broken?  What's the
point of all this?  Why are there no explanatory comments in the
generated file dmpstruct.h?

Help!

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> Date: Thu, 28 Feb 2019 20:21:46 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> For the sake of my sanity, will whoever it is please tell me what I have
> now to do to build my Emacs.  make bootstrap fails.  This is a bug; make
> bootstrap should _never_ fail.  I've done make dmpstruct.h.  Again to no
> avail.

I don't understand why it doesn't work for you.  Just to make sure
there's no mystery, I renamed src/dmpstruct.h and then said "make" at
top level -- the file was regenerated, and the result was identical to
the renamed version; and then a new Emacs binary was built
successfully.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Emacs.

On Thu, Feb 28, 2019 at 20:21:46 +0000, Alan Mackenzie wrote:
> The CHECK_STRUCT/dmpstruct.h mechanism is a very clever way of ensuring
> that Emacs cannot be built after amending certain structs, or even the
> comments within them.

> I have amended such a comment, thus cannot build my Emacs.

> This mechanism is broken, since it would appear to be entirely
> undocumented.  Or does this documentation exist, somewhere obscure?
> There is nothing about it in INSTALL.REPO, for example.

> For the sake of my sanity, will whoever it is please tell me what I have
> now to do to build my Emacs.  make bootstrap fails.  This is a bug; make
> bootstrap should _never_ fail.  I've done make dmpstruct.h.  Again to no
> avail.

OK, I've worked out what's to be done, and done it.  dmpstruct.h is a
file containing up to date hashes of structures.  The old hash in
pdumper.c needs to replaced by hand by the updated hash from
dmpstruct.h.  My Emacs now builds.

This is anything but clear from the comment at L71 in pdumper.c.  It
says "and update the hash..." without saying which hash, and without
saying how.  Which utility is needed to update this hash?  (Answer:
none).

> There's a file dmpstruct.awk involved in this, but it contains no
> instructions in its header comments; how is it meant to be called for
> example?  What else needs to be run to make it work, for another
> example?

> And what's it all for?  Why should make bootstrap be broken?  What's the
> point of all this?  Why are there no explanatory comments in the
> generated file dmpstruct.h?

Again, is all this really needed?  Is pdumper.c really that fragile,
that it can't cope with changes in certain structs?

> Help!

> -- 
> Alan Mackenzie (Nuremberg, Germany).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> Date: Thu, 28 Feb 2019 20:59:55 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> OK, I've worked out what's to be done, and done it.  dmpstruct.h is a
> file containing up to date hashes of structures.  The old hash in
> pdumper.c needs to replaced by hand by the updated hash from
> dmpstruct.h.  My Emacs now builds.
> 
> This is anything but clear from the comment at L71 in pdumper.c.  It
> says "and update the hash..." without saying which hash, and without
> saying how.  Which utility is needed to update this hash?  (Answer:
> none).

Please feel free to improve the comments to clarify anything that is
not crystal clear.

TIA

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Eli.

On Fri, Mar 01, 2019 at 09:31:08 +0200, Eli Zaretskii wrote:
> > Date: Thu, 28 Feb 2019 20:59:55 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > OK, I've worked out what's to be done, and done it.  dmpstruct.h is a
> > file containing up to date hashes of structures.  The old hash in
> > pdumper.c needs to replaced by hand by the updated hash from
> > dmpstruct.h.  My Emacs now builds.

> > This is anything but clear from the comment at L71 in pdumper.c.  It
> > says "and update the hash..." without saying which hash, and without
> > saying how.  Which utility is needed to update this hash?  (Answer:
> > none).

> Please feel free to improve the comments to clarify anything that is
> not crystal clear.

Done.

> TIA

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 2/28/19 12:59 PM, Alan Mackenzie wrote:
> is all this really needed?  Is pdumper.c really that fragile,
> that it can't cope with changes in certain structs?

No, it's not needed, and in my experience the mechanism's costs far
exceed any benefit. Every time it has complained to me, it has been a
false alarm. Conversely, it has not caught any of the mistakes I've made
when making changes that affect the pdumper. So its batting average is
.000 and it's cost me far too much irritation (admittedly minor each
time, but it adds up ...).

The Emacs source code has thousands of interconnections, all of which
have to be just right to get it to work. There is nothing special about
the interconnections to the pdumper that justify this extra level of
bureaucracy.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 470 bytes --]

On 3/4/19 6:17 PM, Paul Eggert wrote:
> On 2/28/19 12:59 PM, Alan Mackenzie wrote:
>> is all this really needed?  Is pdumper.c really that fragile,
>> that it can't cope with changes in certain structs?
> No, it's not needed, and in my experience the mechanism's costs far
> exceed any benefit.

No further comment and the mechanism just bit me again, so I installed
the attached patch to disable it. We can reenable it later if needed
(which I hope won't happen....).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-dmpstruct.h.patch --]
[-- Type: text/x-patch; name="0001-Remove-dmpstruct.h.patch", Size: 16303 bytes --]

From 891e507d06c3bfcd9ac181de6bb0ff9c27dfa4aa Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 9 Apr 2019 15:42:10 -0700
Subject: [PATCH] Remove dmpstruct.h
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The hassles of updating the dmpstruct.h-using code bit me again.
These updates are more trouble than they’re worth.  See:
https://lists.gnu.org/r/emacs-devel/2019-03/msg00122.html
As I’m the main person who’s made changes in this area since
dmpstruct.h was introduced, I’m the most motivated to clean up
the situation.
* make-dist (possibly_non_vc_files): Remove src/dmpstruct.h.
* src/Makefile.in (dmpstruct_headers, dmpstruct.h): Remove.
(pdumper.o): Do not depend on dmpstruct.h.
(mostlyclean): Do not remove dmpstruct.h.
* src/dmpstruct.awk: Remove.
* src/pdumper.c: Do not include dmpstruct.h.
(CHECK_STRUCTS): Remove.  All uses removed.
---
 .gitignore        |  1 -
 make-dist         |  2 +-
 src/Makefile.in   | 10 +-----
 src/dmpstruct.awk | 45 ------------------------
 src/pdumper.c     | 89 -----------------------------------------------
 5 files changed, 2 insertions(+), 145 deletions(-)
 delete mode 100755 src/dmpstruct.awk

diff --git a/.gitignore b/.gitignore
index 355824f390..bd5a8e7947 100644
--- a/.gitignore
+++ b/.gitignore
@@ -187,7 +187,6 @@ src/emacs-[0-9]*
 src/temacs
 src/temacs.in
 src/fingerprint.c
-src/dmpstruct.h
 src/*.pdmp
 
 # Character-set info.
diff --git a/make-dist b/make-dist
index 4e18d77a87..821895a005 100755
--- a/make-dist
+++ b/make-dist
@@ -366,7 +366,7 @@ possibly_non_vc_files=
   $top_level_ChangeLog
   MANIFEST aclocal.m4 configure
   admin/charsets/jisx2131-filter
-  src/config.in src/dmpstruct.h src/emacs-module.h
+  src/config.in src/emacs-module.h
   src/fingerprint.c
 "$(
   find admin doc etc lisp \
diff --git a/src/Makefile.in b/src/Makefile.in
index dee3a534db..10b2da319b 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -456,14 +456,6 @@ ALLOBJS =
 all: emacs$(EXEEXT) $(pdmp) $(OTHER_FILES)
 .PHONY: all
 
-dmpstruct_headers=$(srcdir)/lisp.h $(srcdir)/buffer.h \
-	$(srcdir)/intervals.h $(srcdir)/charset.h $(srcdir)/bignum.h
-pdumper.o: dmpstruct.h
-dmpstruct.h: $(srcdir)/dmpstruct.awk
-dmpstruct.h: $(libsrc)/make-fingerprint$(EXEEXT) $(dmpstruct_headers)
-	$(AM_V_GEN)POSIXLY_CORRECT=1 awk -f $(srcdir)/dmpstruct.awk \
-		$(dmpstruct_headers) > $@
-
 AUTO_DEPEND = @AUTO_DEPEND@
 DEPDIR = deps
 ifeq ($(AUTO_DEPEND),yes)
@@ -681,7 +673,7 @@ .PHONY:
 
 mostlyclean:
 	rm -f temacs$(EXEEXT) core ./*.core \#* ./*.o
-	rm -f temacs.in$(EXEEXT) fingerprint.c dmpstruct.h
+	rm -f temacs.in$(EXEEXT) fingerprint.c
 	rm -f emacs.pdmp
 	rm -f ../etc/DOC
 	rm -f bootstrap-emacs$(EXEEXT) $(bootstrap_pdmp)
diff --git a/src/dmpstruct.awk b/src/dmpstruct.awk
deleted file mode 100755
index 55626cf8b2..0000000000
--- a/src/dmpstruct.awk
+++ /dev/null
@@ -1,45 +0,0 @@
-# Copyright (C) 2018-2019 Free Software Foundation, Inc.
-#
-# This file is part of GNU Emacs.
-#
-# GNU Emacs is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or (at
-# your option) any later version.
-#
-# GNU Emacs is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
-
-BEGIN {
-  print "/* Generated by dmpstruct.awk */"
-  print "#ifndef EMACS_DMPSTRUCT_H"
-  print "#define EMACS_DMPSTRUCT_H"
-  struct_name = ""
-  tmpfile = "dmpstruct.tmp"
-}
-# Match a type followed by optional syntactic whitespace
-/^(enum|struct|union) [a-zA-Z0-9_]+([\t ]|\/\*.*\*\/)*$/ {
-  struct_name = $2
-  close (tmpfile)
-}
-/^(enum|struct|union) [a-zA-Z0-9_]+([\t ]|\/\*.*\*\/)*$/, /^(  )?};$/ {
-  print $0 > tmpfile
-}
-/^(  )?} *(GCALIGNED_STRUCT)? *;$/ {
-  if (struct_name != "") {
-    fflush (tmpfile)
-    cmd = "../lib-src/make-fingerprint -r " tmpfile
-    cmd | getline hash
-    close (cmd)
-    printf "#define HASH_%s_%.10s\n", struct_name, hash
-    struct_name = ""
-  }
-}
-END {
-  print "#endif /* EMACS_DMPSTRUCT_H */"
-}
diff --git a/src/pdumper.c b/src/pdumper.c
index cb2915cb20..68c412d47c 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -46,8 +46,6 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "thread.h"
 #include "bignum.h"
 
-#include "dmpstruct.h"
-
 /*
   TODO:
 
@@ -68,16 +66,6 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 #ifdef HAVE_PDUMPER
 
-/* CHECK_STRUCTS being true makes the build break if we notice
-   changes to the source defining certain Lisp structures we dump. If
-   you change one of these structures, check that the pdumper code is
-   still valid, and update the pertinent hash lower down in this file
-   (pdumper.c) by manually copying the value from the dmpstruct.h
-   generated from your new code.  */
-#ifndef CHECK_STRUCTS
-# define CHECK_STRUCTS 1
-#endif
-
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7)
 # pragma GCC diagnostic error "-Wconversion"
 # pragma GCC diagnostic error "-Wshadow"
@@ -2043,9 +2031,6 @@ dump_pseudovector_lisp_fields (struct dump_context *ctx,
 static dump_off
 dump_cons (struct dump_context *ctx, const struct Lisp_Cons *cons)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Cons_00EEE63F67)
-# error "Lisp_Cons changed. See CHECK_STRUCTS comment."
-#endif
   struct Lisp_Cons out;
   dump_object_start (ctx, &out, sizeof (out));
   dump_field_lv (ctx, &out, cons, &cons->u.s.car, WEIGHT_STRONG);
@@ -2058,9 +2043,6 @@ dump_interval_tree (struct dump_context *ctx,
                     INTERVAL tree,
                     dump_off parent_offset)
 {
-#if CHECK_STRUCTS && !defined (HASH_interval_1B38941C37)
-# error "interval changed. See CHECK_STRUCTS comment."
-#endif
   /* TODO: output tree breadth-first?  */
   struct interval out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2102,9 +2084,6 @@ dump_interval_tree (struct dump_context *ctx,
 static dump_off
 dump_string (struct dump_context *ctx, const struct Lisp_String *string)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_String_86FEA6EC7C)
-# error "Lisp_String changed. See CHECK_STRUCTS comment."
-#endif
   /* If we have text properties, write them _after_ the string so that
      at runtime, the prefetcher and cache will DTRT. (We access the
      string before its properties.).
@@ -2148,10 +2127,6 @@ dump_string (struct dump_context *ctx, const struct Lisp_String *string)
 static dump_off
 dump_marker (struct dump_context *ctx, const struct Lisp_Marker *marker)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Marker_642DBAF866)
-# error "Lisp_Marker changed. See CHECK_STRUCTS comment."
-#endif
-
   START_DUMP_PVEC (ctx, &marker->header, struct Lisp_Marker, out);
   dump_pseudovector_lisp_fields (ctx, &out->header, &marker->header);
   DUMP_FIELD_COPY (out, marker, need_adjustment);
@@ -2171,9 +2146,6 @@ dump_marker (struct dump_context *ctx, const struct Lisp_Marker *marker)
 static dump_off
 dump_overlay (struct dump_context *ctx, const struct Lisp_Overlay *overlay)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Overlay_72EADA9882)
-# error "Lisp_Overlay changed. See CHECK_STRUCTS comment."
-#endif
   START_DUMP_PVEC (ctx, &overlay->header, struct Lisp_Overlay, out);
   dump_pseudovector_lisp_fields (ctx, &out->header, &overlay->header);
   dump_field_lv_rawptr (ctx, out, overlay, &overlay->next,
@@ -2199,9 +2171,6 @@ static dump_off
 dump_finalizer (struct dump_context *ctx,
                 const struct Lisp_Finalizer *finalizer)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Finalizer_D58E647CB8)
-# error "Lisp_Finalizer changed. See CHECK_STRUCTS comment."
-#endif
   START_DUMP_PVEC (ctx, &finalizer->header, struct Lisp_Finalizer, out);
   /* Do _not_ call dump_pseudovector_lisp_fields here: we dump the
      only Lisp field, finalizer->function, manually, so we can give it
@@ -2221,9 +2190,6 @@ struct bignum_reload_info
 static dump_off
 dump_bignum (struct dump_context *ctx, Lisp_Object object)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Bignum_661945DE2B)
-# error "Lisp_Bignum changed. See CHECK_STRUCTS comment."
-#endif
   const struct Lisp_Bignum *bignum = XBIGNUM (object);
   START_DUMP_PVEC (ctx, &bignum->header, struct Lisp_Bignum, out);
   verify (sizeof (out->value) >= sizeof (struct bignum_reload_info));
@@ -2259,9 +2225,6 @@ dump_bignum (struct dump_context *ctx, Lisp_Object object)
 static dump_off
 dump_float (struct dump_context *ctx, const struct Lisp_Float *lfloat)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Float_50A7B216D9)
-# error "Lisp_Float changed. See CHECK_STRUCTS comment."
-#endif
   eassert (ctx->header.cold_start);
   struct Lisp_Float out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2272,9 +2235,6 @@ dump_float (struct dump_context *ctx, const struct Lisp_Float *lfloat)
 static dump_off
 dump_fwd_int (struct dump_context *ctx, const struct Lisp_Intfwd *intfwd)
 {
-#if CHECK_STRUCTS && !defined HASH_Lisp_Intfwd_4D887A7387
-# error "Lisp_Intfwd changed. See CHECK_STRUCTS comment."
-#endif
   dump_emacs_reloc_immediate_intmax_t (ctx, intfwd->intvar, *intfwd->intvar);
   struct Lisp_Intfwd out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2286,9 +2246,6 @@ dump_fwd_int (struct dump_context *ctx, const struct Lisp_Intfwd *intfwd)
 static dump_off
 dump_fwd_bool (struct dump_context *ctx, const struct Lisp_Boolfwd *boolfwd)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Boolfwd_0EA1C7ADCC)
-# error "Lisp_Boolfwd changed. See CHECK_STRUCTS comment."
-#endif
   dump_emacs_reloc_immediate_bool (ctx, boolfwd->boolvar, *boolfwd->boolvar);
   struct Lisp_Boolfwd out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2300,9 +2257,6 @@ dump_fwd_bool (struct dump_context *ctx, const struct Lisp_Boolfwd *boolfwd)
 static dump_off
 dump_fwd_obj (struct dump_context *ctx, const struct Lisp_Objfwd *objfwd)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Objfwd_45D3E513DC)
-# error "Lisp_Objfwd changed. See CHECK_STRUCTS comment."
-#endif
   if (NILP (Fgethash (dump_off_to_lisp (emacs_offset (objfwd->objvar)),
                       ctx->staticpro_table,
                       Qnil)))
@@ -2318,9 +2272,6 @@ static dump_off
 dump_fwd_buffer_obj (struct dump_context *ctx,
                      const struct Lisp_Buffer_Objfwd *buffer_objfwd)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Buffer_Objfwd_13CA6B04FC)
-# error "Lisp_Buffer_Objfwd changed. See CHECK_STRUCTS comment."
-#endif
   struct Lisp_Buffer_Objfwd out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, buffer_objfwd, type);
@@ -2334,9 +2285,6 @@ static dump_off
 dump_fwd_kboard_obj (struct dump_context *ctx,
                      const struct Lisp_Kboard_Objfwd *kboard_objfwd)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Kboard_Objfwd_CAA7E71069)
-# error "Lisp_Intfwd changed. See CHECK_STRUCTS comment."
-#endif
   struct Lisp_Kboard_Objfwd out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, kboard_objfwd, type);
@@ -2347,9 +2295,6 @@ dump_fwd_kboard_obj (struct dump_context *ctx,
 static dump_off
 dump_fwd (struct dump_context *ctx, lispfwd fwd)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Fwd_Type_9CBA6EE55E)
-# error "Lisp_Fwd_Type changed. See CHECK_STRUCTS comment."
-#endif
   void const *p = fwd.fwdptr;
   dump_off offset;
 
@@ -2381,9 +2326,6 @@ static dump_off
 dump_blv (struct dump_context *ctx,
           const struct Lisp_Buffer_Local_Value *blv)
 {
-#if CHECK_STRUCTS && !defined HASH_Lisp_Buffer_Local_Value_3C363FAC3C
-# error "Lisp_Buffer_Local_Value changed. See CHECK_STRUCTS comment."
-#endif
   struct Lisp_Buffer_Local_Value out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, blv, local_if_set);
@@ -2446,13 +2388,6 @@ dump_symbol (struct dump_context *ctx,
              Lisp_Object object,
              dump_off offset)
 {
-#if CHECK_STRUCTS && !defined HASH_Lisp_Symbol_999DC26DEC
-# error "Lisp_Symbol changed. See CHECK_STRUCTS comment."
-#endif
-#if CHECK_STRUCTS && !defined (HASH_symbol_redirect_ADB4F5B113)
-# error "symbol_redirect changed. See CHECK_STRUCTS comment."
-#endif
-
   if (ctx->flags.defer_symbols)
     {
       if (offset != DUMP_OBJECT_ON_SYMBOL_QUEUE)
@@ -2542,9 +2477,6 @@ static dump_off
 dump_vectorlike_generic (struct dump_context *ctx,
 			 const union vectorlike_header *header)
 {
-#if CHECK_STRUCTS && !defined (HASH_vectorlike_header_00A5A4BFB2)
-# error "vectorlike_header changed. See CHECK_STRUCTS comment."
-#endif
   const struct Lisp_Vector *v = (const struct Lisp_Vector *) header;
   ptrdiff_t size = header->size;
   enum pvec_type pvectype = PSEUDOVECTOR_TYPE (v);
@@ -2702,9 +2634,6 @@ dump_hash_table (struct dump_context *ctx,
                  Lisp_Object object,
                  dump_off offset)
 {
-#if CHECK_STRUCTS && !defined HASH_Lisp_Hash_Table_EF95ED06FF
-# error "Lisp_Hash_Table changed. See CHECK_STRUCTS comment."
-#endif
   const struct Lisp_Hash_Table *hash_in = XHASH_TABLE (object);
   bool is_stable = dump_hash_table_stable_p (hash_in);
   /* If the hash table is likely to be modified in memory (either
@@ -2770,9 +2699,6 @@ dump_hash_table (struct dump_context *ctx,
 static dump_off
 dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
 {
-#if CHECK_STRUCTS && !defined HASH_buffer_E34A11C6B9
-# error "buffer changed. See CHECK_STRUCTS comment."
-#endif
   struct buffer munged_buffer = *in_buffer;
   struct buffer *buffer = &munged_buffer;
 
@@ -2906,9 +2832,6 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
 static dump_off
 dump_bool_vector (struct dump_context *ctx, const struct Lisp_Vector *v)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Vector_3091289B35)
-# error "Lisp_Vector changed. See CHECK_STRUCTS comment."
-#endif
   /* No relocation needed, so we don't need dump_object_start.  */
   dump_align_output (ctx, DUMP_ALIGNMENT);
   eassert (ctx->offset >= ctx->header.cold_start);
@@ -2923,9 +2846,6 @@ dump_bool_vector (struct dump_context *ctx, const struct Lisp_Vector *v)
 static dump_off
 dump_subr (struct dump_context *ctx, const struct Lisp_Subr *subr)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Subr_594AB72B54)
-# error "Lisp_Subr changed. See CHECK_STRUCTS comment."
-#endif
   struct Lisp_Subr out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, subr, header.size);
@@ -2962,9 +2882,6 @@ dump_vectorlike (struct dump_context *ctx,
                  Lisp_Object lv,
                  dump_off offset)
 {
-#if CHECK_STRUCTS && !defined (HASH_pvec_type_549C833A54)
-# error "pvec_type changed. See CHECK_STRUCTS comment."
-#endif
   const struct Lisp_Vector *v = XVECTOR (lv);
   switch (PSEUDOVECTOR_TYPE (v))
     {
@@ -3072,9 +2989,6 @@ dump_vectorlike (struct dump_context *ctx,
 static dump_off
 dump_object (struct dump_context *ctx, Lisp_Object object)
 {
-#if CHECK_STRUCTS && !defined (HASH_Lisp_Type_E2AD97D3F7)
-# error "Lisp_Type changed. See CHECK_STRUCTS comment."
-#endif
 #ifdef ENABLE_CHECKING
   /* Vdead is extern only when ENABLE_CHECKING.  */
   eassert (!EQ (object, Vdead));
@@ -3177,9 +3091,6 @@ dump_object_for_offset (struct dump_context *ctx, Lisp_Object object)
 static dump_off
 dump_charset (struct dump_context *ctx, int cs_i)
 {
-#if CHECK_STRUCTS && !defined (HASH_charset_317C49E291)
-# error "charset changed. See CHECK_STRUCTS comment."
-#endif
   dump_align_output (ctx, alignof (int));
   const struct charset *cs = charset_table + cs_i;
   struct charset out;
-- 
2.20.1

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Andy Moreton]

On Tue 09 Apr 2019, Paul Eggert wrote:

> On 3/4/19 6:17 PM, Paul Eggert wrote:
>> On 2/28/19 12:59 PM, Alan Mackenzie wrote:
>>> is all this really needed?  Is pdumper.c really that fragile,
>>> that it can't cope with changes in certain structs?
>> No, it's not needed, and in my experience the mechanism's costs far
>> exceed any benefit.
>
> No further comment and the mechanism just bit me again, so I installed
> the attached patch to disable it. We can reenable it later if needed
> (which I hope won't happen....).

This patch seems to be ok, but your following patch in commit d826037475
("Remove the need for temacs.in") breaks out of tree builds.

It seems to me that the patch contains a mixture of changes to remove
temacs.in support, and a number of unrelated changes to Makefile.in
files which break the build.

For example, this breaks out-of-tree builds, as make tries to use
/path/to/emacs/admin/charsets/Makefile rather than the correct path
<builddir>/admin/charsets/Makefile:

    diff --git a/src/Makefile.in b/src/Makefile.in
    index 0613a0dbed..f8a2ffadc2 100644
    --- a/src/Makefile.in
    +++ b/src/Makefile.in
    @@ -533,7 +533,7 @@ ${lispintdir}/cp51932.el ${lispintdir}/eucjp-ms.el:

     charsets = ${top_srcdir}/admin/charsets/charsets.stamp
     ${charsets}: FORCE
    -	${MAKE} -C ../admin/charsets all
    +	$(MAKE) -C $(dir $@) all

     charscript = ${lispintdir}/charscript.el
     ${charscript}: FORCE

The unrelated changes should have been committed in a separate patch
for easier bisection: a patch should contain a single logical change.

Please revert all of the unrelated makefile path handling changes so
master is buildable (the example above is one of many breakages).

    AndyM

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Andy Moreton]

On Wed 10 Apr 2019, Andy Moreton wrote:

> On Tue 09 Apr 2019, Paul Eggert wrote:
>
>> On 3/4/19 6:17 PM, Paul Eggert wrote:
>>> On 2/28/19 12:59 PM, Alan Mackenzie wrote:
>>>> is all this really needed?  Is pdumper.c really that fragile,
>>>> that it can't cope with changes in certain structs?
>>> No, it's not needed, and in my experience the mechanism's costs far
>>> exceed any benefit.
>>
>> No further comment and the mechanism just bit me again, so I installed
>> the attached patch to disable it. We can reenable it later if needed
>> (which I hope won't happen....).
>
> This patch seems to be ok, but your following patch in commit d826037475
> ("Remove the need for temacs.in") breaks out of tree builds.
>
> It seems to me that the patch contains a mixture of changes to remove
> temacs.in support, and a number of unrelated changes to Makefile.in
> files which break the build.
>
> For example, this breaks out-of-tree builds, as make tries to use
> /path/to/emacs/admin/charsets/Makefile rather than the correct path
> <builddir>/admin/charsets/Makefile:
>
>     diff --git a/src/Makefile.in b/src/Makefile.in
>     index 0613a0dbed..f8a2ffadc2 100644
>     --- a/src/Makefile.in
>     +++ b/src/Makefile.in
>     @@ -533,7 +533,7 @@ ${lispintdir}/cp51932.el ${lispintdir}/eucjp-ms.el:
>
>      charsets = ${top_srcdir}/admin/charsets/charsets.stamp
>      ${charsets}: FORCE
>     -	${MAKE} -C ../admin/charsets all
>     +	$(MAKE) -C $(dir $@) all
>
>      charscript = ${lispintdir}/charscript.el
>      ${charscript}: FORCE
>
> The unrelated changes should have been committed in a separate patch
> for easier bisection: a patch should contain a single logical change.
>
> Please revert all of the unrelated makefile path handling changes so
> master is buildable (the example above is one of many breakages).

The following changes (a partial revert of commit d826037475) make it
possible to build emacs out of tree on Windows (msys2 mingw64):


diff --git a/src/Makefile.in b/src/Makefile.in
index f8a2ffadc2..6816b8bf8d 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -533,7 +533,7 @@ ${lispintdir}/cp51932.el ${lispintdir}/eucjp-ms.el:
 
 charsets = ${top_srcdir}/admin/charsets/charsets.stamp
 ${charsets}: FORCE
-	$(MAKE) -C $(dir $@) all
+	$(MAKE) -C ../admin/charsets all
 
 charscript = ${lispintdir}/charscript.el
 ${charscript}: FORCE
@@ -586,7 +586,7 @@ $(etc)/DOC:
 
 $(libsrc)/make-docfile$(EXEEXT) $(libsrc)/make-fingerprint$(EXEEXT): \
   $(lib)/libgnu.a
-	$(MAKE) -C $(dir $@) $(notdir $@)
+	$(MAKE) -C $(libsrc) make-docfile$(EXEEXT)
 
 buildobj.h: Makefile
 	$(AM_V_GEN)for i in $(ALLOBJS); do \
@@ -614,7 +614,7 @@ $(ALLOBJS):
 LIBEGNU_ARCHIVE = $(lib)/lib$(if $(HYBRID_MALLOC),e)gnu.a
 
 $(LIBEGNU_ARCHIVE): $(config_h)
-	$(MAKE) -C $(dir $@) all
+	$(MAKE) -C $(lib) all
 
 FINGERPRINTED = $(LIBXMENU) $(ALLOBJS) $(LIBEGNU_ARCHIVE) $(EMACSRES)
 fingerprint.c: $(FINGERPRINTED) $(libsrc)/make-fingerprint$(EXEEXT)
@@ -639,15 +639,15 @@ temacs$(EXEEXT):
 ## The following oldxmenu-related rules are only (possibly) used if
 ## HAVE_X11 && !USE_GTK, but there is no harm in always defining them.
 $(lwlibdir)/liblw.a: $(config_h) globals.h lisp.h FORCE
-	$(MAKE) -C $(dir $@) $(notdir $@)
+	$(MAKE) -C $(lwlibdir) liblw.a
 $(oldXMenudir)/libXMenu11.a: FORCE
-	$(MAKE) -C $(dir $@) $(notdir $@)
+	$(MAKE) -C -C $(oldXMenudir) libXMenu11.a
 FORCE:
 .PHONY: FORCE
 
 .PRECIOUS: ../config.status Makefile
 ../config.status: $(top_srcdir)/configure.ac $(top_srcdir)/m4/*.m4
-	$(MAKE) -C $(dir $@) $(notdir $@)
+	$(MAKE) -C .. $(notdir $@)
 Makefile: ../config.status $(srcdir)/Makefile.in
 	$(MAKE) -C .. src/$@
 
@@ -703,7 +703,7 @@ extraclean:
 ETAGS = ../lib-src/etags${EXEEXT}
 
 ${ETAGS}: FORCE
-	$(MAKE) -C $(dir $@) $(notdir $@)
+	$(MAKE) -C ../lib-src $(notdir $@)
 
 # Remove macuvs.h and fingerprint.c since they'd cause `src/emacs`
 # to be built before we can get TAGS.
@@ -728,8 +728,11 @@ TAGS:
 
 ## Arrange to make tags tables for ../lisp and ../lwlib,
 ## which the above TAGS file for the C files includes by reference.
-../lisp/TAGS $(lwlibdir)/TAGS: FORCE
-	$(MAKE) -C $(dir $@) $(notdir $@) ETAGS="$(ETAGS)"
+../lisp/TAGS: FORCE
+	$(MAKE) -C ../lisp TAGS ETAGS="$(ETAGS)"
+
+$(lwlibdir)/TAGS: FORCE
+	$(MAKE) -C $(lwlibdir) TAGS ETAGS="$(ETAGS)"
 
 tags: TAGS ../lisp/TAGS $(lwlibdir)/TAGS
 .PHONY: tags
@@ -765,7 +768,7 @@ VCSWITNESS =
 
 $(lispsource)/loaddefs.el: $(VCSWITNESS) | \
 		bootstrap-emacs$(EXEEXT) $(bootstrap_pdmp)
-	$(MAKE) -C $(dir $@) autoloads EMACS="$(bootstrap_exe)"
+	$(MAKE) -C ../lisp autoloads EMACS="$(bootstrap_exe)"
 
 ## Dump an Emacs executable named bootstrap-emacs containing the
 ## files from loadup.el in source form.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

On 4/10/19 6:12 AM, Andy Moreton wrote:
> this breaks out-of-tree builds, as make tries to use
> /path/to/emacs/admin/charsets/Makefile rather than the correct path
> <builddir>/admin/charsets/Makefile:

Sorry, my tests didn't include a complete out-of-tree bootstrap. I
installed the attached patch to fix that problem, along with the
problem's other instance. The other -C changes should be OK. You're
correct that some of the -C changes were unrelated to the original
patch. Still, they make the Makefile more consistent and I'd like to
keep the ones that work.

PS. I'd like Emacs to switch to a nonrecursive 'make' process, as this
subsidiary $(MAKE) -C business is for the birds. We've made the switch
for other GNU projects and it's been a win. This is not a new idea; see:

Miller P. Recursive Make considered harmful. AAUGN J of AUUG.
1998;19(1):14-25. https://grosskurth.ca/bib/1997/miller.pdf


[-- Attachment #2: 0001-Fix-MAKE-C-for-out-of-tree-bootstraps.patch --]
[-- Type: text/x-patch, Size: 1319 bytes --]

From d2255c6065b0bc3949d494edf8864a2bd13918f3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Apr 2019 10:06:21 -0700
Subject: [PATCH] Fix $(MAKE) -C for out-of-tree bootstraps

Problem reported by Andy Moreton in:
https://lists.gnu.org/r/emacs-devel/2019-04/msg00359.html
* src/Makefile.in (${charsets}, $(lispsource)/loaddefs.el):
Revert incorrect changes to $(MAKE) -C invocations when the
target is in the source tree not the build tree.
---
 src/Makefile.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.in b/src/Makefile.in
index f8a2ffadc2..6d6308fde6 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -533,7 +533,7 @@ ${lispintdir}/cp51932.el ${lispintdir}/eucjp-ms.el:
 
 charsets = ${top_srcdir}/admin/charsets/charsets.stamp
 ${charsets}: FORCE
-	$(MAKE) -C $(dir $@) all
+	$(MAKE) -C ../admin/charsets all
 
 charscript = ${lispintdir}/charscript.el
 ${charscript}: FORCE
@@ -765,7 +765,7 @@ VCSWITNESS =
 
 $(lispsource)/loaddefs.el: $(VCSWITNESS) | \
 		bootstrap-emacs$(EXEEXT) $(bootstrap_pdmp)
-	$(MAKE) -C $(dir $@) autoloads EMACS="$(bootstrap_exe)"
+	$(MAKE) -C ../lisp autoloads EMACS="$(bootstrap_exe)"
 
 ## Dump an Emacs executable named bootstrap-emacs containing the
 ## files from loadup.el in source form.
-- 
2.20.1

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Andy Moreton]

On Wed 10 Apr 2019, Paul Eggert wrote:

> On 4/10/19 6:12 AM, Andy Moreton wrote:
>> this breaks out-of-tree builds, as make tries to use
>> /path/to/emacs/admin/charsets/Makefile rather than the correct path
>> <builddir>/admin/charsets/Makefile:
>
> Sorry, my tests didn't include a complete out-of-tree bootstrap. I
> installed the attached patch to fix that problem, along with the
> problem's other instance. The other -C changes should be OK. You're
> correct that some of the -C changes were unrelated to the original
> patch. Still, they make the Makefile more consistent and I'd like to
> keep the ones that work.

The minimal fixes you have committed work for me for a bootstrap build
from a completely clean tree (after 'git clean -Xdf').

I'm not convinced that computing filenames using GNU make functions
rather than using literal values is clearer, and the makefiles include
both styles, so more work is needed to achieve consistency using either
approach.

> PS. I'd like Emacs to switch to a nonrecursive 'make' process, as this
> subsidiary $(MAKE) -C business is for the birds. We've made the switch
> for other GNU projects and it's been a win. This is not a new idea; see:
>
> Miller P. Recursive Make considered harmful. AAUGN J of AUUG.
> 1998;19(1):14-25. https://grosskurth.ca/bib/1997/miller.pdf

This is a good idea, and should result in better dependency tracking and
possibly faster parallel builds. Please make sure it works for in tree
and out of tree builds, as both are useful.

    AndyM

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

> On Wed 10 Apr 2019, Paul Eggert wrote:
>
>> On 4/10/19 6:12 AM, Andy Moreton wrote:
>>> this breaks out-of-tree builds, as make tries to use
>>> /path/to/emacs/admin/charsets/Makefile rather than the correct path
>>> <builddir>/admin/charsets/Makefile:
>>
>> Sorry, my tests didn't include a complete out-of-tree bootstrap. I
>> installed the attached patch to fix that problem, along with the
>> problem's other instance. The other -C changes should be OK. You're
>> correct that some of the -C changes were unrelated to the original
>> patch. Still, they make the Makefile more consistent and I'd like to
>> keep the ones that work.
>
> The minimal fixes you have committed work for me for a bootstrap build
> from a completely clean tree (after 'git clean -Xdf').
>
> I'm not convinced that computing filenames using GNU make functions
> rather than using literal values is clearer, and the makefiles include
> both styles, so more work is needed to achieve consistency using either
> approach.
>
>> PS. I'd like Emacs to switch to a nonrecursive 'make' process, as this
>> subsidiary $(MAKE) -C business is for the birds. We've made the switch
>> for other GNU projects and it's been a win. This is not a new idea; see:
>>
>> Miller P. Recursive Make considered harmful. AAUGN J of AUUG.
>> 1998;19(1):14-25. https://grosskurth.ca/bib/1997/miller.pdf
>
> This is a good idea, and should result in better dependency tracking and
> possibly faster parallel builds. Please make sure it works for in tree
> and out of tree builds, as both are useful.

Agreed on moving to nonrecursive make.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Paul.

On Tue, Apr 09, 2019 at 15:47:17 -0700, Paul Eggert wrote:
> On 3/4/19 6:17 PM, Paul Eggert wrote:
> > On 2/28/19 12:59 PM, Alan Mackenzie wrote:
> >> is all this really needed?  Is pdumper.c really that fragile,
> >> that it can't cope with changes in certain structs?
> > No, it's not needed, and in my experience the mechanism's costs far
> > exceed any benefit.

> No further comment and the mechanism just bit me again, so I installed
> the attached patch to disable it.

Why?  Completely removing this mechanism seems very heavy handed.  All
that was needed to disable it was a single line change to set
CHECK_STRUCTS to zero.

You would have got further comment if you'd proposed a patch, rather
than just a vague idea that might or might not have been followed
through.  Discussing the merits of a patch is always best done before
committing it to master, not afterwards.

What does Daniel say?

> We can reenable it later if needed (which I hope won't happen....).

Funnily enough, it was of use to me recently when it reminded me to
amend dump_subr after extending struct subr.

My main problem with this mechanism was the vagueness of the error
messages it generated and of the comment in the code they pointed at,
which wasted lots of time.  Those defects have since been fixed.

> >>From 891e507d06c3bfcd9ac181de6bb0ff9c27dfa4aa Mon Sep 17 00:00:00 2001
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 9 Apr 2019 15:42:10 -0700
> Subject: [PATCH] Remove dmpstruct.h
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> The hassles of updating the dmpstruct.h-using code bit me again.
> These updates are more trouble than they???re worth.  See:
> https://lists.gnu.org/r/emacs-devel/2019-03/msg00122.html
> As I???m the main person who???s made changes in this area since
> dmpstruct.h was introduced, I???m the most motivated to clean up
> the situation.
> * make-dist (possibly_non_vc_files): Remove src/dmpstruct.h.
> * src/Makefile.in (dmpstruct_headers, dmpstruct.h): Remove.
> (pdumper.o): Do not depend on dmpstruct.h.
> (mostlyclean): Do not remove dmpstruct.h.
> * src/dmpstruct.awk: Remove.
> * src/pdumper.c: Do not include dmpstruct.h.
> (CHECK_STRUCTS): Remove.  All uses removed.
> ---
>  .gitignore        |  1 -
>  make-dist         |  2 +-
>  src/Makefile.in   | 10 +-----
>  src/dmpstruct.awk | 45 ------------------------
>  src/pdumper.c     | 89 -----------------------------------------------
>  5 files changed, 2 insertions(+), 145 deletions(-)
>  delete mode 100755 src/dmpstruct.awk

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/10/19 9:22 AM, Alan Mackenzie wrote:
> All that was needed to disable it was a single line change to set
> CHECK_STRUCTS to zero.

That would also have sufficed, but the code clutter (and slowdown in the
build due to the computation of hashes which forces that part of the
build to be sequential) was also annoying. If you really want it we can
bring back the mechanism (with CHECK_STRUCTS off by default please!),
though I'd rather not - see below.

> it was of use to me recently when it reminded me to
> amend dump_subr after extending struct subr.
I didn't observe that in master - presumably this was in some
experimental branch? In master, dump_subr is currently the same as it
was when the pdumper was installed.

Before making the recent change, I reviewed all the changes made to
pdumper.c in master, and observed none where the hashes helped and
several where they hurt. Since the portable dumper was added I have made
four commits that involved the hashes, and the hashes only got into my
way and slowed me down. You made one commit
(9c0fa1172fd987a8f23b115145270383a11c12fc) that involved the buffer.h
hash, and portions of that commit were mistakenly pushed in pdumper.c's
previous commit so the hashes didn't seem to have helped there. The only
other persons to make hash-related commits to pdumper.c were Alan and
Stefan, and the hashes didn't help there either.

As the hashes get in the way of ordinary development (mostly affecting
me) and don't seem to help in practice, I really want them to go.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Paul.

On Wed, Apr 10, 2019 at 11:05:02 -0700, Paul Eggert wrote:
> On 4/10/19 9:22 AM, Alan Mackenzie wrote:
> > All that was needed to disable it was a single line change to set
> > CHECK_STRUCTS to zero.

> That would also have sufficed, but the code clutter (and slowdown in the
> build due to the computation of hashes which forces that part of the
> build to be sequential) was also annoying. If you really want it we can
> bring back the mechanism (with CHECK_STRUCTS off by default please!),
> though I'd rather not - see below.

According to Daniel (and my experience backs it up), the problems
CHECK_STRUCTS detects happen when merging master into a branch.  I'm not
sure you do such merges much.  But if you are making frequent changes to
the structures guarded by the mechanism, having deleted that mechanism,
you are saving effort for yourself, but imposing extra effort on other
people.

> > it was of use to me recently when it reminded me to
> > amend dump_subr after extending struct subr.
> I didn't observe that in master - presumably this was in some
> experimental branch? In master, dump_subr is currently the same as it
> was when the pdumper was installed.

Naturally.  I don't change basic structures in master without (usually
extensive) discussion on emacs-devel.  The changes were in branch
scratch/accurate-warning-pos.  The hashes were helpful in making these
changes.

> Before making the recent change, I reviewed all the changes made to
> pdumper.c in master, and observed none where the hashes helped and
> several where they hurt. Since the portable dumper was added I have made
> four commits that involved the hashes, and the hashes only got into my
> way and slowed me down.

The hashes do slow us down, yes.  But not really by very much, IMAO.
For example I timed make dmpstruct.h, and it only took 0.16s.  The extra
time taken in a build can't be more than a very small number of seconds.

Having a build aborted by CHECK_STRUCTS, and having manually to change
the hashes in pdumper.c is what takes the time.  But really, how often
are we changing those structures of an evening?

> You made one commit (9c0fa1172fd987a8f23b115145270383a11c12fc) that
> involved the buffer.h hash, and portions of that commit were
> mistakenly pushed in pdumper.c's previous commit so the hashes didn't
> seem to have helped there.

That was actually Eli's commit.

> The only other persons to make hash-related commits to pdumper.c were
> Alan and Stefan, and the hashes didn't help there either.

I think the hashes are a hindrance when developing on master, but a help
on branches (when merging from master).

> As the hashes get in the way of ordinary development (mostly affecting
> me) and don't seem to help in practice, I really want them to go.

I think I would prefer the hashes to stay, but I can see the other side
of the argument, too.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

> Hello, Paul.
>
> On Wed, Apr 10, 2019 at 11:05:02 -0700, Paul Eggert wrote:
>> On 4/10/19 9:22 AM, Alan Mackenzie wrote:
>> > All that was needed to disable it was a single line change to set
>> > CHECK_STRUCTS to zero.
>
>> That would also have sufficed, but the code clutter (and slowdown in the
>> build due to the computation of hashes which forces that part of the
>> build to be sequential) was also annoying. If you really want it we can
>> bring back the mechanism (with CHECK_STRUCTS off by default please!),
>> though I'd rather not - see below.
>
> According to Daniel (and my experience backs it up), the problems
> CHECK_STRUCTS detects happen when merging master into a branch.  I'm not
> sure you do such merges much.  But if you are making frequent changes to
> the structures guarded by the mechanism, having deleted that mechanism,
> you are saving effort for yourself, but imposing extra effort on other
> people.
>
>> > it was of use to me recently when it reminded me to
>> > amend dump_subr after extending struct subr.
>> I didn't observe that in master - presumably this was in some
>> experimental branch? In master, dump_subr is currently the same as it
>> was when the pdumper was installed.
>
> Naturally.  I don't change basic structures in master without (usually
> extensive) discussion on emacs-devel.  The changes were in branch
> scratch/accurate-warning-pos.  The hashes were helpful in making these
> changes.
>
>> Before making the recent change, I reviewed all the changes made to
>> pdumper.c in master, and observed none where the hashes helped and
>> several where they hurt. Since the portable dumper was added I have made
>> four commits that involved the hashes, and the hashes only got into my
>> way and slowed me down.
>
> The hashes do slow us down, yes.  But not really by very much, IMAO.
> For example I timed make dmpstruct.h, and it only took 0.16s.  The extra
> time taken in a build can't be more than a very small number of seconds.
>
> Having a build aborted by CHECK_STRUCTS, and having manually to change
> the hashes in pdumper.c is what takes the time.  But really, how often
> are we changing those structures of an evening?
>
>> You made one commit (9c0fa1172fd987a8f23b115145270383a11c12fc) that
>> involved the buffer.h hash, and portions of that commit were
>> mistakenly pushed in pdumper.c's previous commit so the hashes didn't
>> seem to have helped there.
>
> That was actually Eli's commit.
>
>> The only other persons to make hash-related commits to pdumper.c were
>> Alan and Stefan, and the hashes didn't help there either.
>
> I think the hashes are a hindrance when developing on master, but a help
> on branches (when merging from master).
>
>> As the hashes get in the way of ordinary development (mostly affecting
>> me) and don't seem to help in practice, I really want them to go.
>
> I think I would prefer the hashes to stay, but I can see the other side
> of the argument, too.

What would make the hashes easier to deal with? Some make target for
updating them automatically? An easier way to disable the check at
configure time? Transformation into a prominent runtime warning instead of
a build break?

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

Daniel Colascione wrote:
> What would make the hashes easier to deal with? Some make target for
> updating them automatically? An easier way to disable the check at
> configure time? Transformation into a prominent runtime warning instead of
> a build break?

It's better to have the build break than to have a runtime check. Of the two 
other approaches you mentioned, I think a configure-time option is the better 
idea. Since these hashes are aimed at developers and are are not needed for 
ordinary builds from tarballs, I think the hashes should be checked on request. 
I resurrected the hashes by installing the attached patch and they are now 
checked with the longstanding 'configure' option --enable-checking=all or with 
the new 'configure' option --enable-checking=structs. Comments welcome as usual.

[-- Attachment #2: 0001-Bring-back-dmpstruct.h.patch --]
[-- Type: text/x-patch, Size: 17921 bytes --]

From 9994bf17cf532f2e1d4310341da7180342202191 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Apr 2019 19:42:37 -0700
Subject: [PATCH] Bring back dmpstruct.h

Bring back the dmpstruct.h checking, and use it when
--enable-checking=structs is specified.  The checking can be helpful
to some developers, although it gets in the way of others and is
not needed for ordinary tarball builds.
* src/dmpstruct.awk: Restore this file, with mode 644 not 755.
* configure.ac: New option-arg --enable-checking=structs,
implied by --enable-checking.
(CHECK_STRUCTS): New macro and var.
* src/Makefile.in (CHECK_STRUCTS): New macro.
(dmpstruct_headers, dmpstruct.h, dmpstruct.h):
Restore these macros and rules.
(pdumper.o): Restore this dependency if $(CHECK_STRUCTS) is true.
(mostlyclean): Remove dmpstruct.h.
* src/pdumper.c [CHECK_STRUCTS]: Include dmpstruct.h,
and restore checks against hashes.
---
 .gitignore        |  1 +
 configure.ac      | 17 ++++++++--
 etc/NEWS          |  5 +++
 src/Makefile.in   | 13 +++++++-
 src/dmpstruct.awk | 45 ++++++++++++++++++++++++++
 src/pdumper.c     | 81 +++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 159 insertions(+), 3 deletions(-)
 create mode 100644 src/dmpstruct.awk

diff --git a/.gitignore b/.gitignore
index 98b8222180..88b29760b7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -186,6 +186,7 @@ src/emacs
 src/emacs-[0-9]*
 src/temacs
 src/fingerprint.c
+src/dmpstruct.h
 src/*.pdmp
 
 # Character-set info.
diff --git a/configure.ac b/configure.ac
index c93cfbbb59..1814a30cbc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -537,19 +537,21 @@ AC_DEFUN
 
 AC_ARG_ENABLE(checking,
 [AS_HELP_STRING([--enable-checking@<:@=LIST@:>@],
-		[enable expensive run-time checks.  With LIST,
+		[enable expensive checks.  With LIST,
 		 enable only specific categories of checks.
 		 Categories are: all,yes,no.
 		 Flags are: stringbytes, stringoverrun, stringfreelist,
-		 xmallocoverrun, conslist, glyphs])],
+		 structs, xmallocoverrun, conslist, glyphs])],
 [ac_checking_flags="${enableval}"],[])
 IFS="${IFS= 	}"; ac_save_IFS="$IFS"; IFS="$IFS,"
+CHECK_STRUCTS=false
 for check in $ac_checking_flags
 do
 	case $check in
 	# these set all the flags to specific states
 	yes)		ac_enable_checking=1 ;;
 	no)		ac_enable_checking= ;
+			CHECK_STRUCTS=false
 			ac_gc_check_stringbytes= ;
 	                ac_gc_check_string_overrun= ;
 	                ac_gc_check_string_free_list= ;
@@ -557,6 +559,7 @@ AC_DEFUN
 	                ac_gc_check_cons_list= ;
 			ac_glyphs_debug= ;;
 	all)		ac_enable_checking=1 ;
+			CHECK_STRUCTS=true
 			ac_gc_check_stringbytes=1 ;
 	                ac_gc_check_string_overrun=1 ;
 	                ac_gc_check_string_free_list=1 ;
@@ -567,6 +570,7 @@ AC_DEFUN
 	stringbytes)	ac_gc_check_stringbytes=1 ;;
 	stringoverrun)	ac_gc_check_string_overrun=1 ;;
 	stringfreelist) ac_gc_check_string_free_list=1 ;;
+	structs)	CHECK_STRUCTS=true ;;
 	xmallocoverrun)	ac_xmalloc_overrun=1 ;;
 	conslist)	ac_gc_check_cons_list=1 ;;
 	glyphs)		ac_glyphs_debug=1 ;;
@@ -579,6 +583,15 @@ AC_DEFUN
   AC_DEFINE(ENABLE_CHECKING, 1,
 [Define to 1 if expensive run-time data type and consistency checks are enabled.])
 fi
+if $CHECK_STRUCTS; then
+  AC_DEFINE([CHECK_STRUCTS], 1,
+    [Define this to check whether someone updated the portable dumper
+     code after changing the layout of a structure that it uses.
+     If you change one of these structures, check that the pdumper.c
+     code is still valid, and update the pertinent hash in pdumper.c
+     by manually copying the hash from the newly-generated dmpstruct.h.])
+fi
+AC_SUBST([CHECK_STRUCTS])
 if test x$ac_gc_check_stringbytes != x ; then
   AC_DEFINE(GC_CHECK_STRING_BYTES, 1,
 [Define this temporarily to hunt a bug.  If defined, the size of
diff --git a/etc/NEWS b/etc/NEWS
index fbde6e0b66..9644c1ca22 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -84,6 +84,11 @@ The new command-line argument '--dump-file=FILE' allows to specify a
 non-default '.pdmp' file to load the state from; see the node "Initial
 Options" in the Emacs manual for more information.
 
++++
+** The new configure option '--enable-checking=structs' attempts to
+check that the portable dumper code has been updated to match the last
+change to one of the data structures that it relies on.
+
 \f
 * Startup Changes in Emacs 27.1
 
diff --git a/src/Makefile.in b/src/Makefile.in
index 6d6308fde6..2348c8dae4 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -331,6 +331,7 @@ BUILD_DETAILS =
 UNEXEC_OBJ = @UNEXEC_OBJ@
 
 DUMPING=@DUMPING@
+CHECK_STRUCTS = @CHECK_STRUCTS@
 
 # 'make' verbosity.
 AM_DEFAULT_VERBOSITY = @AM_DEFAULT_VERBOSITY@
@@ -456,6 +457,16 @@ ALLOBJS =
 all: emacs$(EXEEXT) $(pdmp) $(OTHER_FILES)
 .PHONY: all
 
+dmpstruct_headers=$(srcdir)/lisp.h $(srcdir)/buffer.h \
+	$(srcdir)/intervals.h $(srcdir)/charset.h $(srcdir)/bignum.h
+ifeq ($(CHECK_STRUCTS),true)
+pdumper.o: dmpstruct.h
+endif
+dmpstruct.h: $(srcdir)/dmpstruct.awk
+dmpstruct.h: $(libsrc)/make-fingerprint$(EXEEXT) $(dmpstruct_headers)
+	$(AM_V_GEN)POSIXLY_CORRECT=1 awk -f $(srcdir)/dmpstruct.awk \
+		$(dmpstruct_headers) > $@
+
 AUTO_DEPEND = @AUTO_DEPEND@
 DEPDIR = deps
 ifeq ($(AUTO_DEPEND),yes)
@@ -665,7 +676,7 @@ .PHONY:
 
 mostlyclean:
 	rm -f temacs$(EXEEXT) core ./*.core \#* ./*.o
-	rm -f fingerprint.c
+	rm -f dmpstruct.h fingerprint.c
 	rm -f emacs.pdmp
 	rm -f ../etc/DOC
 	rm -f bootstrap-emacs$(EXEEXT) $(bootstrap_pdmp)
diff --git a/src/dmpstruct.awk b/src/dmpstruct.awk
new file mode 100644
index 0000000000..55626cf8b2
--- /dev/null
+++ b/src/dmpstruct.awk
@@ -0,0 +1,45 @@
+# Copyright (C) 2018-2019 Free Software Foundation, Inc.
+#
+# This file is part of GNU Emacs.
+#
+# GNU Emacs is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or (at
+# your option) any later version.
+#
+# GNU Emacs is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+BEGIN {
+  print "/* Generated by dmpstruct.awk */"
+  print "#ifndef EMACS_DMPSTRUCT_H"
+  print "#define EMACS_DMPSTRUCT_H"
+  struct_name = ""
+  tmpfile = "dmpstruct.tmp"
+}
+# Match a type followed by optional syntactic whitespace
+/^(enum|struct|union) [a-zA-Z0-9_]+([\t ]|\/\*.*\*\/)*$/ {
+  struct_name = $2
+  close (tmpfile)
+}
+/^(enum|struct|union) [a-zA-Z0-9_]+([\t ]|\/\*.*\*\/)*$/, /^(  )?};$/ {
+  print $0 > tmpfile
+}
+/^(  )?} *(GCALIGNED_STRUCT)? *;$/ {
+  if (struct_name != "") {
+    fflush (tmpfile)
+    cmd = "../lib-src/make-fingerprint -r " tmpfile
+    cmd | getline hash
+    close (cmd)
+    printf "#define HASH_%s_%.10s\n", struct_name, hash
+    struct_name = ""
+  }
+}
+END {
+  print "#endif /* EMACS_DMPSTRUCT_H */"
+}
diff --git a/src/pdumper.c b/src/pdumper.c
index 3aa941221d..600c5b3ca3 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -46,6 +46,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "thread.h"
 #include "bignum.h"
 
+#ifdef CHECK_STRUCTS
+# include "dmpstruct.h"
+#endif
+
 /*
   TODO:
 
@@ -2029,6 +2033,9 @@ dump_pseudovector_lisp_fields (struct dump_context *ctx,
 static dump_off
 dump_cons (struct dump_context *ctx, const struct Lisp_Cons *cons)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Cons_00EEE63F67)
+# error "Lisp_Cons changed. See CHECK_STRUCTS comment."
+#endif
   struct Lisp_Cons out;
   dump_object_start (ctx, &out, sizeof (out));
   dump_field_lv (ctx, &out, cons, &cons->u.s.car, WEIGHT_STRONG);
@@ -2041,6 +2048,9 @@ dump_interval_tree (struct dump_context *ctx,
                     INTERVAL tree,
                     dump_off parent_offset)
 {
+#if CHECK_STRUCTS && !defined (HASH_interval_1B38941C37)
+# error "interval changed. See CHECK_STRUCTS comment."
+#endif
   /* TODO: output tree breadth-first?  */
   struct interval out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2082,6 +2092,9 @@ dump_interval_tree (struct dump_context *ctx,
 static dump_off
 dump_string (struct dump_context *ctx, const struct Lisp_String *string)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_String_86FEA6EC7C)
+# error "Lisp_String changed. See CHECK_STRUCTS comment."
+#endif
   /* If we have text properties, write them _after_ the string so that
      at runtime, the prefetcher and cache will DTRT. (We access the
      string before its properties.).
@@ -2125,6 +2138,10 @@ dump_string (struct dump_context *ctx, const struct Lisp_String *string)
 static dump_off
 dump_marker (struct dump_context *ctx, const struct Lisp_Marker *marker)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Marker_642DBAF866)
+# error "Lisp_Marker changed. See CHECK_STRUCTS comment."
+#endif
+
   START_DUMP_PVEC (ctx, &marker->header, struct Lisp_Marker, out);
   dump_pseudovector_lisp_fields (ctx, &out->header, &marker->header);
   DUMP_FIELD_COPY (out, marker, need_adjustment);
@@ -2144,6 +2161,9 @@ dump_marker (struct dump_context *ctx, const struct Lisp_Marker *marker)
 static dump_off
 dump_overlay (struct dump_context *ctx, const struct Lisp_Overlay *overlay)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Overlay_72EADA9882)
+# error "Lisp_Overlay changed. See CHECK_STRUCTS comment."
+#endif
   START_DUMP_PVEC (ctx, &overlay->header, struct Lisp_Overlay, out);
   dump_pseudovector_lisp_fields (ctx, &out->header, &overlay->header);
   dump_field_lv_rawptr (ctx, out, overlay, &overlay->next,
@@ -2169,6 +2189,9 @@ static dump_off
 dump_finalizer (struct dump_context *ctx,
                 const struct Lisp_Finalizer *finalizer)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Finalizer_D58E647CB8)
+# error "Lisp_Finalizer changed. See CHECK_STRUCTS comment."
+#endif
   START_DUMP_PVEC (ctx, &finalizer->header, struct Lisp_Finalizer, out);
   /* Do _not_ call dump_pseudovector_lisp_fields here: we dump the
      only Lisp field, finalizer->function, manually, so we can give it
@@ -2188,6 +2211,9 @@ struct bignum_reload_info
 static dump_off
 dump_bignum (struct dump_context *ctx, Lisp_Object object)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Bignum_661945DE2B)
+# error "Lisp_Bignum changed. See CHECK_STRUCTS comment."
+#endif
   const struct Lisp_Bignum *bignum = XBIGNUM (object);
   START_DUMP_PVEC (ctx, &bignum->header, struct Lisp_Bignum, out);
   verify (sizeof (out->value) >= sizeof (struct bignum_reload_info));
@@ -2223,6 +2249,9 @@ dump_bignum (struct dump_context *ctx, Lisp_Object object)
 static dump_off
 dump_float (struct dump_context *ctx, const struct Lisp_Float *lfloat)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Float_50A7B216D9)
+# error "Lisp_Float changed. See CHECK_STRUCTS comment."
+#endif
   eassert (ctx->header.cold_start);
   struct Lisp_Float out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2233,6 +2262,9 @@ dump_float (struct dump_context *ctx, const struct Lisp_Float *lfloat)
 static dump_off
 dump_fwd_int (struct dump_context *ctx, const struct Lisp_Intfwd *intfwd)
 {
+#if CHECK_STRUCTS && !defined HASH_Lisp_Intfwd_4D887A7387
+# error "Lisp_Intfwd changed. See CHECK_STRUCTS comment."
+#endif
   dump_emacs_reloc_immediate_intmax_t (ctx, intfwd->intvar, *intfwd->intvar);
   struct Lisp_Intfwd out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2244,6 +2276,9 @@ dump_fwd_int (struct dump_context *ctx, const struct Lisp_Intfwd *intfwd)
 static dump_off
 dump_fwd_bool (struct dump_context *ctx, const struct Lisp_Boolfwd *boolfwd)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Boolfwd_0EA1C7ADCC)
+# error "Lisp_Boolfwd changed. See CHECK_STRUCTS comment."
+#endif
   dump_emacs_reloc_immediate_bool (ctx, boolfwd->boolvar, *boolfwd->boolvar);
   struct Lisp_Boolfwd out;
   dump_object_start (ctx, &out, sizeof (out));
@@ -2255,6 +2290,9 @@ dump_fwd_bool (struct dump_context *ctx, const struct Lisp_Boolfwd *boolfwd)
 static dump_off
 dump_fwd_obj (struct dump_context *ctx, const struct Lisp_Objfwd *objfwd)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Objfwd_45D3E513DC)
+# error "Lisp_Objfwd changed. See CHECK_STRUCTS comment."
+#endif
   if (NILP (Fgethash (dump_off_to_lisp (emacs_offset (objfwd->objvar)),
                       ctx->staticpro_table,
                       Qnil)))
@@ -2270,6 +2308,9 @@ static dump_off
 dump_fwd_buffer_obj (struct dump_context *ctx,
                      const struct Lisp_Buffer_Objfwd *buffer_objfwd)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Buffer_Objfwd_13CA6B04FC)
+# error "Lisp_Buffer_Objfwd changed. See CHECK_STRUCTS comment."
+#endif
   struct Lisp_Buffer_Objfwd out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, buffer_objfwd, type);
@@ -2283,6 +2324,9 @@ static dump_off
 dump_fwd_kboard_obj (struct dump_context *ctx,
                      const struct Lisp_Kboard_Objfwd *kboard_objfwd)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Kboard_Objfwd_CAA7E71069)
+# error "Lisp_Intfwd changed. See CHECK_STRUCTS comment."
+#endif
   struct Lisp_Kboard_Objfwd out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, kboard_objfwd, type);
@@ -2293,6 +2337,9 @@ dump_fwd_kboard_obj (struct dump_context *ctx,
 static dump_off
 dump_fwd (struct dump_context *ctx, lispfwd fwd)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Fwd_Type_9CBA6EE55E)
+# error "Lisp_Fwd_Type changed. See CHECK_STRUCTS comment."
+#endif
   void const *p = fwd.fwdptr;
   dump_off offset;
 
@@ -2324,6 +2371,9 @@ static dump_off
 dump_blv (struct dump_context *ctx,
           const struct Lisp_Buffer_Local_Value *blv)
 {
+#if CHECK_STRUCTS && !defined HASH_Lisp_Buffer_Local_Value_3C363FAC3C
+# error "Lisp_Buffer_Local_Value changed. See CHECK_STRUCTS comment."
+#endif
   struct Lisp_Buffer_Local_Value out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, blv, local_if_set);
@@ -2386,6 +2436,13 @@ dump_symbol (struct dump_context *ctx,
              Lisp_Object object,
              dump_off offset)
 {
+#if CHECK_STRUCTS && !defined HASH_Lisp_Symbol_999DC26DEC
+# error "Lisp_Symbol changed. See CHECK_STRUCTS comment."
+#endif
+#if CHECK_STRUCTS && !defined (HASH_symbol_redirect_ADB4F5B113)
+# error "symbol_redirect changed. See CHECK_STRUCTS comment."
+#endif
+
   if (ctx->flags.defer_symbols)
     {
       if (offset != DUMP_OBJECT_ON_SYMBOL_QUEUE)
@@ -2475,6 +2532,9 @@ static dump_off
 dump_vectorlike_generic (struct dump_context *ctx,
 			 const union vectorlike_header *header)
 {
+#if CHECK_STRUCTS && !defined (HASH_vectorlike_header_00A5A4BFB2)
+# error "vectorlike_header changed. See CHECK_STRUCTS comment."
+#endif
   const struct Lisp_Vector *v = (const struct Lisp_Vector *) header;
   ptrdiff_t size = header->size;
   enum pvec_type pvectype = PSEUDOVECTOR_TYPE (v);
@@ -2632,6 +2692,9 @@ dump_hash_table (struct dump_context *ctx,
                  Lisp_Object object,
                  dump_off offset)
 {
+#if CHECK_STRUCTS && !defined HASH_Lisp_Hash_Table_EF95ED06FF
+# error "Lisp_Hash_Table changed. See CHECK_STRUCTS comment."
+#endif
   const struct Lisp_Hash_Table *hash_in = XHASH_TABLE (object);
   bool is_stable = dump_hash_table_stable_p (hash_in);
   /* If the hash table is likely to be modified in memory (either
@@ -2697,6 +2760,9 @@ dump_hash_table (struct dump_context *ctx,
 static dump_off
 dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
 {
+#if CHECK_STRUCTS && !defined HASH_buffer_E34A11C6B9
+# error "buffer changed. See CHECK_STRUCTS comment."
+#endif
   struct buffer munged_buffer = *in_buffer;
   struct buffer *buffer = &munged_buffer;
 
@@ -2830,6 +2896,9 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
 static dump_off
 dump_bool_vector (struct dump_context *ctx, const struct Lisp_Vector *v)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Vector_3091289B35)
+# error "Lisp_Vector changed. See CHECK_STRUCTS comment."
+#endif
   /* No relocation needed, so we don't need dump_object_start.  */
   dump_align_output (ctx, DUMP_ALIGNMENT);
   eassert (ctx->offset >= ctx->header.cold_start);
@@ -2844,6 +2913,9 @@ dump_bool_vector (struct dump_context *ctx, const struct Lisp_Vector *v)
 static dump_off
 dump_subr (struct dump_context *ctx, const struct Lisp_Subr *subr)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Subr_594AB72B54)
+# error "Lisp_Subr changed. See CHECK_STRUCTS comment."
+#endif
   struct Lisp_Subr out;
   dump_object_start (ctx, &out, sizeof (out));
   DUMP_FIELD_COPY (&out, subr, header.size);
@@ -2880,6 +2952,9 @@ dump_vectorlike (struct dump_context *ctx,
                  Lisp_Object lv,
                  dump_off offset)
 {
+#if CHECK_STRUCTS && !defined (HASH_pvec_type_549C833A54)
+# error "pvec_type changed. See CHECK_STRUCTS comment."
+#endif
   const struct Lisp_Vector *v = XVECTOR (lv);
   switch (PSEUDOVECTOR_TYPE (v))
     {
@@ -2987,6 +3062,9 @@ dump_vectorlike (struct dump_context *ctx,
 static dump_off
 dump_object (struct dump_context *ctx, Lisp_Object object)
 {
+#if CHECK_STRUCTS && !defined (HASH_Lisp_Type_E2AD97D3F7)
+# error "Lisp_Type changed. See CHECK_STRUCTS comment."
+#endif
 #ifdef ENABLE_CHECKING
   /* Vdead is extern only when ENABLE_CHECKING.  */
   eassert (!EQ (object, Vdead));
@@ -3089,6 +3167,9 @@ dump_object_for_offset (struct dump_context *ctx, Lisp_Object object)
 static dump_off
 dump_charset (struct dump_context *ctx, int cs_i)
 {
+#if CHECK_STRUCTS && !defined (HASH_charset_317C49E291)
+# error "charset changed. See CHECK_STRUCTS comment."
+#endif
   dump_align_output (ctx, alignof (int));
   const struct charset *cs = charset_table + cs_i;
   struct charset out;
-- 
2.17.1

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Andy Moreton]

On Wed 10 Apr 2019, Paul Eggert wrote:

> Daniel Colascione wrote:
>> What would make the hashes easier to deal with? Some make target for
>> updating them automatically? An easier way to disable the check at
>> configure time? Transformation into a prominent runtime warning instead of
>> a build break?
>
> It's better to have the build break than to have a runtime check. Of the two
> other approaches you mentioned, I think a configure-time option is the better
> idea. Since these hashes are aimed at developers and are are not needed for
> ordinary builds from tarballs, I think the hashes should be checked on
> request. I resurrected the hashes by installing the attached patch and they
> are now checked with the longstanding 'configure' option --enable-checking=all
> or with the new 'configure' option --enable-checking=structs. Comments welcome
> as usual.

This newly resurrected checking is already broken on master, which does
not build with confgiure option "--enable-checking=yes,glyphs".

The struct fingerprint checking should default to enabled (as before)
with the configure arguments allowing it to be (temporarily) disabled.

    AndyM

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Andy Moreton]

On Mon 15 Apr 2019, Andy Moreton wrote:

> On Wed 10 Apr 2019, Paul Eggert wrote:
>
>> Daniel Colascione wrote:
>>> What would make the hashes easier to deal with? Some make target for
>>> updating them automatically? An easier way to disable the check at
>>> configure time? Transformation into a prominent runtime warning instead of
>>> a build break?
>>
>> It's better to have the build break than to have a runtime check. Of the two
>> other approaches you mentioned, I think a configure-time option is the better
>> idea. Since these hashes are aimed at developers and are are not needed for
>> ordinary builds from tarballs, I think the hashes should be checked on
>> request. I resurrected the hashes by installing the attached patch and they
>> are now checked with the longstanding 'configure' option --enable-checking=all
>> or with the new 'configure' option --enable-checking=structs. Comments welcome
>> as usual.
>
> This newly resurrected checking is already broken on master, which does
> not build with confgiure option "--enable-checking=yes,glyphs".
>
> The struct fingerprint checking should default to enabled (as before)
> with the configure arguments allowing it to be (temporarily) disabled.

Apologies - the problem is actually in commit 5c2f94a182 ("Replace
executable’s fingerprint in place"):

  CCLD     temacs.exe
C:\emacs\git\emacs\master\build\mingw64-x86_64\lib-src\make-fingerprint.exe: Error: temacs.exe.tmp is not a regular file
make[1]: *** [Makefile:644: temacs.exe] Error 1
make[1]: Leaving directory '/c/emacs/git/emacs/master/build/mingw64-x86_64/src'
make: *** [Makefile:423: src] Error 2

I expect this is something involving the libc handling for Windows
builds, as the fstat() call reports st.mode as 0 rather than continaing
the expected 0x8000 bit for a regular file.

    AndyM

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/15/19 8:32 AM, Andy Moreton wrote:
> I expect this is something involving the libc handling for Windows
> builds, as the fstat() call reports st.mode as 0 rather than continaing
> the expected 0x8000 bit for a regular file.

Yes, thanks for reporting it, Eli fixed it on the MS-Windows side before
I saw your message.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Stefan Monnier]

> What would make the hashes easier to deal with?

I can't think of anything other than make the hash computation smarter
so it ignores changes in comments, indentation, ...


        Stefan

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

Alan Mackenzie wrote:
> Having a build aborted by CHECK_STRUCTS, and having manually to change
> the hashes in pdumper.c is what takes the time.  But really, how often
> are we changing those structures of an evening?

I must do it more often than anyone else, which is why I'm the squeakiest wheel 
here. I've done it four times on master (compared to one time for everyone else 
combined), and I've done it several times more on unpublished private branches. 
For me this new bureaucracy is an aggravation, and it's never helped me find a bug.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

> Hello, Paul.
>
> On Tue, Apr 09, 2019 at 15:47:17 -0700, Paul Eggert wrote:
>> On 3/4/19 6:17 PM, Paul Eggert wrote:
>> > On 2/28/19 12:59 PM, Alan Mackenzie wrote:
>> >> is all this really needed?  Is pdumper.c really that fragile,
>> >> that it can't cope with changes in certain structs?
>> > No, it's not needed, and in my experience the mechanism's costs far
>> > exceed any benefit.
>
>> No further comment and the mechanism just bit me again, so I installed
>> the attached patch to disable it.
>
> Why?  Completely removing this mechanism seems very heavy handed.  All
> that was needed to disable it was a single line change to set
> CHECK_STRUCTS to zero.
>
> You would have got further comment if you'd proposed a patch, rather
> than just a vague idea that might or might not have been followed
> through.  Discussing the merits of a patch is always best done before
> committing it to master, not afterwards.
>
> What does Daniel say?

Please revert this removal. Without CHECK_STRUCTS, we *will* break dumped
Emacs in some subtle way sooner or later.

The only other safe option is to just codegen all the lisp.h structure
definitions from some higher-level description from which we can also
generate pdumper dumping code. If you want to do that, be my guest. Until
then, please restore CHECK_STRUCTS.

>> We can reenable it later if needed (which I hope won't happen....).
>
> Funnily enough, it was of use to me recently when it reminded me to
> amend dump_subr after extending struct subr.
>
> My main problem with this mechanism was the vagueness of the error
> messages it generated and of the comment in the code they pointed at,
> which wasted lots of time.  Those defects have since been fixed.

Thanks for improving that.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/10/19 11:47 AM, Daniel Colascione wrote:
> Without CHECK_STRUCTS, we *will* break dumped
> Emacs in some subtle way sooner or later.

We're gonna break dumped Emacs in subtle ways no matter what. After all,
pdumper.c was broken in subtle ways when it was first introduced,
despite CHECK_STRUCTS. That's just life, and it's not the issue here.
The issue is whether the benefits of the CHECK_STRUCTS mechanism is
worth the hassle. In my experience it's definitely not worth it.

The more gingerbread we put into the Emacs build procedure, the more
it's off-putting to other potential developers (and the more work it is
for us). We already have too much build overhead (starting with
Autoconf! I hate Autoconf! It should go away!) and we should not neglect
its costs when looking at the benefits of a new hassle we add to the
build procedure. So far the benefits of CHECK_STRUCT have been zero in
the master, and the costs have been slowdown and significant annoyance
to the developer who's made the most changes to this part of the code.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

> On 4/10/19 11:47 AM, Daniel Colascione wrote:
>> Without CHECK_STRUCTS, we *will* break dumped
>> Emacs in some subtle way sooner or later.
>
> We're gonna break dumped Emacs in subtle ways no matter what. After all,
> pdumper.c was broken in subtle ways when it was first introduced,
> despite CHECK_STRUCTS. That's just life, and it's not the issue here.
> The issue is whether the benefits of the CHECK_STRUCTS mechanism is
> worth the hassle. In my experience it's definitely not worth it.
>
> The more gingerbread we put into the Emacs build procedure, the more
> it's off-putting to other potential developers (and the more work it is
> for us). We already have too much build overhead (starting with
> Autoconf! I hate Autoconf! It should go away!) and we should not neglect
> its costs when looking at the benefits of a new hassle we add to the
> build procedure. So far the benefits of CHECK_STRUCT have been zero in
> the master, and the costs have been slowdown and significant annoyance
> to the developer who's made the most changes to this part of the code.

CHECK_STRUCTS was very useful when pdumper was out of tree and I had to
rebase it regularly. You can disable the thing locally for iterative
development.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 10 Apr 2019 11:58:20 -0700
> Cc: emacs-devel@gnu.org
> 
> So far the benefits of CHECK_STRUCT have been zero in the master

Not zero, at least not for me: it caught my mistake at least once.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Robert Pluim]

>>>>> On Wed, 10 Apr 2019 11:58:20 -0700, Paul Eggert <eggert@cs.ucla.edu> said:
    Paul> The more gingerbread we put into the Emacs build procedure,
    Paul> the more it's off-putting to other potential developers (and
    Paul> the more work it is for us). We already have too much build
    Paul> overhead (starting with Autoconf! I hate Autoconf! It should
    Paul> go away!)

Much as I dislike Autoconf, what would you have us replace it with?

Robert

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/11/19 2:35 AM, Robert Pluim wrote:
> Much as I dislike Autoconf, what would you have us replace it with?

I was thinking of using just standard tools (as per the GNU Coding
Standards) along with GNU Make - and, once the Emacs core is built, we
can use Emacs itself. Although we started assuming GNU Make in Emacs 25,
we haven't been using GNU Make's features fully and some of its features
could effectively replace the need for Autoconf.

A benefit of this approach would be faster builds. Right now the biggest
bottleneck on my system is the time to run 'configure' whenever I make a
trivial change to configure.ac or whatever. I *hate* that.

Although I looked into other possible approaches (switching to SCons,
say) none of them seemed to offer compelling advantages to the
more-conservative approach I have in mind.

Eric Raymond reported success with this sort of approach when
deautoconfiscating giflib and the NTP code base:

https://lists.gnu.org/r/bison-patches/2019-02/msg00041.html

It'd take some work to do this for Emacs - among other things, Gnulib
would need to support building without Autoconf, at least for the Gnulib
modules that Emacs uses. But I think it'd be doable.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 11 Apr 2019 11:31:36 -0700
> 
> On 4/11/19 2:35 AM, Robert Pluim wrote:
> > Much as I dislike Autoconf, what would you have us replace it with?
> 
> I was thinking of using just standard tools (as per the GNU Coding
> Standards) along with GNU Make - and, once the Emacs core is built, we
> can use Emacs itself. Although we started assuming GNU Make in Emacs 25,
> we haven't been using GNU Make's features fully and some of its features
> could effectively replace the need for Autoconf.

You want to test for system-dependent features with Make?  How does
one do that?  All those HAVE_foo macros in the sources -- how do you
compute them with Make?  Wouldn't we end up with heaps of Make
wizardry only a few understand?

> A benefit of this approach would be faster builds. Right now the biggest
> bottleneck on my system is the time to run 'configure' whenever I make a
> trivial change to configure.ac or whatever. I *hate* that.

IME, the slowest part is byte compilation of Lisp files.  Of course,
that is only a significant factor when you bootstrap or rebuild large
parts of Emacs, but still.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Date: Thu, 11 Apr 2019 11:31:36 -0700
>>
>> On 4/11/19 2:35 AM, Robert Pluim wrote:
>> > Much as I dislike Autoconf, what would you have us replace it with?
>>
>> I was thinking of using just standard tools (as per the GNU Coding
>> Standards) along with GNU Make - and, once the Emacs core is built, we
>> can use Emacs itself. Although we started assuming GNU Make in Emacs 25,
>> we haven't been using GNU Make's features fully and some of its features
>> could effectively replace the need for Autoconf.
>
> You want to test for system-dependent features with Make?  How does
> one do that?  All those HAVE_foo macros in the sources -- how do you
> compute them with Make?  Wouldn't we end up with heaps of Make
> wizardry only a few understand?

GNU make can run arbitrary shell commands. You'd just write

  HAVE_foo := $(shell ./test-foo)

or something like that, then have a separate make function that compiled
all these tests into a header. I don't think it's a *good* idea, but it
works, and you can do surprisingly complex things this way.

>> A benefit of this approach would be faster builds. Right now the biggest
>> bottleneck on my system is the time to run 'configure' whenever I make a
>> trivial change to configure.ac or whatever. I *hate* that.
>
> IME, the slowest part is byte compilation of Lisp files.  Of course,
> that is only a significant factor when you bootstrap or rebuild large
> parts of Emacs, but still.

Someday, it'd be nice to get rid of explicit byte compilation entirely and
rely on either a JIT or automatic recompilation on load.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> Date: Thu, 11 Apr 2019 15:13:10 -0700
> From: "Daniel Colascione" <dancol@dancol.org>
> Cc: "Paul Eggert" <eggert@cs.ucla.edu>,
>  emacs-devel@gnu.org
> 
> > You want to test for system-dependent features with Make?  How does
> > one do that?  All those HAVE_foo macros in the sources -- how do you
> > compute them with Make?  Wouldn't we end up with heaps of Make
> > wizardry only a few understand?
> 
> GNU make can run arbitrary shell commands. You'd just write
> 
>   HAVE_foo := $(shell ./test-foo)

You can also just use the shell.  Wait, didn't Autoconf start like
that?

More seriously, there are build systems out there which don't use
Autoconf.  My limited experience with some of them is that they all
bump into the same problems of complexity when catering to large
packages that need to support many different platforms.  So it all
boils down to a different syntax (and in many cases inadequate
documentation).  A faster execution doesn't justify the massive effort
required for mastering any such new system, IME.

> Someday, it'd be nice to get rid of explicit byte compilation entirely and
> rely on either a JIT or automatic recompilation on load.

JIT byte compilation is just one part of slow part of the build.  But
even with this, the last attempt to bring JIT to Emacs stalled at a
point where it isn't yet usable except as POC, and I didn't actually
see any tangible speedup from using it when it did work.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/11/19 12:15 PM, Eli Zaretskii wrote:
> You want to test for system-dependent features with Make?  How does
> one do that?

You execute the test as a 'make' rule that records the test's result as
a makefile snippet (or as a .h file, or whatever).

> Wouldn't we end up with heaps of Make wizardry only a few understand?
Sure, just as we now have heaps of .m4 and shell etc. wizardry that only
a few understand. (Think of it as the law of conservation of wizardry.
:-) However, an advantage of using Make instead of M4 is that we already
need to use Make anyway, so avoiding m4 means one less thing for
developers to learn. Another advantage is that Make can easily run in
parallel whereas 'configure' does not.

> IME, the slowest part is byte compilation of Lisp files.  Of course,
> that is only a significant factor when you bootstrap or rebuild large
> parts of Emacs, but still.
Yes. Typically I don't recompile Lisp files so the bottleneck for me is
'configure'.

Moreover, Lisp file recompilation is parallelized so it's reasonably
fast on larger platforms (the wave of the future :-), whereas
'configure' will remain slow. I just now timed it, and './configure'
took 29 real-time seconds whereas 'cd lisp; make -j16 compile-always'
took 38 real-time seconds; this was with two circa-2013 Xeon E5-2640 v2
CPUs with 8 cores each. So on a 32-core platform, I expect ./configure
to take longer than byte-compiling all the Lisp files - i.e.,
'configure' is ridiculously slow.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

> On 4/11/19 12:15 PM, Eli Zaretskii wrote:
>> You want to test for system-dependent features with Make?  How does
>> one do that?
>
> You execute the test as a 'make' rule that records the test's result as
> a makefile snippet (or as a .h file, or whatever).
>
>> Wouldn't we end up with heaps of Make wizardry only a few understand?
> Sure, just as we now have heaps of .m4 and shell etc. wizardry that only
> a few understand. (Think of it as the law of conservation of wizardry.
> :-) However, an advantage of using Make instead of M4 is that we already
> need to use Make anyway, so avoiding m4 means one less thing for
> developers to learn. Another advantage is that Make can easily run in
> parallel whereas 'configure' does not.
>
>> IME, the slowest part is byte compilation of Lisp files.  Of course,
>> that is only a significant factor when you bootstrap or rebuild large
>> parts of Emacs, but still.
> Yes. Typically I don't recompile Lisp files so the bottleneck for me is
> 'configure'.
>
> Moreover, Lisp file recompilation is parallelized so it's reasonably
> fast on larger platforms (the wave of the future :-), whereas
> 'configure' will remain slow. I just now timed it, and './configure'
> took 29 real-time seconds whereas 'cd lisp; make -j16 compile-always'
> took 38 real-time seconds; this was with two circa-2013 Xeon E5-2640 v2
> CPUs with 8 cores each. So on a 32-core platform, I expect ./configure
> to take longer than byte-compiling all the Lisp files - i.e.,
> 'configure' is ridiculously slow.

caching is supposed to help with that, isn't it?

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/11/19 3:26 PM, Daniel Colascione wrote:
> caching is supposed to help with that, isn't it?

It is supposed to, yes, but it doesn't help for my builds. I'm often
trying different 'configure' options (because I'm trying different
compiler or linker flags, or whatever), or I'm modifying configure.ac or
m4/* files, so the cache from my previous 'configure' run is wrong for
the current run. I've had so many problems with stale 'configure' caches
that I have learned to avoid these caches entirely.

'configure' caching is intended, I think, more for people who build lots
of programs and want to save time configuring them all. It doesn't work
as well for someone like me who too often modifies the configuration
machinery for a single program.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 11 Apr 2019 15:23:15 -0700
> 
> On 4/11/19 12:15 PM, Eli Zaretskii wrote:
> > You want to test for system-dependent features with Make?  How does
> > one do that?
> 
> You execute the test as a 'make' rule that records the test's result as
> a makefile snippet (or as a .h file, or whatever).

Sure, but that means we'll need to rewrite all the Autoconf tests as
Make scripts, complete with the test programs and other stuff.  Why is
such an effort worth our while?

> > Wouldn't we end up with heaps of Make wizardry only a few understand?
> Sure, just as we now have heaps of .m4 and shell etc. wizardry that only
> a few understand.

There's enough high-level, well documented Autoconf infrastructure
built on top of m4 that even I can write tests quite easily.  If we
move to Make, we will have to reinvent that wheel entirely from
scratch.

> However, an advantage of using Make instead of M4 is that we already
> need to use Make anyway

I'm sure you know that we currently use about 10% of what GNU Make can
do.  Moreover, using sophisticated Make facilities would mean we have
to require relatively recent versions of Make, which will be a
complication for some platforms.

> Another advantage is that Make can easily run in parallel whereas
> 'configure' does not.

The disadvantages are so many that this advantage drowns in them.

> 'cd lisp; make -j16 compile-always' took 38 real-time seconds

That's not the full build, you miss regenerating many files that
happens in a fresh checkout.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/11/19 11:39 PM, Eli Zaretskii wrote:
> Sure, but that means we'll need to rewrite all the Autoconf tests as
> Make scripts, complete with the test programs and other stuff.  Why is
> such an effort worth our while?

Because I'm so annoyed at how long 'configure' takes that I'm willing to
spend time to make it go faster.


> using sophisticated Make facilities would mean we have
> to require relatively recent versions of Make, which will be a
> complication for some platforms.

We could start with features supported by GNU Make 3.81 (2006) and
later, which is the version that Emacs already requires. If features of
later 'make' are useful we can cross that bridge when we come to it. My
impression is that Gnu Make 3.82 (2010) or later is pretty universal
nowadays, though requiring versions after that would be iffier.


>
>> 'cd lisp; make -j16 compile-always' took 38 real-time seconds
> That's not the full build, you miss regenerating many files that
> happens in a fresh checkout.

Sure, but you mentioned that in your experience the slowest part is byte
compilation of Lisp files, so I was attempting to measure the time of
doing that, versus the time of running 'configure'. For the platform I
mentioned, running 'configure' takes nearly as long as byte-compiling to
all the installed .elc files, and in the not-too-distant future as I get
better hardware I expect 'configure' will take even longer than the
byte-compiling. It's pretty bad.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 12 Apr 2019 12:40:41 -0700
> Cc: emacs-devel@gnu.org
> 
> On 4/11/19 11:39 PM, Eli Zaretskii wrote:
> > Sure, but that means we'll need to rewrite all the Autoconf tests as
> > Make scripts, complete with the test programs and other stuff.  Why is
> > such an effort worth our while?
> 
> Because I'm so annoyed at how long 'configure' takes that I'm willing to
> spend time to make it go faster.

That's okay, but it's not only your time that's at stake.  Once the
tests using Make are written, the rest of the Emacs developers will
have to study them, maybe document them, and learn to use and modify
them, so that this system could be kept for the observable future.

In general, having an Emacs-specific build system used only by Emacs
sounds wrong to me, as we don't have enough resources for that.  If we
want to switch to another build system, it would be more logical to
look at the popular alternatives used out there, rather than inventing
our own.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

Eli Zaretskii wrote:
> In general, having an Emacs-specific build system used only by Emacs
> sounds wrong to me

No, the idea is to migrate to the kind of build system that other software is or 
will be migrating to. We shouldn't discard the many useful shell snippets in the 
current Emacs configury; we should instead hook them together in a better way.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Andy Moreton]

On Thu 11 Apr 2019, Paul Eggert wrote:

> On 4/11/19 12:15 PM, Eli Zaretskii wrote:
>> You want to test for system-dependent features with Make?  How does
>> one do that?
>
> You execute the test as a 'make' rule that records the test's result as
> a makefile snippet (or as a .h file, or whatever).

Which gives you a buggy reimplementation of half of autoconf.

>> IME, the slowest part is byte compilation of Lisp files.  Of course,
>> that is only a significant factor when you bootstrap or rebuild large
>> parts of Emacs, but still.
> Yes. Typically I don't recompile Lisp files so the bottleneck for me is
> 'configure'.
>
> Moreover, Lisp file recompilation is parallelized so it's reasonably
> fast on larger platforms (the wave of the future :-), whereas
> 'configure' will remain slow. I just now timed it, and './configure'
> took 29 real-time seconds whereas 'cd lisp; make -j16 compile-always'
> took 38 real-time seconds; this was with two circa-2013 Xeon E5-2640 v2
> CPUs with 8 cores each. So on a 32-core platform, I expect ./configure
> to take longer than byte-compiling all the Lisp files - i.e.,
> 'configure' is ridiculously slow.

You could help speed up configure by fixing the ridiculously long set of
checks for supported compiler flags that come from gnulib. That is
glacially slow and mostly pointless.

    AndyM

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Paul.

On Thu, Apr 11, 2019 at 15:23:15 -0700, Paul Eggert wrote:

[ .... ]

> Moreover, Lisp file recompilation is parallelized so it's reasonably
> fast on larger platforms (the wave of the future :-), whereas
> 'configure' will remain slow.

Why?  Because we're not fixing it.

There's no reason why `configure' needs to run on a single CPU core, at
least, not that I can see at the moment.

The hero will be the hacker who contributes a -j parameter to configure
in the autoconf project.  Gentoo users would be grateful for ever.

As a stopgap, would it perhaps be possible to feed `configure''s tests
through GNU Parallel, somehow?  I see `configure' being fed through a
sed or AWK script to make it do this.  Perhaps.

> I just now timed it, and './configure' took 29 real-time seconds
> whereas 'cd lisp; make -j16 compile-always' took 38 real-time seconds;
> this was with two circa-2013 Xeon E5-2640 v2 CPUs with 8 cores each.
> So on a 32-core platform, I expect ./configure to take longer than
> byte-compiling all the Lisp files - i.e., 'configure' is ridiculously
> slow.

For the record, the slowness of `configure' irritates me, too.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> Date: Fri, 12 Apr 2019 13:37:27 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> The hero will be the hacker who contributes a -j parameter to configure
> in the autoconf project.  Gentoo users would be grateful for ever.
> 
> As a stopgap, would it perhaps be possible to feed `configure''s tests
> through GNU Parallel, somehow?  I see `configure' being fed through a
> sed or AWK script to make it do this.  Perhaps.

Why cannot we break configure.ac into several orthogonal parts, and
then run them in parallel?

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Noam Postavsky]

On Fri, 12 Apr 2019 at 09:37, Alan Mackenzie <acm@muc.de> wrote:

> > I just now timed it, and './configure' took 29 real-time seconds
> > whereas 'cd lisp; make -j16 compile-always' took 38 real-time seconds;
> > this was with two circa-2013 Xeon E5-2640 v2 CPUs with 8 cores each.
> > So on a 32-core platform, I expect ./configure to take longer than
> > byte-compiling all the Lisp files - i.e., 'configure' is ridiculously
> > slow.
>
> For the record, the slowness of `configure' irritates me, too.

For the record, I use the --cache-file option, and `configure' runs in
2 real-time seconds. I use separate cache files per configuration, it
works pretty well.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Noam.

On Fri, Apr 12, 2019 at 09:58:17 -0400, Noam Postavsky wrote:
> On Fri, 12 Apr 2019 at 09:37, Alan Mackenzie <acm@muc.de> wrote:

> > > I just now timed it, and './configure' took 29 real-time seconds
> > > whereas 'cd lisp; make -j16 compile-always' took 38 real-time
> > > seconds; this was with two circa-2013 Xeon E5-2640 v2 CPUs with 8
> > > cores each.  So on a 32-core platform, I expect ./configure to
> > > take longer than byte-compiling all the Lisp files - i.e.,
> > > 'configure' is ridiculously slow.

> > For the record, the slowness of `configure' irritates me, too.

> For the record, I use the --cache-file option, and `configure' runs in
> 2 real-time seconds. I use separate cache files per configuration, it
> works pretty well.

I tried that out yesterday, and indeed it takes about 2 seconds.  :-)

But it's not clear how ./configure -C works.  What gets cached, and how
does configure know when something needs determining again rather than
just copying a result from the cache?  Presumably one needs to be
prepared to delete the cache file when moving the top level directory to
a different environment, and things like that.

Is ./configure -C documented at all?

-- 
Alan Mackenzie (Nuremberg, Germany).

About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Noam Postavsky]

On Sat, 13 Apr 2019 at 10:06, Alan Mackenzie <acm@muc.de> wrote:

> > > > I just now timed it, and './configure' took 29 real-time seconds

> > > For the record, the slowness of `configure' irritates me, too.
>
> > For the record, I use the --cache-file option, and `configure' runs in
> > 2 real-time seconds. I use separate cache files per configuration, it
> > works pretty well.
>
> I tried that out yesterday, and indeed it takes about 2 seconds.  :-)
>
> But it's not clear how ./configure -C works.  What gets cached,

The macros used in configure.ac control what is cached.

> and how
> does configure know when something needs determining again rather than
> just copying a result from the cache?

The cache is not invalidated automatically, so you need to delete it
after upgrading devel libraries or compiler.

>  Presumably one needs to be
> prepared to delete the cache file when moving the top level directory to
> a different environment, and things like that.

Definitely can't take the cache file to a different machine.

> Is ./configure -C documented at all?

Not from the perspective of a user running ./configure as far as I
know, but see (autoconf.info) Caching Results.
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Caching-Results.html

Re: About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Paul Eggert]

Noam Postavsky wrote:
> The cache is not invalidated automatically, so you need to delete it
> after upgrading devel libraries or compiler.

... or if you configure with different flags, or if you edit configure.ac or if 
you do a bunch of other things. For the kind of work I do, the cache is way more 
trouble than it's worth.

Re: About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Daniel Colascione]

On 4/13/19 7:44 PM, Paul Eggert wrote:
> Noam Postavsky wrote:
>> The cache is not invalidated automatically, so you need to delete it
>> after upgrading devel libraries or compiler.
> 
> ... or if you configure with different flags, or if you edit 
> configure.ac or if you do a bunch of other things. For the kind of work 
> I do, the cache is way more trouble than it's worth.

The caching could probably be a lot better. We could incorporate a hash 
of the passed-in variables and of the environment into the cache file 
now. As for changing configure.ac itself: that's very rare. I also 
wonder CC="ccache gcc" might help even for the case of a 
frequently-changed configure.ac.

Re: About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Noam Postavsky]

On Sat, 13 Apr 2019 at 23:26, Daniel Colascione <dancol@dancol.org> wrote:
>
> On 4/13/19 7:44 PM, Paul Eggert wrote:
> > Noam Postavsky wrote:
> >> The cache is not invalidated automatically, so you need to delete it
> >> after upgrading devel libraries or compiler.
> >
> > ... or if you configure with different flags, or if you edit
> > configure.ac or if you do a bunch of other things. For the kind of work
> > I do, the cache is way more trouble than it's worth.

In practice, I've found the only problematic flag was
--with-x-toolkit, building with gtk caused a couple of tests to act
differently. Other than that, I even share cache files across Emacs
versions, the differences between configure.ac don't seem to be a
problem. Of course I expect it's different if you're editing
configure.ac and it's in an inconsistent state.

> I also
> wonder CC="ccache gcc" might help even for the case of a
> frequently-changed configure.ac.

It wouldn't help for the ./configure step itself, ccache doesn't cache
output of linking. Running ./configure does trigger a rebuild of all
of Emacs' C files, so ccache definitely helps for that.

https://ccache.dev/manual/latest.html:
autoconf compile/link Uncachable compilation or linking by an autoconf test.

Re: About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Alan Mackenzie]

Hello, Paul.

On Sat, Apr 13, 2019 at 19:44:50 -0700, Paul Eggert wrote:
> Noam Postavsky wrote:
> > The cache is not invalidated automatically, so you need to delete it
> > after upgrading devel libraries or compiler.

> ... or if you configure with different flags, or if you edit
> configure.ac or if you do a bunch of other things. For the kind of
> work I do, the cache is way more trouble than it's worth.

Are you sure?  I just tried it out.  My first run,
    ./configure -C --with-gif=no --with-tiff=no --with-gpm=no
, took ~2.3 seconds.  I built Emacs, and GPM was indeed not built in.

Then I did
    ./configure -C --with-gif=no --with-tiff=no --with-gpm
, which again took ~2.3s, built Emacs, and GPM was then present and
working.

So it seems that ./configure -C can cope with at least some flags being
changed between runs.  Which are these "different flags" which don't
work when you change them whilst using the -C cache?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Eli Zaretskii]

> Date: Sun, 14 Apr 2019 09:45:35 +0000
> Cc: Noam Postavsky <npostavs@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>   Emacs developers <emacs-devel@gnu.org>
> From: Alan Mackenzie <acm@muc.de>
> 
> > ... or if you configure with different flags, or if you edit
> > configure.ac or if you do a bunch of other things. For the kind of
> > work I do, the cache is way more trouble than it's worth.
> 
> Are you sure?  I just tried it out.  My first run,
>     ./configure -C --with-gif=no --with-tiff=no --with-gpm=no
> , took ~2.3 seconds.  I built Emacs, and GPM was indeed not built in.
> 
> Then I did
>     ./configure -C --with-gif=no --with-tiff=no --with-gpm
> , which again took ~2.3s, built Emacs, and GPM was then present and
> working.

I think you should try building with --with-gpm=no, and after than
without this switch, i.e. without an explicit --with-gpm.

> So it seems that ./configure -C can cope with at least some flags being
> changed between runs.

I think you are just being lucky: configure always recomputes
variables that are not cached yet.

Re: About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Noam Postavsky]

On Sun, 14 Apr 2019 at 10:08, Eli Zaretskii <eliz@gnu.org> wrote:

> I think you should try building with --with-gpm=no, and after than
> without this switch, i.e. without an explicit --with-gpm.

Works fine, I get

  Does Emacs use -lgpm?                                  no

the first time, and

  Does Emacs use -lgpm?                                   yes

the second time, as expected. Why wouldn't you expect that to work?

> > So it seems that ./configure -C can cope with at least some flags being
> > changed between runs.
>
> I think you are just being lucky: configure always recomputes
> variables that are not cached yet.

If you pass --with-gpm=no, it skips the test so there's nothing to
cache anyway. As I mentioned earlier, so far I've found configuring
with gtk the only problematic flag, because it affects some other test
(the test for glib, if I recall correctly).

Re: About ./configure --cache-file (WAS: CHECK_STRUCTS/dmpstruct.h mechanism is broken.)

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Sun, 14 Apr 2019 10:44:03 -0400
> Cc: Alan Mackenzie <acm@muc.de>, Paul Eggert <eggert@cs.ucla.edu>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> Why wouldn't you expect that to work?

I assumed (without checking) that --with-gpm=no caches some variable.

> > I think you are just being lucky: configure always recomputes
> > variables that are not cached yet.
> 
> If you pass --with-gpm=no, it skips the test so there's nothing to
> cache anyway.

A.k.a. "lucky".

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Achim Gratz]

Paul Eggert writes:
> On 4/11/19 12:15 PM, Eli Zaretskii wrote:
>> You want to test for system-dependent features with Make?  How does
>> one do that?
>
> You execute the test as a 'make' rule that records the test's result as
> a makefile snippet (or as a .h file, or whatever).

You'll end up re-implementing years and years of autoconf experience and
probably badly.  We can assume GNU make as long as we keep talking about
make so that will not be quite as painful as it would be otherwise, but
it'd still be an effort I don't quite see as justified.  Lately I've
seen an uptick in "let's build the fastest build system" ideas and all
of them seem to have forgotten that the real work for complex projects
is in configuration.

>> Wouldn't we end up with heaps of Make wizardry only a few understand?
> Sure, just as we now have heaps of .m4 and shell etc. wizardry that only
> a few understand.

M4 syntax is ugly, but it has few enough rules that you can learn and
understand it in an afternoon.  OTOH, quoting and shell-quoting in
Makefiles, not so.

> (Think of it as the law of conservation of wizardry.
> :-) However, an advantage of using Make instead of M4 is that we already
> need to use Make anyway, so avoiding m4 means one less thing for
> developers to learn. Another advantage is that Make can easily run in
> parallel whereas 'configure' does not.

Well, you keep assuming make, but when throwing out so much, why not
throw out the rest as well?  There are many attempts at getting rid of
make out there as well and some of them actually work quite nicely.

> Moreover, Lisp file recompilation is parallelized so it's reasonably
> fast on larger platforms (the wave of the future :-), whereas
> 'configure' will remain slow.

It doesn't have to be.  For starters, checking for all the header files
and libraries could be parallelized easily enough and most of the
configury doesn't actually need to be re-run when any of the configure
options change.  I don't know how active the autoconf project is these
days beyond fixing bugs, but some of it could very well be addressed
with them?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

DIY Stuff:
http://Synth.Stromeko.net/DIY.html

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

Achim Gratz wrote:
> You'll end up re-implementing years and years of autoconf experience and
> probably badly.

The "years and years" of experience you're taking about is experience that is 
under my belt. I have been a reasonably major contributor to Autoconf and to all 
the Emacs configury code and I know how it works. I would not reimplement it badly.

The idea I have is basically the idea that you proposed in another email: do the 
obvious parallelizations with the current recipes. However, doing this under the 
aegis of Autoconf would be a mistake. Autoconf is essentially unmaintained now, 
and for good reason: it's a dead-end and nobody wants to deal with it.  We 
should be looking at migrating its good stuff (mostly shell+m4 recipes) to a 
better framework.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

On 4/13/19 7:52 PM, Paul Eggert wrote:
> Achim Gratz wrote:
>> You'll end up re-implementing years and years of autoconf experience and
>> probably badly.
> 
> The "years and years" of experience you're taking about is experience 
> that is under my belt. I have been a reasonably major contributor to 
> Autoconf and to all the Emacs configury code and I know how it works. I 
> would not reimplement it badly.
> 
> The idea I have is basically the idea that you proposed in another 
> email: do the obvious parallelizations with the current recipes. 
> However, doing this under the aegis of Autoconf would be a mistake. 
> Autoconf is essentially unmaintained now, and for good reason: it's a 
> dead-end and nobody wants to deal with it.  We should be looking at 
> migrating its good stuff (mostly shell+m4 recipes) to a better framework.

I could get behind an "autoconf3" (since autoconf is already at 2.x) 
that emphasized continuity with existing autoconf projects while, at the 
same time, generated better configure programs amenable to 
parallelization and caching. These configure programs could use make 
pretty easily, I'd think.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Achim Gratz]

Paul Eggert writes:
> Achim Gratz wrote:
>> You'll end up re-implementing years and years of autoconf experience and
>> probably badly.
>
> The "years and years" of experience you're taking about is experience
> that is under my belt. I have been a reasonably major contributor to
> Autoconf and to all the Emacs configury code and I know how it
> works. I would not reimplement it badly.

I didn't mean to imply you would personally re-implement it.  I'm well
aware of your involvement with gnulib and autoconf.

By "badly" I meant that there will be a (likely extended) period of
transistion where things that used to work with autoconf don't work (at
all or breaking in more subtle ways) with whatever the new system is.
That'll grow resentment towards the new system as history with other
such attempts (re-implementing make, anyone?) has shown.

"The Perfect Should Not Have Grown"
  (F. Nietzschein "Human, all too Human", transl. by H. Zimmern)

"Das Vollkommene soll nicht geworden sein."
  (F. Nietzsche in "Menschliches, Allzumenschliches")

> The idea I have is basically the idea that you proposed in another
> email: do the obvious parallelizations with the current
> recipes. However, doing this under the aegis of Autoconf would be a
> mistake. Autoconf is essentially unmaintained now, and for good
> reason: it's a dead-end and nobody wants to deal with it.

Well, I feel towards autoconf like that old quip about democracy: it's
the worst form of configuring your project, except for all the others.
As for it being a dead-end: show me another project that even approaches
the same breadth and depth on _configuration_ and I'll reconsider.

> We should be looking at migrating its good stuff (mostly shell+m4
> recipes) to a better framework.

(Not sure what exactly you mean by framework here.  I'm assuming you
meant "something like autoconf", but different.)

The existing build systems or build system generators (as far as I spent
time looking at them) don't seem to fit the bill, so we'd either have to
find something more suitable (why didn't we hear about it already?) or
pony up for something entirely new.  I still think that incrementally
improving autoconf is more likely to successfully (and quickly) move
things forward than trying to outright re-implement it.

Another consideration for Emacs specifically would be how much extra
baggage we can afford to pack and still support all the systems that we
do now.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

Achim Gratz wrote:
> Another consideration for Emacs specifically would be how much extra
> baggage we can afford to pack and still support all the systems that we
> do now.

Sure, but that is a reason to prefer Emacs as a candidate for switching from 
Autoconf/Automake to GNU Make. Unlike most other major GNU packages, Emacs 
already assumes GNU Make, so the switch won't introduce a dependency. Also, 
Emacs (unlike most other GNU packages) already avoids Automake and this has 
already prompted Gnulib to support packages that don't use Automake. I'm merely 
thinking of another step in this process, where Gnulib adds support for packages 
that don't use Autoconf either. Since Gnulib currently provides 2/3 of the code 
that goes into 'configure', this would be a good step toward moving Emacs away 
from Autoconf.

Note that I'm not proposing to move away from the Autoconf Way of testing 
features rather than guessing them. I'm merely proposing a way to do that within 
the framework of GNU Make (which is parallelized) instead of m4+sh (which is not).

> I still think that incrementally
> improving autoconf is more likely to successfully (and quickly) move
> things forward than trying to outright re-implement it.

As a veteran of Autoconf I disagree, but the only way to resolve this is to see 
what happens. Parallelizing Autoconf is a much bigger project than transitioning 
GNU Emacs away from Autoconf, and nobody has seriously proposed attempting it. I 
think transitioning GNU Emacs is doable but to be honest the initial reactions 
here have not been encouraging and so my enthusiasm for the idea is lagging (for 
non-technical reasons, not for technical ones).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Alan Mackenzie]

Hello, Paul.

On Sun, Apr 14, 2019 at 16:29:50 -0700, Paul Eggert wrote:

[ .... ]

> Parallelizing Autoconf is a much bigger project than transitioning GNU
> Emacs away from Autoconf, and nobody has seriously proposed attempting
> it.

Any reason why not?  Is there anything particular about Autoconf that
makes it difficult to parallelise, or is it just a case of nobody
stepping up to do the work?

Given how prevalent Autoconf is, and the fact that processors with
several cores have been the normal thing for so long, and that these are
being superseded by processors with many cores, surely making configure
run its things in parallel would be a very productive use of time and
effort.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

Alan Mackenzie wrote:
> Is there anything particular about Autoconf that
> makes it difficult to parallelise, or is it just a case of nobody
> stepping up to do the work?

Autoconf does not keep track of dependencies, and attempting to add dependency 
tracking would require a significant internal redesign. A plausible approach 
would be to turn 'configure' into a front-end to a GNU makefile instead of being 
a sequential shell script. (Alternatively I suppose one could try to keep track 
of dependencies in the shell; good luck with that!) There would be significant 
changes to the Autoconf input language. Plus I expect there would be a new 
requirement for GNU Make, which would be a show-stopper for some projects. For 
compatibility we'd need to continue to support older sequential configure.ac 
files. I'm not saying it couldn't be done, only that it's a much bigger deal 
than simply moving Emacs away from Autoconf.

"Nobody stepping up to do the work" is also a factor, of course.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

I think that packages that needs GNU Make should have a simple
makefile for any old Make, which invokes GNU Make.  That is so that
(./configure; make install) will work provided GNU Make is available
under some conventional name, even if it isn't called 'make'.

Since Emacs already "assumes GNU Make", I wonder whether it does that.

I see that Emacs comes with a file GNUmakefile
and configuring creates Makefile.  What is the job
of each of these files, and how do they relate to each other?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/15/19 11:11 AM, Richard Stallman wrote:
> I think that packages that needs GNU Make should have a simple
> makefile for any old Make, which invokes GNU Make.  That is so that
> (./configure; make install) will work provided GNU Make is available
> under some conventional name, even if it isn't called 'make'.
>
> Since Emacs already "assumes GNU Make", I wonder whether it does that.

It does that in a different way. 'configure' searches for GNU Make and
if it's not called 'make', 'configure' tells the user GNU Make's name as
the very last thing it outputs in a message like "Now you can run
'gmake'." In practice this seems to have been good enough. I recall
trying to do something fancier, along the lines you suggested, but
having some portability issues and settling for the current approach.


> I see that Emacs comes with a file GNUmakefile
> and configuring creates Makefile.  What is the job
> of each of these files, and how do they relate to each other?

The following comment in GNUmakefile is intended to explain that. If
it's unclear please let me know.

# This GNUmakefile is for GNU Make.  It is for convenience, so that
# one can run 'make' in an unconfigured source tree.  In such a tree,
# this file causes GNU Make to first create a standard configuration
# with the default options, and then reinvokes itself on the
# newly-built Makefile.  If the source tree is already configured,
# this file defers to the existing Makefile.

# If you want non-default build options, or if you want to build in an
# out-of-source tree, you should run 'configure' before running 'make'.
# But run 'autogen.sh' first, if the source was checked out directly
# from the repository.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > It does that in a different way. 'configure' searches for GNU Make and
  > if it's not called 'make', 'configure' tells the user GNU Make's name as
  > the very last thing it outputs in a message like "Now you can run
  > 'gmake'." In practice this seems to have been good enough. I recall
  > trying to do something fancier, along the lines you suggested, but
  > having some portability issues and settling for the current approach.

I suppose many users will figure out what to do,
but scripts won't.

We want ./configure ARGS; make ARGS  to be a universal recipe
for building any package.  It's not good enough if
users have to hand-simulate it.  Especially if this is meant to be
an example that other packages will follow.

So would you please try again to make this work automatically?

If you encounter those obstacles again, let's talk about how to
surmount them.  Perhaps by adding new features to GNU make.

  > # This GNUmakefile is for GNU Make.  It is for convenience, so that
  > # one can run 'make' in an unconfigured source tree.  In such a tree,
  > # this file causes GNU Make to first create a standard configuration
  > # with the default options, and then reinvokes itself on the
  > # newly-built Makefile.  If the source tree is already configured,
  > # this file defers to the existing Makefile.

Why can't there be a fall-back makefile that does this in any old make?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

Richard Stallman wrote:

> We want ./configure ARGS; make ARGS  to be a universal recipe
> for building any package.  It's not good enough if
> users have to hand-simulate it.

These days typically the only 'hand-simulation' needed on non-GNU platforms is 
to put the right 'make' into one's PATH. I'm not sure I'd call that 
'hand-simulation' - it's more just preparing the proper build environment.

I suppose the problem could be worked around by having 'configure' check whether 
'make' supports GNU 'make' features, and if not installing a dummy Makefile that 
turns around and calls GNU 'make' (whatever its name is) with the real 
'makefile'. However, given that nobody has complained about the problem since 
September 2016 when Emacs 25 was released, adding a workaround for this issue 
surely cannot be high priority, as anybody who was likely to have run into the 
issue has almost surely figured things out already.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I suppose the problem could be worked around by having 'configure' check whether 
  > 'make' supports GNU 'make' features, and if not installing a dummy Makefile that 
  > turns around and calls GNU 'make' (whatever its name is) with the real 
  > 'makefile'. 

It's not a big job, so would you please do that?  This is an area
where it is especially important to dot the i's and cross the t's.

The code only has to be written once; then people will copy it into
other programs that use the same general approach with GNU Make.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

Richard Stallman wrote:
> The code only has to be written once; then people will copy it into
> other programs that use the same general approach with GNU Make.

After writing a patch along these lines, I somewhat doubt whether it will be 
worth the trouble. But if it causes trouble I suppose we can always go back to 
what Emacs 25 and 26 do. So I installed the patch into master; see attached.

[-- Attachment #2: 0001-Let-plain-make-work-even-not-GNU-Make.txt --]
[-- Type: text/plain, Size: 1438 bytes --]

From 6fa8d3c894062e7d3bde2d1ed35b40f2272e59f5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 22 Apr 2019 20:44:11 -0700
Subject: [PATCH] =?UTF-8?q?Let=20plain=20=E2=80=98make=E2=80=99=20work=20e?=
 =?UTF-8?q?ven=20not=20GNU=20Make?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Makefile.in (top_distclean): Clean makefile as well as Makefile.
* configure.ac: If not using plain ‘make’, create a makefile
so that plain ‘make’ simply calls $(MAKE).
---
 Makefile.in  |  2 +-
 configure.ac | 10 +++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 53703638c4..6b99d24da4 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -869,7 +869,7 @@ top_bootclean=
 top_distclean=\
 	${top_bootclean}; \
 	rm -f config.status config.log~ \
-	  Makefile lib/gnulib.mk ${SUBDIR_MAKEFILES}
+	  Makefile makefile lib/gnulib.mk ${SUBDIR_MAKEFILES}
 
 distclean_dirs = $(clean_dirs) leim lisp
 
diff --git a/configure.ac b/configure.ac
index 0ecb8c40e6..8b363c7fca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -5814,4 +5814,12 @@ m4_define
   esac
 fi
 
-test "$MAKE" = make || AC_MSG_NOTICE([Now you can run '$MAKE'.])
+# Let plain 'make' work.
+test "$MAKE" = make || test -f makefile || cat >makefile <<EOF
+.POSIX:
+MAKE = $MAKE
+all:
+	\$(MAKE) -f Makefile \$?
+.DEFAULT:
+	\$(MAKE) -f Makefile \$<
+EOF
-- 
2.17.1

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 22 Apr 2019 20:48:54 -0700
> Cc: Stromeko@nexgo.de, emacs-devel@gnu.org
> 
> -test "$MAKE" = make || AC_MSG_NOTICE([Now you can run '$MAKE'.])
> +# Let plain 'make' work.
> +test "$MAKE" = make || test -f makefile || cat >makefile <<EOF
> +.POSIX:
> +MAKE = $MAKE
> +all:
> +	\$(MAKE) -f Makefile \$?
> +.DEFAULT:
> +	\$(MAKE) -f Makefile \$<
> +EOF

Does this assume we could have both Makefile and makefile in the same
directory?  That's false on case-insensitive filesystems.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/22/19 11:25 PM, Eli Zaretskii wrote:
> Does this assume we could have both Makefile and makefile in the same
> directory?  That's false on case-insensitive filesystems.

The 'test -f makefile' prevents 'configure' from overwriting Makefile on
case-insensitive filesystems. So on these systems one needs a
GNU-compatible 'make'.

I didn't think it worth the trouble to work around the problem of using
a 'make' that isn't GNU-compatible on a filesystem that isn't
POSIX-compatible. If someone wants to take the extra trouble to address
that issue then more power to them. I am still of the opinion that this
whole thing is a mistake and that we should stick with what Emacs 25 and
26 do, as that's simpler and will avoid problems such as the one you
mention.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Eli Zaretskii]

> Cc: rms@gnu.org, Stromeko@nexgo.de, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 23 Apr 2019 09:28:06 -0700
> 
> On 4/22/19 11:25 PM, Eli Zaretskii wrote:
> > Does this assume we could have both Makefile and makefile in the same
> > directory?  That's false on case-insensitive filesystems.
> 
> The 'test -f makefile' prevents 'configure' from overwriting Makefile on
> case-insensitive filesystems. So on these systems one needs a
> GNU-compatible 'make'.

Ah, okay.  Then there's no problem.

> I am still of the opinion that this whole thing is a mistake and
> that we should stick with what Emacs 25 and 26 do, as that's simpler
> and will avoid problems such as the one you mention.

Then perhaps we shouldn't do this mistake.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Stefan Monnier]

> I am still of the opinion that this whole thing is a mistake and that
> we should stick with what Emacs 25 and 26 do, as that's simpler and
> will avoid problems such as the one you mention.

Agreed.  We do want Emacs to be buildable without too many contortions
even on rather exotic setups, but that's a far cry from requiring that
we should bear the cost of those exotic setups by striving to
transparently guess what's the right thing to do on those setups.


        Stefan

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I didn't think it worth the trouble to work around the problem of using
  > a 'make' that isn't GNU-compatible on a filesystem that isn't
  > POSIX-compatible.

I agree.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

If you have an idea for something better that Autoconf, and you can
see clean ways to handle the complex situations in Emacs, that would
be a very useful program.

Have you got a design in mind?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

Richard Stallman wrote:
> Have you got a design in mind?

No details written down, no. The basic idea is to use GNU Make to glue together 
Autoconf-style snippets, instead of Autoconf's sequentializing them into a shell 
script. This would 'configure' run in parallel.

Doing this properly will require telling GNU Make about dependencies among the 
snippets, but Gnulib is already keeping track of dependencies, and most of 
Emacs's configury is taken from Gnulib so that would give us a head-start in 
doing the conversion.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Achim Gratz]

Paul Eggert writes:
> On 4/11/19 2:35 AM, Robert Pluim wrote:
>> Much as I dislike Autoconf, what would you have us replace it with?
>
> I was thinking of using just standard tools (as per the GNU Coding
> Standards) along with GNU Make - and, once the Emacs core is built, we
> can use Emacs itself. Although we started assuming GNU Make in Emacs 25,
> we haven't been using GNU Make's features fully and some of its features
> could effectively replace the need for Autoconf.

I don't see how… autoconf got one thing right: it tries to actually
_compile_, not just check preprocessor defines or compiler versions.
That also makes it slow, especially as it's all serial.

> A benefit of this approach would be faster builds. Right now the biggest
> bottleneck on my system is the time to run 'configure' whenever I make a
> trivial change to configure.ac or whatever. I *hate* that.

The fun thing is that in this case you could run autoconf almost
entirely from already cached decisions.  I have not tried to feed
autoconf prepared caches in a long time, but it may be worth a try.

> Although I looked into other possible approaches (switching to SCons,
> say) none of them seemed to offer compelling advantages to the
> more-conservative approach I have in mind.

Most of the newfangled systems I've looked at focus on build speed
(mostly by pre-computing dependencies) and take the configuration for
granted (some even using autoconf again).

> Eric Raymond reported success with this sort of approach when
> deautoconfiscating giflib and the NTP code base:
>
> https://lists.gnu.org/r/bison-patches/2019-02/msg00041.html

I can tell you something about NTPsec.  Yes, it has no autoconf, but
that's been replaced with the configure step from waf, which has exactly
one person you can ask if something doesn't work the way you think it
should.  Based on Python it has a somewhat more agreeable syntax than M4
macros (no surprise), but it's not actually more clear (to me anyway).
Plus, there are a lot less depencies that need to be checked.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniel Colascione]

> Paul Eggert writes:
>> On 4/11/19 2:35 AM, Robert Pluim wrote:
>>> Much as I dislike Autoconf, what would you have us replace it with?
>>
>> I was thinking of using just standard tools (as per the GNU Coding
>> Standards) along with GNU Make - and, once the Emacs core is built, we
>> can use Emacs itself. Although we started assuming GNU Make in Emacs 25,
>> we haven't been using GNU Make's features fully and some of its features
>> could effectively replace the need for Autoconf.
>
> I don't see how… autoconf got one thing right: it tries to actually
> _compile_, not just check preprocessor defines or compiler versions.
> That also makes it slow, especially as it's all serial.
>
>> A benefit of this approach would be faster builds. Right now the biggest
>> bottleneck on my system is the time to run 'configure' whenever I make a
>> trivial change to configure.ac or whatever. I *hate* that.
>
> The fun thing is that in this case you could run autoconf almost
> entirely from already cached decisions.  I have not tried to feed
> autoconf prepared caches in a long time, but it may be worth a try.
>
>> Although I looked into other possible approaches (switching to SCons,
>> say) none of them seemed to offer compelling advantages to the
>> more-conservative approach I have in mind.
>
> Most of the newfangled systems I've looked at focus on build speed
> (mostly by pre-computing dependencies) and take the configuration for
> granted (some even using autoconf again).
>
>> Eric Raymond reported success with this sort of approach when
>> deautoconfiscating giflib and the NTP code base:
>>
>> https://lists.gnu.org/r/bison-patches/2019-02/msg00041.html
>
> I can tell you something about NTPsec.  Yes, it has no autoconf, but
> that's been replaced with the configure step from waf, which has exactly
> one person you can ask if something doesn't work the way you think it
> should.  Based on Python it has a somewhat more agreeable syntax than M4
> macros (no surprise), but it's not actually more clear (to me anyway).
> Plus, there are a lot less depencies that need to be checked.

I like autoconf. It's flexible and it works for every weird thing I've
ever wanted to do with it. Trying to color outside the lines with
alternate systems like cmake has been hell. While m4 isn't the most
elegant macro system out there, its quirks are well-understood, and
autoconf has decades of institutional knowledge built into it. I've also
found that alternative systems frequently have problems with complex
configurations like out-of-tree builds, cross compilation, various forms
of path overriding, and tool specification. autoconf dealt with all of
these issues a very long time ago.

IMHO, instead of investing in some new system, we'd be better off
improving autoconf speed by

1) eliminating tests that have become unnecessary in the modern world,
2) adding an optimization pass that batched redundant tests, and
3) making caching Just Work, maybe by stracing and checking all inputs for
cache validation.

The basic model is fine. The basic configuration model is fine.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/11/19 3:10 PM, Daniel Colascione wrote:
> I've also
> found that alternative systems frequently have problems with complex
> configurations like out-of-tree builds, cross compilation, various forms
> of path overriding, and tool specification. autoconf dealt with all of
> these issues a very long time ago.

Yes, we want to continue to support the things you mention (except Emacs
has never supported cross-compilation as far as I know).

> 1) eliminating tests that have become unnecessary in the modern world,

This would be nice, but will be tricky to do in Autoconf because many of
these tests are baked into Autoconf. I'm not saying it can't be done,
but at some point it may be better simply to dump Autoconf.

> 2) adding an optimization pass that batched redundant tests

Not sure what you mean by this.

> 3) making caching Just Work, maybe by stracing and checking all inputs for
> cache validation.
I have my doubts about this. For one thing, we can't assume strace.
Also, stracing is slow.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Paul Eggert]

On 4/11/19 12:55 PM, Achim Gratz wrote:
> Yes, it has no autoconf, but
> that's been replaced with the configure step from waf, which has exactly
> one person you can ask if something doesn't work the way you think it
> should.

Yes, I considered waf too (coincidentally I was just talking to a
student today about waf for one of his projects!). However, I don't
think waf would be as good a match for Emacs, as it brings in more
dependencies (Python, bzip2) and it's not yet clear that waf will be a
stable-enough build tool for Emacs in the long run.

In contrast, if we simply use GNU Make we are reducing dependencies (by
removing dependencies on autoconf/automake/m4) and we know GNU Make will
be there in the long haul.

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Daniele Nicolodi]

On 11-04-2019 16:44, Paul Eggert wrote:
> On 4/11/19 12:55 PM, Achim Gratz wrote:
>> Yes, it has no autoconf, but
>> that's been replaced with the configure step from waf, which has exactly
>> one person you can ask if something doesn't work the way you think it
>> should.
> 
> Yes, I considered waf too (coincidentally I was just talking to a
> student today about waf for one of his projects!).

I think meson http://mesonbuild.com/ is a better tool than waf. It is
also implemented in Python but it uses a DSL for the build definition
making his syntax much more regular. Meson is gaining a lot of traction
and the development is very active (and upstream is not hostile to
packaging in distributions). However, I haven't used it for anything
very complex yet.

Cheers,
Dan

Re: CHECK_STRUCTS/dmpstruct.h mechanism is broken.

[Achim Gratz]

Paul Eggert writes:
> On 4/11/19 12:55 PM, Achim Gratz wrote:
>> Yes, it has no autoconf, but
>> that's been replaced with the configure step from waf, which has exactly
>> one person you can ask if something doesn't work the way you think it
>> should.
>
> Yes, I considered waf too (coincidentally I was just talking to a
> student today about waf for one of his projects!). However, I don't
> think waf would be as good a match for Emacs, as it brings in more
> dependencies (Python, bzip2) and it's not yet clear that waf will be a
> stable-enough build tool for Emacs in the long run.

As a build tool it's OK I think, but the configury for Emacs would be
slow in waf, too, if it was implemented the same way as now.  That is,
unless you had a real expert for coding it up for waf in Python, which
would mean nobody else could understand what it does.

Meson would have the same problem (unless you sidestep and use autoconf
again) from what I understand of it.

> In contrast, if we simply use GNU Make we are reducing dependencies (by
> removing dependencies on autoconf/automake/m4) and we know GNU Make will
> be there in the long haul.

Again, I'd be more interested in autoconf picking up the obvious
opportunities for parallelization that are there for a start.  Then
Emacs would have to look at how the configuration process is organized
so it can be split into parts that are invariant and can be cached and
other parts that have to be re-run with option changes or always (for
whatever reason).  I'm pretty sure you'd be able to drop initial
configure time by a good factor on an SMP machine that way and
subsequent configure time to 10% of what it is now and that's be the end
of this discussion since any more speedup would not be noticeable
anymore.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds