diff mbox series

[v2,1/3] selftests/futex: don't redefine .PHONY targets (all, clean)

Message ID 20240529022938.129624-2-jhubbard@nvidia.com (mailing list archive)
State Accepted
Commit 32c75ad4a79259609ee19f749832bc2d99bbdd13
Headers show
Series selftests/futex: clang-inspired fixes | expand

Commit Message

John Hubbard May 29, 2024, 2:29 a.m. UTC
The .PHONY targets "all" and "clean"  are both defined in the file that
is included in the very next line: ../lib.mk.

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/futex/Makefile | 2 --
 1 file changed, 2 deletions(-)

Comments

Shuah Khan May 30, 2024, 7:03 p.m. UTC | #1
On 5/28/24 20:29, John Hubbard wrote:
> The .PHONY targets "all" and "clean"  are both defined in the file that
> is included in the very next line: ../lib.mk.
> 

What problems are you seeing without this patch?
If I recall correctly, futex needs these defined.

Please provide information on why this change is
needed.

> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>   tools/testing/selftests/futex/Makefile | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile
> index 11e157d7533b..78ab2cd111f6 100644
> --- a/tools/testing/selftests/futex/Makefile
> +++ b/tools/testing/selftests/futex/Makefile
> @@ -3,8 +3,6 @@ SUBDIRS := functional
>   
>   TEST_PROGS := run.sh
>   
> -.PHONY: all clean
> -
>   include ../lib.mk
>   
>   all:

thanks,
-- Shuah
John Hubbard May 30, 2024, 7:13 p.m. UTC | #2
On 5/30/24 12:03 PM, Shuah Khan wrote:
> On 5/28/24 20:29, John Hubbard wrote:
>> The .PHONY targets "all" and "clean"  are both defined in the file that
>> is included in the very next line: ../lib.mk.
>>
> 
> What problems are you seeing without this patch?

Code duplication. It's a sin. :)

> If I recall correctly, futex needs these defined.

And so they are defined, in the very next line:

     include ../lib.mk

...which has this:

.PHONY: run_tests all clean install emit_tests gen_mods_dir clean_mods_dir

> 
> Please provide information on why this change is
> needed.

This patch is a valid cleanup, and doesn't introduce any problems
that I'm aware of. If there *are* problems that show up, those would
be deeper bugs, and I'll be happy to look into them and post solutions
if it comes up.

We don't just let latent bugs rest in peace. We fix them.

thanks,
Shuah Khan May 30, 2024, 9:12 p.m. UTC | #3
On 5/30/24 13:13, John Hubbard wrote:
> On 5/30/24 12:03 PM, Shuah Khan wrote:
>> On 5/28/24 20:29, John Hubbard wrote:
>>> The .PHONY targets "all" and "clean"  are both defined in the file that
>>> is included in the very next line: ../lib.mk.
>>>
>>
>> What problems are you seeing without this patch?
> 
> Code duplication. It's a sin. :)

Please mention that you are removing duplicate code.

futex Makefile overrides CLEAN - just making sure it
does the cleanup correctly.

thanks,
-- Shuah
John Hubbard May 30, 2024, 9:14 p.m. UTC | #4
On 5/30/24 2:12 PM, Shuah Khan wrote:
> On 5/30/24 13:13, John Hubbard wrote:
>> On 5/30/24 12:03 PM, Shuah Khan wrote:
>>> On 5/28/24 20:29, John Hubbard wrote:
>>>> The .PHONY targets "all" and "clean"  are both defined in the file that
>>>> is included in the very next line: ../lib.mk.
>>>>
>>>
>>> What problems are you seeing without this patch?
>>
>> Code duplication. It's a sin. :)
> 
> Please mention that you are removing duplicate code.
> 
> futex Makefile overrides CLEAN - just making sure it

(Overrides of Makefile things are per-item, so to speak, not per
file, just to be clear.)

> does the cleanup correctly.
> 

OK sure, I'll update the commit message and post a v3, coming soon.


thanks,
diff mbox series

Patch

diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile
index 11e157d7533b..78ab2cd111f6 100644
--- a/tools/testing/selftests/futex/Makefile
+++ b/tools/testing/selftests/futex/Makefile
@@ -3,8 +3,6 @@  SUBDIRS := functional
 
 TEST_PROGS := run.sh
 
-.PHONY: all clean
-
 include ../lib.mk
 
 all: