Subject: Re: bug#25360: File mode specification errors during building

From: [email-masked] (Phillip Lord)
Date: Thu, 19 Jan 2017 10:45:51 +0000

[email-masked] writes:

> [email-masked] (Phillip Lord) writes:
>
>> Glenn Morris <[email-masked]> writes:
>>
>>> [email-masked] wrote:
>>>
>>>> Using insert-file-contents instead of file-file-noselect seems to fix it
>>>> for me:
>>>
>>> That sounds like a good change anyway. (I forget, does this respect
>>> "coding:"? Does this even matter in this case?)
>>
>> I think, no, it doesn't respect coding, but that it doesn't matter.
>
> I believe it does respect "coding:" in general (at least it looks like
> there are some comments to that effect in Finsert_file_contents), but
> indeed it doesn't matter in this case because we're let-binding
> `coding-system-for-read' which overrides that.

I have checked this now with a full bootstrap and it appears not to
work, unfortunately -- the .el files that are generated are broken (this
isn't obvious until they fail to byte-compile -- you have let the build
complete).

So, find-file-noselect is doing something that insert-file-contents is not.

>>> (But it still seems like a potential problem to me if functions that used to
>>> be available early in the build no longer are.)
>>
>> It shouldn't be, if they are not being called.
>>
>> Anyway, the cause of the error in this case is this line in files.el.
>>
>> 	 (assoc-default nil magic-fallback-mode-alist
>> 		(lambda (re _dummy)
>> 		  (if (functionp re)
>> 		      (funcall re)
>> 		    (looking-at re)))))))
>>
>> This is called when loading cangjie-table.b5 with
>> 'image-type-auto-detected-p as the first parameter. Normally, the
>> condition returns "t", but in this case it will return f, then
>> the looking-at form fails with the error given. I don't understand which
>> this process does not happen during the build which generates
>> ldefs-boot-auto.el.
>
> Would it help to use a --without-x build to generate ldefs-boot-auto.el?

I'm a little loath to do this, because if a developer wants to run this,
it will reconfigure their build.

I've tried instead disabling all the "optional" statements in loadup.el
when building the bootstrap binary. bootstrap-emacs doesn't use images
or toolbars anyway. Will work up a patch tonight.

>
>> + (t (error "Problem with `magic-mode-alist'."))
>
> I'm not sure this improves the error message ("Problem" seems a bit
> vague).  Before:
>
>     Converting cangjie-table.b5 to tsang-b5.el...
>     File mode specification error: (wrong-type-argument stringp
> image-type-auto-detected-p)
>     Converting cangjie-table.b5 to tsang-b5.el...done
>
> After (applying only the lisp/files.el part of your patch):
>
>     Converting cangjie-table.b5 to tsang-b5.el...
>     File mode specification error: (error Problem with
> ¥magic-fallback-mode-alist¦.)
>     Converting cangjie-table.b5 to tsang-b5.el...done
>
> Perhaps add in the problematic element to the message?  Something along
> the lines of (error "Bad `magic-mode-alist' element: %S" re).

Yes, realised I can't use backquotes in error messages.

Your change is reasonable. In practice, I think, only developers are
likely to see this, and either of these messages is grepable, unlike the
wrong-type-argument message.

Phil