The current 'find' command does not prune the rust/test directory
itself, requiring an additional 'grep -Fv' command to exclude it.
This is cumbersome.

The correct use of the -prune option can be seen in the 'make clean'
rule.

[Current command]

  $ find . -type f -name '*.rs' -o -path ./rust/test -prune | wc
       70      70    1939
  $ find . -type f -name '*.rs' -o -path ./rust/test -prune | grep rust/test
  ./rust/test

[Improved command]

  $ find . -path ./rust/test -prune -o -type f -name '*.rs' -print | wc
       69      69    1927
  $ find . -path ./rust/test -prune -o -type f -name '*.rs' -print | grep rust/test

With the improved 'find' command, the grep command is no longer needed.

There is also no need to use the absolute path, so $(abs_srctree) can be
replaced with $(srctree).

The pruned directory rust/test must be prefixed with $(srctree) instead
of $(objtree). Otherwise, 'make O=... rustfmt' would visit the stale
rust/test directory remaining in the source tree.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 5b16e0605a77..4992b2895dd5 100644
--- a/Makefile
+++ b/Makefile
@@ -1740,9 +1740,8 @@ PHONY += rustfmt rustfmtcheck
 # when matching, which is a problem when e.g. `srctree` is `..`.
 # We `grep` afterwards in order to remove the directory entry itself.
 rustfmt:
-	$(Q)find $(abs_srctree) -type f -name '*.rs' \
-		-o -path $(abs_objtree)/rust/test -prune \
-		| grep -Fv $(abs_objtree)/rust/test \
+	$(Q)find $(srctree) -path $(srctree)/rust/test -prune \
+		-o -type f -name '*.rs' -print \
 		| grep -Fv generated \
 		| xargs $(RUSTFMT) $(rustfmt_flags)
 
-- 
2.43.0

On Tue, Sep 17, 2024 at 4:17 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The correct use of the -prune option can be seen in the 'make clean'
> rule.

Yeah, this `-prune` should not have been like that -- sorry about that.

The comment above this recipe should be updated.

I am not sure I understand the part of the commit message about the
rust/test change. Do you mean that we should use `srctree` in case
there is a stale one in the source tree from a previous
non-completely-clean in-source-tree build? I think the original
intention was to skip the objtree one if it were a subdir of srctree
(and that is why the use of absolute paths).

Although I think we can simplify further by just removing the logic
about `rust/test`, since we don't generate `*.rs` files there anyway
at the moment.

Cheers,
Miguel

On Tue, Oct 1, 2024 at 3:38 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Tue, Sep 17, 2024 at 4:17 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > The correct use of the -prune option can be seen in the 'make clean'
> > rule.
>
> Yeah, this `-prune` should not have been like that -- sorry about that.
>
> The comment above this recipe should be updated.
>
> I am not sure I understand the part of the commit message about the
> rust/test change. Do you mean that we should use `srctree` in case
> there is a stale one in the source tree from a previous
> non-completely-clean in-source-tree build?

Correct.


> I think the original
> intention was to skip the objtree one if it were a subdir of srctree
> (and that is why the use of absolute paths).


OK, understood.

If you insist on the current logic, I will keep it as-is,
but I need to replace $(abs_objtree) with $(CURDIR)
because $(abs_objtree) will be removed by this series.




> Although I think we can simplify further by just removing the logic
> about `rust/test`, since we don't generate `*.rs` files there anyway
> at the moment.


OK, then we can remove -prune entirely.









--
Best Regards
Masahiro Yamada

On Tue, Oct 1, 2024 at 4:18 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> OK, then we can remove -prune entirely.

Yeah, I think that is the simplest approach.

With those changes, please feel free to add:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Thanks!

Cheers,
Miguel

On Tue, Sep 17, 2024 at 11:16:36PM +0900, Masahiro Yamada wrote:
> The current 'find' command does not prune the rust/test directory
> itself, requiring an additional 'grep -Fv' command to exclude it.
> This is cumbersome.
> 
> The correct use of the -prune option can be seen in the 'make clean'
> rule.
> 
> [Current command]
> 
>   $ find . -type f -name '*.rs' -o -path ./rust/test -prune | wc
>        70      70    1939
>   $ find . -type f -name '*.rs' -o -path ./rust/test -prune | grep rust/test
>   ./rust/test
> 
> [Improved command]
> 
>   $ find . -path ./rust/test -prune -o -type f -name '*.rs' -print | wc
>        69      69    1927
>   $ find . -path ./rust/test -prune -o -type f -name '*.rs' -print | grep rust/test
> 
> With the improved 'find' command, the grep command is no longer needed.
> 
> There is also no need to use the absolute path, so $(abs_srctree) can be
> replaced with $(srctree).
> 
> The pruned directory rust/test must be prefixed with $(srctree) instead
> of $(objtree). Otherwise, 'make O=... rustfmt' would visit the stale
> rust/test directory remaining in the source tree.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  Makefile | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 5b16e0605a77..4992b2895dd5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1740,9 +1740,8 @@ PHONY += rustfmt rustfmtcheck
>  # when matching, which is a problem when e.g. `srctree` is `..`.
>  # We `grep` afterwards in order to remove the directory entry itself.
>  rustfmt:
> -	$(Q)find $(abs_srctree) -type f -name '*.rs' \
> -		-o -path $(abs_objtree)/rust/test -prune \
> -		| grep -Fv $(abs_objtree)/rust/test \
> +	$(Q)find $(srctree) -path $(srctree)/rust/test -prune \
> +		-o -type f -name '*.rs' -print \
>  		| grep -Fv generated \

Is there a reason for keeping the grep for generated instead of turning
it also into a find prune argument?

Reviewed-by: Nicolas Schier <n.schier@avm.de>

On Thu, Sep 19, 2024 at 10:54 PM Nicolas Schier <n.schier@avm.de> wrote:
>
> On Tue, Sep 17, 2024 at 11:16:36PM +0900, Masahiro Yamada wrote:
> > The current 'find' command does not prune the rust/test directory
> > itself, requiring an additional 'grep -Fv' command to exclude it.
> > This is cumbersome.
> >
> > The correct use of the -prune option can be seen in the 'make clean'
> > rule.
> >
> > [Current command]
> >
> >   $ find . -type f -name '*.rs' -o -path ./rust/test -prune | wc
> >        70      70    1939
> >   $ find . -type f -name '*.rs' -o -path ./rust/test -prune | grep rust/test
> >   ./rust/test
> >
> > [Improved command]
> >
> >   $ find . -path ./rust/test -prune -o -type f -name '*.rs' -print | wc
> >        69      69    1927
> >   $ find . -path ./rust/test -prune -o -type f -name '*.rs' -print | grep rust/test
> >
> > With the improved 'find' command, the grep command is no longer needed.
> >
> > There is also no need to use the absolute path, so $(abs_srctree) can be
> > replaced with $(srctree).
> >
> > The pruned directory rust/test must be prefixed with $(srctree) instead
> > of $(objtree). Otherwise, 'make O=... rustfmt' would visit the stale
> > rust/test directory remaining in the source tree.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  Makefile | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 5b16e0605a77..4992b2895dd5 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1740,9 +1740,8 @@ PHONY += rustfmt rustfmtcheck
> >  # when matching, which is a problem when e.g. `srctree` is `..`.
> >  # We `grep` afterwards in order to remove the directory entry itself.
> >  rustfmt:
> > -     $(Q)find $(abs_srctree) -type f -name '*.rs' \
> > -             -o -path $(abs_objtree)/rust/test -prune \
> > -             | grep -Fv $(abs_objtree)/rust/test \
> > +     $(Q)find $(srctree) -path $(srctree)/rust/test -prune \
> > +             -o -type f -name '*.rs' -print \
> >               | grep -Fv generated \
>
> Is there a reason for keeping the grep for generated instead of turning
> it also into a find prune argument?


This commit answers your question:

https://github.com/Rust-for-Linux/linux/commit/73243a8a27a67




-- 
Best Regards
Masahiro Yamada