From: Ujjwal Kumar <ujjwalkumar0501@gmail.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH RFC] kbuild: use interpreters to invoke scripts
Date: Fri, 2 Oct 2020 00:51:08 +0530 [thread overview]
Message-ID: <24932c9f-395d-38de-4bc9-152547abeba1@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2010011554160.24571@felia>
On 01/10/20 7:39 pm, Lukas Bulwahn wrote:
>
>
> On Thu, 1 Oct 2020, Ujjwal Kumar wrote:
>
>> On 01/10/20 4:38 pm, Lukas Bulwahn wrote:
>>>
>>>
>>> On Thu, 1 Oct 2020, Ujjwal Kumar wrote:
>>>
>>>> We cannot rely on execute bits to be set on files in the repository.
>>>> The build script should use the explicit interpreter when invoking any
>>>> script from the repository.
>>>>
>>>> Link: https://lore.kernel.org/lkml/20200830174409.c24c3f67addcce0cea9a9d4c@linux-foundation.org/
>>>> Link: https://lore.kernel.org/lkml/202008271102.FEB906C88@keescook/
>>>>
>>>> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
>>>> Suggested-by: Kees Cook <keescook@chromium.org>
>>>> Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
>>>> Signed-off-by: Ujjwal Kumar <ujjwalkumar0501@gmail.com>
>>>> ---
>>>> Documentation/Makefile | 16 ++++++++--------
>>>> Documentation/userspace-api/media/Makefile | 6 +++---
>>>> Makefile | 4 ++--
>>>> arch/arm64/kernel/vdso/Makefile | 2 +-
>>>> arch/arm64/kernel/vdso32/Makefile | 2 +-
>>>> arch/ia64/Makefile | 4 ++--
>>>> arch/nds32/kernel/vdso/Makefile | 2 +-
>>>> .../comedi/drivers/ni_routing/tools/Makefile | 6 +++---
>>>> scripts/Makefile.build | 2 +-
>>>> scripts/Makefile.package | 4 ++--
>>>> tools/bootconfig/Makefile | 2 +-
>>>> tools/bpf/Makefile.helpers | 2 +-
>>>> tools/lib/bpf/Makefile | 2 +-
>>>> tools/perf/Makefile.perf | 2 +-
>>>> tools/power/cpupower/Makefile | 2 +-
>>>> tools/testing/selftests/rcutorture/Makefile | 2 +-
>>>> tools/testing/selftests/rseq/Makefile | 2 +-
>>>> tools/testing/selftests/vm/Makefile | 6 +++---
>>>> tools/testing/selftests/wireguard/qemu/Makefile | 4 ++--
>>>> tools/testing/selftests/x86/Makefile | 6 +++---
>>>> 20 files changed, 39 insertions(+), 39 deletions(-)
>>>>
>>>
>>> You will probably need to split this patch into multiple patches, but the
>>> discussion will show how to split it best.
>>>
>>> Probably:
>>>
>>> - one for Documentation
>>> - one for general kbuild, maybe including arch
>>> - one for selftests
>>> - one for the other tools
>>> - one for the comedi driver
>>>
>>> So, what did you do for testing your change?
>>
>> First, I had to unset the execute bits on all files.
>> for i in $(find -executable -type f); do chmod -x $i; done
>>
>> To test the changes, I invoked the make rule under which my current
>> changes lie. And I could see a permission-denied whenever a script
>> was invoked. After the patch, the make rule progressed without
>> any permissions error.
>> I couldn't test each and every change, because some rules failed
>> before invoking the script and others could not be invoked by me.
>>
>
> Which could you not test? Please share that.
arch/arm64/kernel/vdso/Makefile
arch/arm64/kernel/vdso32/Makefile
arch/ia64/Makefile
arch/nds32/kernel/vdso/Makefile
drivers/staging/comedi/drivers/ni_routing/tools/Makefile
scripts/Makefile.build
scripts/Makefile.package
tools/bootconfig/Makefile
tools/bpf/Makefile.helpers
tools/lib/bpf/Makefile
tools/testing/selftests/rcutorture/Makefile
tools/testing/selftests/rseq/Makefile
tools/testing/selftests/vm/Makefile
tools/testing/selftests/wireguard/qemu/Makefile
tools/testing/selftests/x86/Makefile
>
>> I successfully tested the changes under Documentation/ (and some others).
>>
>> $ make htmldocs
>> make[1]: execvp: ./scripts/sphinx-pre-install: Permission denied
>> make[1]: *** [Documentation/Makefile:81: htmldocs] Error 127
>> make: *** [Makefile:1661: htmldocs] Error 2
>>
>> Files that I tested my changes on:
>> Documentation/Makefile | 16 ++++++++--------
>> Documentation/userspace-api/media/Makefile | 6 +++---
>> Makefile | 4 ++--
>> tools/perf/Makefile.perf | 2 +-
>> tools/power/cpupower/Makefile | 2 +-
>>
>>
>>>
>>> Did you check if CONFIG_SHELL is actually available in tools?
>>
>> Yes, I did check.
>> To verify, I echo(ed) the variables under a make task. And invoked
>> the task from srctree.
>>
>
> That sounds good.
>
>>>> --- a/arch/nds32/kernel/vdso/Makefile
>>>> +++ b/arch/nds32/kernel/vdso/Makefile
>>>> @@ -39,7 +39,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
>>>> # Generate VDSO offsets using helper script
>>>> gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh
>>>> quiet_cmd_vdsosym = VDSOSYM $@
>>>> - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@
>>>> + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C sort > $@
>>>>
>>>
>>> I guess it is better to modify gen-vdsosym.
>>
>> Do you mean something as follows:
>> gen-vdsosym := $(CONFIG_SHELL) $(srctree)/$(src)/gen_vdso_offsets.sh
>>
>> If so, I have never seen that style in the build files. Infact, the current
>> style is followed at many places.
>> For instance,
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/scripts/Makefile.lib?h=next-20201001#n398
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/tools/Makefile?h=next-20201001#n50
>>
>
> I searched around as well. I think your proposal is fine.
>
> How about splitting out a first patch on the generic kbuild files and send
> that out as RFC, first send the RFC patch to linux-kernel-mentees list,
> then if that looks good, we go for Masahiro and the linux-kbuild list?
>
> Lukas
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2020-10-01 19:21 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 10:37 [Linux-kernel-mentees] [PATCH RFC] kbuild: use interpreters to invoke scripts Ujjwal Kumar
2020-10-01 11:08 ` Lukas Bulwahn
2020-10-01 13:47 ` Ujjwal Kumar
2020-10-01 14:09 ` Lukas Bulwahn
2020-10-01 19:21 ` Ujjwal Kumar [this message]
2020-10-01 19:31 Ujjwal Kumar
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=24932c9f-395d-38de-4bc9-152547abeba1@gmail.com \
--to=ujjwalkumar0501@gmail.com \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=lukas.bulwahn@gmail.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).