From: Ilya Leoshkevich <iii@linux.ibm.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Stanislav Fomichev <sdf@fomichev.me>,
Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
Networking <netdev@vger.kernel.org>,
Alexei Starovoitov <ast@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
Date: Wed, 31 Jul 2019 15:21:25 +0200 [thread overview]
Message-ID: <3D822EE0-C033-4192-B505-A30E5EC23BC3@linux.ibm.com> (raw)
In-Reply-To: <CAEf4Bzbj6RWUo8Q7wgMnbL=T7V2yK2=gbdO3sSfxJ71Gp6jeYA@mail.gmail.com>
> Am 27.07.2019 um 20:53 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
>
> On Fri, Jul 26, 2019 at 3:01 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>>
>> On 07/26, Andrii Nakryiko wrote:
>>> On Fri, Jul 26, 2019 at 2:21 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>>>>
>>>> On 07/26, Andrii Nakryiko wrote:
>>>>> Apprently listing header as a normal dependency for a binary output
>>>>> makes it go through compilation as if it was C code. This currently
>>>>> works without a problem, but in subsequent commits causes problems for
>>>>> differently generated test.h for test_progs. Marking those headers as
>>>>> order-only dependency solves the issue.
>>>> Are you sure it will not result in a situation where
>>>> test_progs/test_maps is not regenerated if tests.h is updated.
>>>>
>>>> If I read the following doc correctly, order deps make sense for
>>>> directories only:
>>>> https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
>>>>
>>>> Can you maybe double check it with:
>>>> * make
>>>> * add new prog_tests/test_something.c
>>>> * make
>>>> to see if the binary is regenerated with test_something.c?
>>>
>>> Yeah, tested that, it triggers test_progs rebuild.
>>>
>>> Ordering is still preserved, because test.h is dependency of
>>> test_progs.c, which is dependency of test_progs binary, so that's why
>>> it works.
>>>
>>> As to why .h file is compiled as C file, I have no idea and ideally
>>> that should be fixed somehow.
>> I guess that's because it's a prerequisite and we have a target that
>> puts all prerequisites when calling CC:
>>
>> test_progs: a.c b.c tests.h
>> gcc a.c b.c tests.h -o test_progs
>>
>> So gcc compiles each input file.
>
> If that's really a definition of the rule, then it seems not exactly
> correct. What if some of the prerequisites are some object files,
> directories, etc. I'd say the rule should only include .c files. I'll
> add it to my TODO list to go and check how all this is defined
> somewhere, but for now I'm leaving everything as is in this patch.
>
I believe it’s an implicit built-in rule, which is defined by make
itself here:
https://git.savannah.gnu.org/cgit/make.git/tree/default.c?h=4.2.1#n131
The observed behavior arises because these rules use $^ all over the
place. My 2c is that it would be better to make the rule explicit,
because it would cost just an extra line, but it would be immediately
obvious why the code is correct w.r.t. rebuilds.
Best regards,
Ilya
next prev parent reply other threads:[~2019-07-31 13:22 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-26 20:37 [PATCH bpf-next 0/9] Revamp test_progs as a test running framework Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code Andrii Nakryiko
2019-07-26 21:21 ` Stanislav Fomichev
2019-07-26 21:42 ` Andrii Nakryiko
2019-07-26 22:01 ` Stanislav Fomichev
2019-07-27 18:53 ` Andrii Nakryiko
2019-07-31 13:21 ` Ilya Leoshkevich [this message]
2019-07-31 17:04 ` Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 2/9] selftests/bpf: revamp test_progs to allow more control Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 3/9] selftests/bpf: add test selectors by number and name to test_progs Andrii Nakryiko
2019-07-26 21:25 ` Stanislav Fomichev
2019-07-26 21:45 ` Andrii Nakryiko
2019-07-26 22:03 ` Stanislav Fomichev
2019-07-26 20:37 ` [PATCH bpf-next 4/9] libbpf: add libbpf_swap_print to get previous print func Andrii Nakryiko
2019-07-26 21:28 ` Stanislav Fomichev
2019-07-26 21:47 ` Andrii Nakryiko
2019-07-27 0:30 ` Alexei Starovoitov
2019-07-27 18:49 ` Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 5/9] selftest/bpf: centralize libbpf logging management for test_progs Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 6/9] selftests/bpf: abstract away test log output Andrii Nakryiko
2019-07-26 21:31 ` Stanislav Fomichev
2019-07-26 21:51 ` Andrii Nakryiko
2019-07-26 22:26 ` Stanislav Fomichev
2019-07-27 0:34 ` Alexei Starovoitov
2019-07-27 18:56 ` Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 7/9] selftests/bpf: add sub-tests support for test_progs Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 8/9] selftests/bpf: convert bpf_verif_scale.c to sub-tests API Andrii Nakryiko
2019-07-26 20:37 ` [PATCH bpf-next 9/9] selftests/bpf: convert send_signal.c to use subtests Andrii Nakryiko
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=3D822EE0-C033-4192-B505-A30E5EC23BC3@linux.ibm.com \
--to=iii@linux.ibm.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
--cc=sdf@fomichev.me \
/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