linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Randy Dunlap <rdunlap@infradead.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: linux-kbuild <linux-kbuild@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Bjørn Forsman" <bjorn.forsman@gmail.com>
Subject: Re: [PATCH v2] kbuild: check for pkg-config on make {menu,n,g,x}config
Date: Fri, 6 Apr 2018 14:37:08 -0700	[thread overview]
Message-ID: <3ce8f87d-69fa-231a-b51c-9805e6525032@infradead.org> (raw)
In-Reply-To: <CAK7LNAQxcVRvhJn6GQVdLLuEq0sJEUcfwkbMpmx9j9nbjscQRg@mail.gmail.com>

On 03/14/2018 10:50 PM, Masahiro Yamada wrote:
> 2018-03-13 11:30 GMT+09:00 Randy Dunlap <rdunlap@infradead.org>:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> Each of 'make {menu,n,g,x}config' uses (needs) pkg-config to make sure
>> that other required files are present, but none of these check that
>> pkg-config itself is present.  Add a check for all 4 of these targets.
>>
>> Fixes kernel bugzilla #77511:
>> https://bugzilla.kernel.org/show_bug.cgi?id=77511
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>> v2: use 'command -v' instead of 'which'
>>
>> I'm also OK with just documenting the pkg-config requirement in
>> Documentation/Changes (= Documentation/process/changes.rst).
>>
>>  scripts/kconfig/Makefile           |   15 ++++++++++++++-
>>  scripts/kconfig/check-pkgconfig.sh |   12 ++++++++++++
>>  2 files changed, 26 insertions(+), 1 deletion(-)
>>
>> --- lnx-416-rc3.orig/scripts/kconfig/Makefile
>> +++ lnx-416-rc3/scripts/kconfig/Makefile
>> @@ -160,6 +160,9 @@ help:
>>         @echo  '  xenconfig       - Enable additional options for xen dom0 and guest kernel support'
>>         @echo  '  tinyconfig      - Configure the tiniest possible kernel'
>>
>> +# pkg-config check
>> +check-pkgconfig  := $(srctree)/$(src)/check-pkgconfig.sh
>> +
>>  # lxdialog stuff
>>  check-lxdialog  := $(srctree)/$(src)/lxdialog/check-lxdialog.sh
>>
>> @@ -205,7 +208,17 @@ $(addprefix $(obj)/, mconf.o $(lxdialog)
>>  $(obj)/dochecklxdialog:
>>         $(Q)$(CONFIG_SHELL) $(check-lxdialog) -check $(HOSTCC) $(HOST_EXTRACFLAGS) $(HOSTLOADLIBES_mconf)
>>
>> -always := dochecklxdialog
>> +# Check that we have pkg-config (used by each of menu/n/g/xconfig)
>> +PHONY += $(obj)/docheckpkgconfig
>> +$(addprefix $(obj)/, mconf.o): $(obj)/docheckpkgconfig
>> +$(addprefix $(obj)/, nconf.o): $(obj)/docheckpkgconfig
>> +$(addprefix $(obj)/, gconf.o): $(obj)/docheckpkgconfig
>> +$(addprefix $(obj)/, qconf.o): $(obj)/docheckpkgconfig
>> +
>> +$(obj)/docheckpkgconfig:
>> +       $(Q)$(CONFIG_SHELL) $(check-pkgconfig)
>> +
>> +always := docheckpkgconfig dochecklxdialog
> 
> 
> I did not test this patch, but does this check work as expected?
> 
> Probably we want to run 'docheckpkgconfig'
> before 'dochecklxdiag', '.tmp_gtkcheck', '.tmp_qtcheck', etc.
> But, I do not see such dependencies.
> 
> 
> Also, if we make 'pkg-config' mandatory,
> should we entirely drop fall-back logics like follows?
> 
> https://github.com/torvalds/linux/blob/v4.16-rc5/scripts/kconfig/lxdialog/check-lxdialog.sh#L10
> https://github.com/torvalds/linux/blob/v4.16-rc5/scripts/kconfig/lxdialog/check-lxdialog.sh#L29
> https://github.com/torvalds/linux/blob/v4.16-rc5/scripts/kconfig/Makefile#L230
> 
> 
> What do you think?
> 

Hi,

I'm willing to keep patching/testing on this, but both pkg-config and depmod
(for depmod, see: https://bugzilla.kernel.org/show_bug.cgi?id=198965)
are basic requirements IMO, so just documenting their requirements is good
enough to me, but might not be good enough for some users.

Comments?


> 
>>  # Add environment specific flags
>>  HOST_EXTRACFLAGS += $(shell $(CONFIG_SHELL) $(srctree)/$(src)/check.sh $(HOSTCC) $(HOSTCFLAGS))
>> --- /dev/null
>> +++ lnx-416-rc3/scripts/kconfig/check-pkgconfig.sh
>> @@ -0,0 +1,12 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Check for pkg-config presence
>> +
>> +pkgcfg=`command -v pkg-config`
>> +
>> +if [ "$pkgcfg" = "" ]; then
>> +       echo "'make *config' requires 'pkg-config'. Please install it." 1>&2
>> +       exit 1          # error
>> +fi
>> +
>> +exit 0

thanks,
-- 
~Randy

  reply	other threads:[~2018-04-06 21:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-13  2:30 [PATCH v2] kbuild: check for pkg-config on make {menu,n,g,x}config Randy Dunlap
2018-03-15  5:50 ` Masahiro Yamada
2018-04-06 21:37   ` Randy Dunlap [this message]
2018-05-18  5:10     ` Masahiro Yamada
2018-05-19 22:42       ` Randy Dunlap
2018-05-24 14:50         ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ce8f87d-69fa-231a-b51c-9805e6525032@infradead.org \
    --to=rdunlap@infradead.org \
    --cc=bjorn.forsman@gmail.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).