[PATCH] Makefile: allow modules_install if CONFIG_MODULES=n

[PATCH] Makefile: allow modules_install if CONFIG_MODULES=n

[Jonas Zeiger]

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, allow the Makefile's modules_install target to be used 
always.

Tested Makefile targets with different CONFIG_MODULES states:

  * (CONFIG_MODULES=n) modules_install: install modules.builtin etc.
  * (CONFIG_MODULES=y) modules_install: produce same result as before
  * (CONFIG_MODULES=y) modules_install: still fail if no modules.order
  * (CONFIG_MODULES=y) modules: build modules, as before
  * (CONFIG_MODULES=n) modules: still fail and warn

Signed-off-by: Jonas Zeiger <jonas.zeiger@talpidae.net>
---

  Makefile |   60 
++++++++++++++++++++++++++++++++----------------------------
  1 file changed, 32 insertions(+), 28 deletions(-)

diff -up linux/Makefile{.orig,}
--- linux/Makefile.orig	2020-05-28 04:27:34.341394622 +0200
+++ linux/Makefile	2020-05-28 05:18:00.540108227 +0200
@@ -1309,31 +1309,7 @@ dt_binding_check: scripts_dtc
  # 
---------------------------------------------------------------------------
  # Modules

-ifdef CONFIG_MODULES
-
-# By default, build modules as well
-
-all: modules
-
-# Build modules
-#
-# A module can be listed more than once in obj-m resulting in
-# duplicate lines in modules.order files.  Those are removed
-# using awk while concatenating to the final file.
-
-PHONY += modules
-modules: $(if $(KBUILD_BUILTIN),vmlinux) modules.order
-	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/modules-check.sh
-
-modules.order: descend
-	$(Q)$(AWK) '!x[$$0]++' $(addsuffix /$@, $(build-dirs)) > $@
-
-# Target to prepare building external modules
-PHONY += modules_prepare
-modules_prepare: prepare
-
-# Target to install modules
+# Target to install modules and accompanying files
  PHONY += modules_install
  modules_install: _modinst_ _modinst_post

@@ -1347,10 +1323,14 @@ _modinst_:
  		rm -f $(MODLIB)/build ; \
  		ln -s $(CURDIR) $(MODLIB)/build ; \
  	fi
-	@sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
  	@cp -f modules.builtin $(MODLIB)/
  	@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
+ifdef CONFIG_MODULES
+	@sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
+else
+	@touch $(MODLIB)/modules.order
+endif

  # This depmod is only for convenience to give the initial
  # boot a modules.dep even before / is mounted read-write.  However the
@@ -1359,6 +1339,30 @@ PHONY += _modinst_post
  _modinst_post: _modinst_
  	$(call cmd,depmod)

+ifdef CONFIG_MODULES
+
+# By default, build modules as well
+
+all: modules
+
+# Build modules
+#
+# A module can be listed more than once in obj-m resulting in
+# duplicate lines in modules.order files.  Those are removed
+# using awk while concatenating to the final file.
+
+PHONY += modules
+modules: $(if $(KBUILD_BUILTIN),vmlinux) modules.order
+	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/modules-check.sh
+
+modules.order: descend
+	$(Q)$(AWK) '!x[$$0]++' $(addsuffix /$@, $(build-dirs)) > $@
+
+# Target to prepare building external modules
+PHONY += modules_prepare
+modules_prepare: prepare
+
  ifeq ($(CONFIG_MODULE_SIG), y)
  PHONY += modules_sign
  modules_sign:
@@ -1370,8 +1374,8 @@ else # CONFIG_MODULES
  # Modules not configured
  # 
---------------------------------------------------------------------------

-PHONY += modules modules_install
-modules modules_install:
+PHONY += modules
+modules:
  	@echo >&2
  	@echo >&2 "The present kernel configuration has modules disabled."
  	@echo >&2 "Type 'make config' and enable loadable module support."

Re: [PATCH] Makefile: allow modules_install if CONFIG_MODULES=n

[Masahiro Yamada]

On Thu, May 28, 2020 at 1:44 PM 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, allow the Makefile's modules_install target to be used
> always.
>
> Tested Makefile targets with different CONFIG_MODULES states:
>
>   * (CONFIG_MODULES=n) modules_install: install modules.builtin etc.
>   * (CONFIG_MODULES=y) modules_install: produce same result as before
>   * (CONFIG_MODULES=y) modules_install: still fail if no modules.order
>   * (CONFIG_MODULES=y) modules: build modules, as before
>   * (CONFIG_MODULES=n) modules: still fail and warn
>
> Signed-off-by: Jonas Zeiger <jonas.zeiger@talpidae.net>


Maybe, module.builtin and module.builtin.modinfo should be
installed by 'make install' because they are byproducts of vmlinux.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] Makefile: allow modules_install if CONFIG_MODULES=n

[Jonas Zeiger]

On 2020-05-31 20:16, Masahiro Yamada wrote:
> On Thu, May 28, 2020 at 1:44 PM 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, allow the Makefile's modules_install target to be used
>> always.
>> 
>> Tested Makefile targets with different CONFIG_MODULES states:
>> 
>>   * (CONFIG_MODULES=n) modules_install: install modules.builtin etc.
>>   * (CONFIG_MODULES=y) modules_install: produce same result as before
>>   * (CONFIG_MODULES=y) modules_install: still fail if no modules.order
>>   * (CONFIG_MODULES=y) modules: build modules, as before
>>   * (CONFIG_MODULES=n) modules: still fail and warn
>> 
>> Signed-off-by: Jonas Zeiger <jonas.zeiger@talpidae.net>
> 
> 
> Maybe, module.builtin and module.builtin.modinfo should be
> installed by 'make install' because they are byproducts of vmlinux.

Yes, thank you for this suggestion.

I will submit a new patch.

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

[Jonas Zeiger]

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(-)

diff --git a/Makefile b/Makefile
index a7bc91cbac8f..a160efd62897 100644
--- a/Makefile
+++ b/Makefile
@@ -1315,6 +1315,16 @@ dt_binding_check: scripts_dtc
  # 
---------------------------------------------------------------------------
  # Modules

+# install modules.builtin regardless of CONFIG_MODULES
+PHONY += _builtin_inst_
+_builtin_inst_:
+	@mkdir -p $(MODLIB)/
+	@cp -f modules.builtin $(MODLIB)/
+	@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
+
+PHONY += install
+install: _builtin_inst_
+
  ifdef CONFIG_MODULES

  # By default, build modules as well
@@ -1344,7 +1354,7 @@ PHONY += modules_install
  modules_install: _modinst_ _modinst_post

  PHONY += _modinst_
-_modinst_:
+_modinst_: _builtin_inst_
  	@rm -rf $(MODLIB)/kernel
  	@rm -f $(MODLIB)/source
  	@mkdir -p $(MODLIB)/kernel
@@ -1354,8 +1364,6 @@ _modinst_:
  		ln -s $(CURDIR) $(MODLIB)/build ; \
  	fi
  	@sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
-	@cp -f modules.builtin $(MODLIB)/
-	@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst

  # This depmod is only for convenience to give the initial
-- 
2.26.2

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

[Masahiro Yamada]

On Wed, Jun 3, 2020 at 10:34 PM 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>
> ---

Applied to linux-kbuild,
but this patch format is broken.

I manually fixed it up, but
please use 'git send-email' next time.




>   Makefile | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index a7bc91cbac8f..a160efd62897 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1315,6 +1315,16 @@ dt_binding_check: scripts_dtc
>   #
> ---------------------------------------------------------------------------
>   # Modules
>
> +# install modules.builtin regardless of CONFIG_MODULES
> +PHONY += _builtin_inst_
> +_builtin_inst_:
> +       @mkdir -p $(MODLIB)/
> +       @cp -f modules.builtin $(MODLIB)/
> +       @cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
> +
> +PHONY += install
> +install: _builtin_inst_
> +
>   ifdef CONFIG_MODULES
>
>   # By default, build modules as well
> @@ -1344,7 +1354,7 @@ PHONY += modules_install
>   modules_install: _modinst_ _modinst_post
>
>   PHONY += _modinst_
> -_modinst_:
> +_modinst_: _builtin_inst_
>         @rm -rf $(MODLIB)/kernel
>         @rm -f $(MODLIB)/source
>         @mkdir -p $(MODLIB)/kernel
> @@ -1354,8 +1364,6 @@ _modinst_:
>                 ln -s $(CURDIR) $(MODLIB)/build ; \
>         fi
>         @sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
> -       @cp -f modules.builtin $(MODLIB)/
> -       @cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
>         $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
>
>   # This depmod is only for convenience to give the initial
> --
> 2.26.2
>


-- 
Best Regards
Masahiro Yamada

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

[Doug Anderson]

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.

-Doug

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

[Guenter Roeck]

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

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

[Masahiro Yamada]

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.



-- 
Best Regards
Masahiro Yamada

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

[Guenter Roeck]

On Thu, Jun 11, 2020 at 11:57 PM Masahiro Yamada <masahiroy@kernel.org> 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.
>

Personally I think that would have been much better than the current
solution. It would specifically give people the choice to install
those files if they want to, without forcing others to do it (and to
do it in an unrelated context/command).

Thanks,
Guenter

>
> Another way might be to install it
> in /boot/modules.builtin.(ver) when CONFIG_MODULES=n
> but checking multiple locations would be inconvenient.
>
>
>
> --
> Best Regards
> Masahiro Yamada

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

[Jonas Zeiger]

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.

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

[Masahiro Yamada]

On Sat, Jun 13, 2020 at 12:35 AM Jonas Zeiger <jonas.zeiger@talpidae.net> wrote:
>
> 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?


Yes, I agree.

Thanks.


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


-- 
Best Regards
Masahiro Yamada