Re: Emacs pretest 28.0.90 is out

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> How did you test the build?  What commands did you try?

Basically what you said to do below, minus the custom button dropdown
and C-z test.

>   . "M-x customize-variable", select some variable, and verify that
>     the drop-down menus in the Custom buffer work
>   . C-z, verify that you are dropped to a subordinate shell, invoke
>     some shell command, then type "exit RET" to verify that you are
>     back in Emacs

Both work fine.

> Thanks, see the comments below.

>> --- a/msdos/sed1v2.inp
>> +++ b/msdos/sed1v2.inp
>> @@ -55,6 +55,10 @@ s/ *@LIBJPEG@//
>>  s/ *@LIBPNG@//
>>  s/ *@LIBGIF@//
>>  s/ *@LIBXPM@//
>> +/^HAVE_NATIVE_COMP *=/s/@HAVE_NATIVE_COMP@//
>> +/^HAVE_PDUMPER *=/s/@HAVE_PDUMPER@//

> These two should be replaced with "no", not with empty values, I
> think.

Thanks.

>> +/^JSON_CFLAGS *=/s/@JSOB_CFLAGS@//
>> +/^JSON_LIBS *=/s/@JSOB_LIBS@//

> Typos: should be JSON_CFLAGS and JSON_LIBS.

>> +/^JSON_CFLAGS *=/s/@JSON_CFLAGS@//
>> +/^JSON_LIBS *=/s/@JSON_LIBS@//
>> +/^JSON_CFLAGS *=/s/@JSON_CFLAGS@//
>> +/^JSON_LIBS *=/s/@JSON_LIBS@//

> These should be redundant once you fix the typos above.

Great catch, thanks.

>> +OMIT_GNULIB_MODULE_crypto\/md5 = true

> Don't we need md5?  Why are you omitting it?

It's only needed for the pdumper build, and that module doesn't build on
MS-DOS.

>> +ifneq ($(MSDOS_OBJ),)
>> +temacs$(EXEEXT): $(LIBXMENU) $(ALLOBJS) $(LIBEGNU_ARCHIVE) $(EMACSRES) \
>> +  $(charsets) $(charscript) ${emoji-zwj} $(MAKE_PDUMPER_FINGERPRINT)
>> +    $(AM_V_CCLD)$(CC) -o $@ \
>> +      $(ALL_CFLAGS) $(TEMACS_LDFLAGS) $(LDFLAGS) \
>> +      $(ALLOBJS) $(LIBEGNU_ARCHIVE) $(W32_RES_LINK) $(LIBES)
>> +    $(MKDIR_P) $(etc)
>> +else
>>  ## We have to create $(etc) here because init_cmdargs tests its
>>  ## existence when setting Vinstallation_directory (FIXME?).
>>  ## This goes on to affect various things, and the emacs binary fails

> Couldn't you use the original recipe by a trivial editing (it seems
> all you need is to replace $@.tmp with $@?), instead of adding an
> ifdef?  Come to think of that, why not use the original recipe without
> any changes, as it moves $@.tmp to $@ after the link succeeds?

The linker apparently doesn't output a "stub" if the filename it's
assembling to doesn't end with ".exe", resulting in an invalid binary.

>> +int
>> +fchmodat (int fd, const char *path, mode_t mode, int flags)
>> +{
>> +  if (fd != AT_FDCWD)
>> +    {
>> +      if (strlen (dir_pathname) + strlen (path) + 1 >= MAXPATHLEN)
>> +    {
>> +      errno = ENAMETOOLONG;
>> +      return -1;
>> +    }
>> +    }
>> +
>> +  return 0;
>> +}

> This returns zero without doing anything, why?  I think it should call
> chmod when fd == AT_FDCWD.

Thanks.

>> +int
>> +futimens (int fd, const struct timespec times[2])
>> +{
>> +  /* TODO */
>> +  return 0;
>> +}
>> +
>> +int
>> +utimensat (int dirfd, const char *pathname,
>> +       const struct timespec times[2], int flags)
>> +{
>> +  /* TODO */
>> +  return 0;
>> +}
>
> What about these TODOs?

I forgot about those, good catch.