Re: [PATCH 2/2] selinux: move genheaders to security/selinux/

[Masahiro Yamada]

On Sat, Sep 7, 2024 at 12:37 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Fri, Sep 6, 2024 at 11:19 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
> > On Tue, Aug 27, 2024 at 6:22 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > On Aug  9, 2024 Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
> > > >
> > > > This tool is only used in security/selinux/Makefile.
> > > >
> > > > There is no reason to keep it under scripts/.
> > > >
> > > > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > > > ---
> > > >  scripts/remove-stale-files                                 | 3 +++
> > > >  scripts/selinux/Makefile                                   | 2 +-
> > > >  scripts/selinux/genheaders/.gitignore                      | 2 --
> > > >  scripts/selinux/genheaders/Makefile                        | 3 ---
> > > >  security/selinux/.gitignore                                | 1 +
> > > >  security/selinux/Makefile                                  | 7 +++++--
> > > >  .../selinux/genheaders => security/selinux}/genheaders.c   | 0
> > > >  7 files changed, 10 insertions(+), 8 deletions(-)
> > > >  delete mode 100644 scripts/selinux/genheaders/.gitignore
> > > >  delete mode 100644 scripts/selinux/genheaders/Makefile
> > > >  rename {scripts/selinux/genheaders => security/selinux}/genheaders.c (100%)
> > >
> > > As long as there is no harm in keeping genheaders under scripts/selinux,
> > > and based on your cover letter it would appear that there is no problem
> > > with the current location, I would prefer to keep it where it currently
> > > lives.
> >
> > 'make clean' is meant to clean up the tree, but keep
> > build artifacts necessary for building external modules.
> >
> > See the help message:
> >
> >   clean           - Remove most generated files but keep the config and
> >                     enough build support to build external modules
> >
> > 'make clean' does not clean up under scripts/
> > because tools located scripts/ are used in tree-wide
> > and often used for external modules as well.
> >
> > So, scripts/selinux/genheaders/genheaders is left over.
> >
> > genheaders is locally used in security/selinux/.
> >
> > 'make clean' will properly clean up security/selinux/genheaders.
>
> Your last sentence is confusing and doesn't align with the rest of
> your email, please clarify.


I do not understand what was unclear.



'make clean' cannot clean the current path,
scripts/selinux/genheaders/genheaders.

'make clean' can clean the proposed path,
security/selinux/genheaders.



genheaders is only used during the kernel build.
When you run 'make clean', there is no reason to keep it
for external modules.
Thus, it should move to the directory path that
'make clean' can handle.


Clearer?






>
> Regardless, this sort of explanation is what one needs to put in the
> commit description, a simple "There is no reason to keep it under
> scripts/" isn't sufficient.  Patches like this need to provide a well
> defined reason to justify the code churn.
>
> --
> paul-moore.com



-- 
Best Regards
Masahiro Yamada