Re: [PATCH v3] modules_install: make missing $DEPMOD a warning instead of an error

[Randy Dunlap]

On 08/23/2018 09:20 PM, Masahiro Yamada wrote:
> Hi Randy,
> 
> 
> 2018-08-24 3:13 GMT+09:00 Randy Dunlap <rdunlap@xxxxxxxxxxxxx>:
>> From: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
>>
>> When $DEPMOD is not found, only print a warning instead of exiting
>> with an error message and error status.
> 
> 
> Could you add the motivation of this change
> (as Nikolaus reported) ?
> 
> Without the reason recorded in git-log,
> somebody may wonder why this commit is useful.
> 

Added for v4.

> 
>> Warning: 'make modules_install' requires /sbin/depmod. Please install it.
>> This is probably in the kmod package.
>>
>> Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
>> Fixes: 934193a654c1 ("kbuild: verify that $DEPMOD is installed")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxxxxxxx>
>> Cc: Lucas De Marchi <lucas.de.marchi@xxxxxxxxx>
>> Cc: Michal Marek <michal.lkml@xxxxxxxxxxx>
>> Cc: Jessica Yu <jeyu@xxxxxxxxxx>
>> Cc: Chih-Wei Huang <cwhuang@xxxxxxxxxxxx>
>> Cc: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
> 
> Reported-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
> 
> 

Added for v4.

> 
> 
>> ---
>> v2: add missing "exit 0" and update the commit message (no Error).
>> v3: add Fixes: and Cc: stable
>>
>>  scripts/depmod.sh |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> --- lnx-418.orig/scripts/depmod.sh
>> +++ lnx-418/scripts/depmod.sh
>> @@ -15,9 +15,9 @@ if ! test -r System.map ; then
>>  fi
>>
>>  if [ -z $(command -v $DEPMOD) ]; then
>> -       echo "'make modules_install' requires $DEPMOD. Please install it." >&2
>> +       echo "Warning: 'make modules_install' requires $DEPMOD. Please install it." >&2
>>         echo "This is probably in the kmod package." >&2
>> -       exit 1
>> +       exit 0
>>  fi
>>
>>  # older versions of depmod require the version string to start with three
> 
> 
> 
> BTW, if System.map is missing, depmod is silently skipped.
> 
> 
> I think if System.map is missing, a user is doing something wrong,
> but you could do like this:
> 
> $ make mrproper
>    [snip]
> $ make defconfig
>    [snip]
> $ make modules
>    [snip]
> $ make modules_install
>   INSTALL drivers/thermal/x86_pkg_temp_thermal.ko
>   INSTALL fs/efivarfs/efivarfs.ko
>   INSTALL net/ipv4/netfilter/ipt_MASQUERADE.ko
>   INSTALL net/ipv4/netfilter/iptable_nat.ko
>   INSTALL net/ipv4/netfilter/nf_log_arp.ko
>   INSTALL net/ipv4/netfilter/nf_log_ipv4.ko
>   INSTALL net/ipv4/netfilter/nf_nat_ipv4.ko
>   INSTALL net/ipv6/netfilter/nf_log_ipv6.ko
>   INSTALL net/netfilter/nf_log_common.ko
>   INSTALL net/netfilter/nf_nat.ko
>   INSTALL net/netfilter/nf_nat_ftp.ko
>   INSTALL net/netfilter/nf_nat_irc.ko
>   INSTALL net/netfilter/nf_nat_sip.ko
>   INSTALL net/netfilter/xt_LOG.ko
>   INSTALL net/netfilter/xt_addrtype.ko
>   INSTALL net/netfilter/xt_mark.ko
>   INSTALL net/netfilter/xt_nat.ko
>   DEPMOD  4.18.0+
> 
> 
> The log looks like all the process went well,
> but depmod is actually skipped because System.map is
> generated by "make vmlinux"
> 
> 
> What do you think we should do about this case?
> 
> Skip depmod silently?
> Make it fail?
> Print noisy warning like missing depmod case?

Since this shouldn't be happening, I prefer the noisy warning message
like the missing depmod case.

Do you want a separate patch for that?

thanks,
-- 
~Randy