diff mbox series

[v3,2/3] selftests/x86/Makefile: Support per-target $(LIBS) configuration

Message ID 54ae0e1f8928160c1c4120263ea21c8133aa3ec4.1641398395.git.fweimer@redhat.com
State Not applicable
Headers
Series [v3,1/3] x86: Implement arch_prctl(ARCH_VSYSCALL_CONTROL) to disable vsyscall |

Checks

Context Check Description
dj/TryBot-apply_patch fail Patch failed to apply to master at the time it was sent

Commit Message

Florian Weimer Jan. 5, 2022, 4:03 p.m. UTC 
  And avoid compiling PCHs by accident.

Signed-off-by: Florian Weimer <fweimer@redhat.com>
---
v3: Patch split out.

 tools/testing/selftests/x86/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Andy Lutomirski Jan. 13, 2022, 9:31 p.m. UTC | #1 
On 1/5/22 08:03, Florian Weimer wrote:
> And avoid compiling PCHs by accident.
> 

The patch seems fine, but I can't make heads or tails of the $SUBJECT. 
Can you help me?

> Signed-off-by: Florian Weimer <fweimer@redhat.com>
> ---
> v3: Patch split out.
> 
>   tools/testing/selftests/x86/Makefile | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
> index 8a1f62ab3c8e..0993d12f2c38 100644
> --- a/tools/testing/selftests/x86/Makefile
> +++ b/tools/testing/selftests/x86/Makefile
> @@ -72,10 +72,12 @@ all_64: $(BINARIES_64)
>   EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
>   
>   $(BINARIES_32): $(OUTPUT)/%_32: %.c helpers.h
> -	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm
> +	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
> +		$(or $(LIBS), -lrt -ldl -lm)
>   
>   $(BINARIES_64): $(OUTPUT)/%_64: %.c helpers.h
> -	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
> +	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
> +		$(or $(LIBS), -lrt -ldl -lm)
>   
>   # x86_64 users should be encouraged to install 32-bit libraries
>   ifeq ($(CAN_BUILD_I386)$(CAN_BUILD_X86_64),01)
Florian Weimer Jan. 13, 2022, 10 p.m. UTC | #2 
* Andy Lutomirski:

> On 1/5/22 08:03, Florian Weimer wrote:
>> And avoid compiling PCHs by accident.
>> 
>
> The patch seems fine, but I can't make heads or tails of the
> $SUBJECT. Can you help me?

What about this?

selftests/x86/Makefile: Set linked libraries using $(LIBS)

I guess that it's possible to use make features to set this per target
isn't important.

Thanks,
Florian

>> Signed-off-by: Florian Weimer <fweimer@redhat.com>
>> ---
>> v3: Patch split out.
>>   tools/testing/selftests/x86/Makefile | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/tools/testing/selftests/x86/Makefile
>> b/tools/testing/selftests/x86/Makefile
>> index 8a1f62ab3c8e..0993d12f2c38 100644
>> --- a/tools/testing/selftests/x86/Makefile
>> +++ b/tools/testing/selftests/x86/Makefile
>> @@ -72,10 +72,12 @@ all_64: $(BINARIES_64)
>>   EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
>>     $(BINARIES_32): $(OUTPUT)/%_32: %.c helpers.h
>> -	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm
>> +	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
>> +		$(or $(LIBS), -lrt -ldl -lm)
>>     $(BINARIES_64): $(OUTPUT)/%_64: %.c helpers.h
>> -	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
>> +	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
>> +		$(or $(LIBS), -lrt -ldl -lm)
>>     # x86_64 users should be encouraged to install 32-bit libraries
>>   ifeq ($(CAN_BUILD_I386)$(CAN_BUILD_X86_64),01)
Andy Lutomirski Jan. 14, 2022, 2:34 a.m. UTC | #3 
On Thu, Jan 13, 2022, at 2:00 PM, Florian Weimer wrote:
> * Andy Lutomirski:
>
>> On 1/5/22 08:03, Florian Weimer wrote:
>>> And avoid compiling PCHs by accident.
>>> 
>>
>> The patch seems fine, but I can't make heads or tails of the
>> $SUBJECT. Can you help me?
>
> What about this?
>
> selftests/x86/Makefile: Set linked libraries using $(LIBS)
>
> I guess that it's possible to use make features to set this per target
> isn't important.

I think that's actually important -- it's nice to explain to make dummies (e.g. me) what the purpose is is.  I assume it's so that a given test can override the libraries.  Also, you've conflated two different changes into one patch: removal of .h and addition of LIBS.

--Andy



>
> Thanks,
> Florian
>
>>> Signed-off-by: Florian Weimer <fweimer@redhat.com>
>>> ---
>>> v3: Patch split out.
>>>   tools/testing/selftests/x86/Makefile | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>> diff --git a/tools/testing/selftests/x86/Makefile
>>> b/tools/testing/selftests/x86/Makefile
>>> index 8a1f62ab3c8e..0993d12f2c38 100644
>>> --- a/tools/testing/selftests/x86/Makefile
>>> +++ b/tools/testing/selftests/x86/Makefile
>>> @@ -72,10 +72,12 @@ all_64: $(BINARIES_64)
>>>   EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
>>>     $(BINARIES_32): $(OUTPUT)/%_32: %.c helpers.h
>>> -	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm
>>> +	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
>>> +		$(or $(LIBS), -lrt -ldl -lm)
>>>     $(BINARIES_64): $(OUTPUT)/%_64: %.c helpers.h
>>> -	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
>>> +	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
>>> +		$(or $(LIBS), -lrt -ldl -lm)
>>>     # x86_64 users should be encouraged to install 32-bit libraries
>>>   ifeq ($(CAN_BUILD_I386)$(CAN_BUILD_X86_64),01)
Florian Weimer Jan. 14, 2022, 1:28 p.m. UTC | #4 
* Andy Lutomirski:

> On Thu, Jan 13, 2022, at 2:00 PM, Florian Weimer wrote:
>> * Andy Lutomirski:
>>
>>> On 1/5/22 08:03, Florian Weimer wrote:
>>>> And avoid compiling PCHs by accident.
>>>> 
>>>
>>> The patch seems fine, but I can't make heads or tails of the
>>> $SUBJECT. Can you help me?
>>
>> What about this?
>>
>> selftests/x86/Makefile: Set linked libraries using $(LIBS)
>>
>> I guess that it's possible to use make features to set this per target
>> isn't important.
>
> I think that's actually important -- it's nice to explain to make
> dummies (e.g. me) what the purpose is is.  I assume it's so that a
> given test can override the libraries.  Also, you've conflated two
> different changes into one patch: removal of .h and addition of LIBS.

Do you want me to split this further into two commits?

  selftests/x86/Makefile: Per-target configuration of linked libraries

  Targets can set $(LIBS) to specify a different set of libraries than the
  defaults (or no libraries at all).

And:

  selftests/x86/Makefile: Do not pass header files as compiler inputs

  Filtering out .h files avoids accidentally creating a precompiled
  header.

I didn't want to game commit metrics.

Thanks,
Florian
diff mbox series

Patch

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 8a1f62ab3c8e..0993d12f2c38 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -72,10 +72,12 @@  all_64: $(BINARIES_64)
 EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
 
 $(BINARIES_32): $(OUTPUT)/%_32: %.c helpers.h
-	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm
+	$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
+		$(or $(LIBS), -lrt -ldl -lm)
 
 $(BINARIES_64): $(OUTPUT)/%_64: %.c helpers.h
-	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
+	$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(filter-out %.h, $^) \
+		$(or $(LIBS), -lrt -ldl -lm)
 
 # x86_64 users should be encouraged to install 32-bit libraries
 ifeq ($(CAN_BUILD_I386)$(CAN_BUILD_X86_64),01)