Re: [PATCH v2] Makefile: install modules.builtin even if CONFIG_MODULES=n

[Jonas Zeiger <jonas.zeiger@talpidae.net>]

On 2020-06-12 08:56, Masahiro Yamada wrote:
> On Wed, Jun 10, 2020 at 2:31 AM Guenter Roeck <groeck@google.com> 
> wrote:
>> 
>> On Tue, Jun 9, 2020 at 9:38 AM Doug Anderson <dianders@chromium.org> 
>> wrote:
>> >
>> > Hi,
>> >
>> > On Tue, Jun 3, 2020 at 9:33 AM Jonas Zeiger <jonas.zeiger@talpidae.net> wrote:
>> > >
>> > > Many applications check for available kernel features via:
>> > >
>> > >   - /proc/modules (loaded modules, present if CONFIG_MODULES=y)
>> > >   - $(MODLIB)/modules.builtin (builtin modules)
>> > >
>> > > They fail to detect features if the kernel was built with
>> > > CONFIG_MODULES=n
>> > > and modules.builtin isn't installed.
>> > >
>> > > Therefore, add the target "_builtin_inst_" and make "install" and
>> > > "modules_install" depend on it.
>> > >
>> > > Tests results:
>> > >
>> > >   - make install: kernel image is copied as before, modules.builtin
>> > > copied
>> > >   - make modules_install: (CONFIG_MODULES=n) nothing is copied, exit 1
>> > >
>> > > Signed-off-by: Jonas Zeiger <jonas.zeiger@talpidae.net>
>> > > ---
>> > >   Makefile | 14 +++++++++++---
>> > >   1 file changed, 11 insertions(+), 3 deletions(-)
>> >
>> > Note that this change broke builds in the Chrome OS build system
>> > because we require modules to be installed to a certain path and we
>> > weren't passing "INSTALL_MOD_PATH" when we called "make install".
>> >
>> > We can certainly fix our build system (I have a patch at
>> > https://crrev.com/c/2237511 for it), but I do wonder if others will
>> > hit the same issue.  Others might not have such a nice sandboxing
>> > system so they might unknowingly try to install files to the build
>> > computer's modules directory instead of their target.
>> >
>> 
>> I am more concerned with people getting errors such as
>> 
>> mkdir: cannot create directory '/lib/modules/5.7.0+/': Permission 
>> denied
>> 
>> when running "make install", with no documentation or explanation that
>> or why INSTALL_MOD_PATH is now mandatory for non-root installations.
>> Even for root installations, it seems odd that "make install" now
>> installs module files; after all, this is what "make modules_install"
>> is for.
>> 
>> I can understand the use case for CONFIG_MODULES=n, but the impact and
>> changed behavior on systems with CONFIG_MODULES=y is quite unexpected.
>> 
>> Guenter
> 
> 
> Sorry, I led this patch in a wrong way.
> 
> Maybe, we should allow 'make modules_install' for CONFIG_MODULES=n
> as Jonas did in v1.
> 
> 
> Another way might be to install it
> in /boot/modules.builtin.(ver) when CONFIG_MODULES=n
> but checking multiple locations would be inconvenient.

I have noticed that my build system specified INSTALL_MOD_PATH for "make 
install", so the patch doesn't cause issues in my environment.

However, I should have noticed that the change is breaking some existing 
setups.

Masahiro, I still believe that the approach you favored (v2) makes more 
sense architecturally, but at this point it seems that v1 is more 
pragmatic.

Would you agree to revert v2 and apply v1 instead?

I will fix issues that may come up with v1, however unlikely.