Re: Linux 4.16-rc1: regression bisected, Debian kernel package tool make-kpkg stalls indefinitely during kernel build due to commit "kconfig: remove check_stdin()"

[Sander Eikelenboom]

On 13/02/18 14:07, Ulf Magnusson wrote:
> On Tue, Feb 13, 2018 at 1:35 PM, Ulf Magnusson <ulfalizer@xxxxxxxxx> wrote:
>> On Tue, Feb 13, 2018 at 12:33:24PM +0100, Ulf Magnusson wrote:
>>> On Tue, Feb 13, 2018 at 11:00:49AM +0100, Sander Eikelenboom wrote:
>>>> On 13/02/18 05:09, Masahiro Yamada wrote:
>>>>> 2018-02-13 12:00 GMT+09:00 Woody Suwalski <terraluna977@xxxxxxxxx>:
>>>>>> Sander Eikelenboom wrote:
>>>>>>>
>>>>>>> L.S.,
>>>>>>>
>>>>>>> The Debian kernel-package tool make-kpkg for easy building of upstream
>>>>>>> kernels on Debian fails with linux 4.16-rc1.
>>>>>>>
>>>>>>> The tool (perl script) while invoked with:
>>>>>>>      make-kpkg --initrd --append_to_version -20180212 kernel_image
>>>>>>>
>>>>>>> On a git tree with a .config from the previous kernel release, so new
>>>>>>> KConfig questions have to be asked on new or changed options.
>>>>>>>
>>>>>>> The script stalls indefinitely while it seems to be excuting:
>>>>>>>      exec make kpkg_version=13.018+nmu1 -f
>>>>>>> /usr/share/kernel-package/ruleset/minimal.mk debian
>>>>>>> APPEND_TO_VERSION=-t440s-20180212  INITRD=YES
>>>>>>>
>>>>>>> After using ctrl-c to break out it, i get:
>>>>>>>     ^CFailed to create a ./debian directory: No such file or directory at
>>>>>>> /usr/bin/make-kpkg line 970.
>>>>>>>
>>>>>>> Bisection turned up as culprit:
>>>>>>>      commit d2a04648a5dbc3d1d043b35257364f0197d4d868
>>>>>>>      kconfig: remove check_stdin()
>>>>>>>           Except silentoldconfig, valid_stdin is 1, so check_stdin() is
>>>>>>> no-op.
>>>>>>>           oldconfig and silentoldconfig work almost in the same way except
>>>>>>> that
>>>>>>>      the latter generates additional files under include/.  Both ask users
>>>>>>>      for input for new symbols.
>>>>>>>           I do not know why only silentoldconfig requires stdio be tty.
>>>>>>>             $ rm -f .config; touch .config
>>>>>>>        $ yes "" | make oldconfig > stdout
>>>>>>>        $ rm -f .config; touch .config
>>>>>>>        $ yes "" | make silentoldconfig > stdout
>>>>>>>        make[1]: *** [silentoldconfig] Error 1
>>>>>>>        make: *** [silentoldconfig] Error 2
>>>>>>>        $ tail -n 4 stdout
>>>>>>>        Console input/output is redirected. Run 'make oldconfig' to update
>>>>>>> configuration.
>>>>>>>             scripts/kconfig/Makefile:40: recipe for target
>>>>>>> 'silentoldconfig' failed
>>>>>>>        Makefile:507: recipe for target 'silentoldconfig' failed
>>>>>>>           Redirection is useful, for example, for testing where we want to
>>>>>>> give
>>>>>>>      particular key inputs from a test file, then check the result.
>>>>>>>           Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
>>>>>>>      Reviewed-by: Ulf Magnusson <ulfalizer@xxxxxxxxx>
>>>>>>>
>>>>>>> Reverting this specific commit makes make-kpkg work again as usual.
>>>>>>>
>>>>>>> Version of the kernel-package used:
>>>>>>> ii  kernel-package
>>>>>>> 13.018+nmu1
>>>>>>>
>>>>>>>
>>>>>>> I also cc'ed the Debian developer who maintains the kernel-package
>>>>>>> package: Manoj Srivastava
>>>>>>>
>>>>>>> --
>>>>>>> Sander
>>>>>>>
>>>>>> I have noticed today the same - the kernel-build blockage was in (as I
>>>>>> recall)
>>>>>> srcipts/kconfig/conf -s --silentoldconfig Kbuild
>>>>>>
>>>>>> I have bypassed it by regenerating the .config "by hand"...
>>>>>
>>>>>
>>>>> silentoldconfig asks you values for new symbols.
>>>>> So, you must answer questions to proceed.
>>>>
>>>> I know, but it stalls before asking the questions.
>>>>
>>>>>
>>>>> How does 'make-kpkg' handle silentoldconfig?
>>>>>
>>>>> Re-direct stdio, then make it forcibly fail?
>>>>
>>>> I don't know, it is a bunch of perl and shell scripts that gets invoked, not the most easy to comprehend if you are not familiar with them. I'm just a user of the tool.
>>>>
>>>> So i would have to defer that question to the Debian package maintainer, hopefully he will chime in.
>>>>
>>>> --
>>>> Sander
>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>> Hello,
>>>
>>> What the commit does is to make silentoldconfig not immediately exit(1)
>>> when both of the following apply:
>>>
>>>     1. stdin is from something that's not a terminal
>>>
>>>     2. New symbols are prompted for
>>>
>>> All the outputs (.config and the include/generated/ and include/config/
>>> trees) are generated after the point where silentoldconfig would
>>> previously exit(1), afaics, so the effects of the commit can be
>>> summarized as follows:
>>>
>>>     * If no new symbols appear (that would be prompted for), the
>>>       behavior is exactly the same as before.
>>>
>>>       (check_stdin() never seems to get called unless a value would
>>>       actually be prompted for, which makes sense.)
>>>
>>>     * If new symbols appear and stdin is a tty, the behavior is also
>>>       exactly the same as before.
>>>
>>> And finally, the only case where the behavior differs:
>>>
>>>     * If new symbols appear and stdin is not a tty, the
>>>       new behavior is to prompt (to expect e.g. "n"/"m"/"y" from
>>>       wherever stdin is coming from).
>>>
>>>       The old behavior was to exit(1) without generating any output
>>>       files.
>>>
>>> Hopefully that clarifies it.
>>>
>>> The intent of the commit was just to clean up the code a bit, since
>>> there doesn't seem to be a good reason to bail just because stdin
>>> happens to not be a tty (plus it's inconsistent with the way oldconfig
>>> works, which never bailed).
>>>
>>> silentoldconfig is something of an internal implementation detail at
>>> this point by the way. It's basically oldconfig + generate the
>>> configuration stuff in include/generated/ and include/config/ that
>>> actually gets used during the build. Normally it's called automatically
>>> as needed when building the kernel.
>>>
>>> See https://www.spinics.net/lists/kernel/msg2720574.html for some more
>>> details re. silentoldconfig.
>>>
>>> Cheers,
>>> Ulf
>>
>> By the way: If you can figure out a reason for why that stdin check was
>> there before while looking into the Debian tool, then please tell us.
>>
>> The problem with uncommented mystery code like that is that it easily
>> looks pointless and is at risk of getting changed or removed, even if
>> there was a good reason for it (which I still suspect there wasn't
>> though).
>>
>> </soapbox>
>>
>> Cheers,
>> Ulf
> 
> Shouldn't be a problem to back this one out either if it turns out to
> cause massive amounts of pain in practice I guess, even if it's the
> Debian tools doing something weird.
> 
> Good to look into what it is they're doing in any case.
> 
> Cheers,
> Ulf
> 

Hi Ulf / Masahiro,

Unfortunately the Debian maintainer hasn't responded to this thread, nor on
the Debian bugtracker (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890817) 
issue i made.

--
Sander









Linux-Regression-ID: lr#2fd778