security/selinux/Makefile | 3 +++ 1 file changed, 3 insertions(+)
av_permissions.h is not declared as a target and therefore won't be
added to clean-files automatically by kbuild.
For details why it is not a target see the Makefile itself.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
security/selinux/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 86f0575f670da66a9dc57e13a236d6a5551af38e..58129a7c8cfa08f9caf5444f7df776f41056b77a 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -41,5 +41,8 @@ targets += flask.h
$(obj)/flask.h: $(obj)/genheaders FORCE
$(call if_changed,genhdrs)
+# see the note above, remove this line
+clean-files += av_permissions.h
+
hostprogs := genheaders
HOST_EXTRACFLAGS += -I$(srctree)/security/selinux/include
---
base-commit: bf9aa14fc523d2763fc9a10672a709224e8fcaf4
change-id: 20241120-selinux-clean-dfcd0e7a344b
Best regards,
--
Thomas Weißschuh <thomas.weissschuh@linutronix.de>
On Wed, Nov 20, 2024 at 6:15 PM Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote: > > av_permissions.h is not declared as a target and therefore won't be > added to clean-files automatically by kbuild. > For details why it is not a target see the Makefile itself. > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > --- > security/selinux/Makefile | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/security/selinux/Makefile b/security/selinux/Makefile > index 86f0575f670da66a9dc57e13a236d6a5551af38e..58129a7c8cfa08f9caf5444f7df776f41056b77a 100644 > --- a/security/selinux/Makefile > +++ b/security/selinux/Makefile > @@ -41,5 +41,8 @@ targets += flask.h > $(obj)/flask.h: $(obj)/genheaders FORCE > $(call if_changed,genhdrs) > > +# see the note above, remove this line > +clean-files += av_permissions.h > + > hostprogs := genheaders > HOST_EXTRACFLAGS += -I$(srctree)/security/selinux/include Presumably, the attached fixup.diff (comment in 'targets' assignment) would align with the intention of the maintainer of this Makefile because you can do targets += $(genhdrs) without the need of the grouped target feature. 'make clean' removes files listed in 'targets'. BTW, the NOTE in this Makefile is not true. https://github.com/torvalds/linux/blob/v6.12/security/selinux/Makefile#L7 Even if you use GNU Make 4.3, the grouped target does not work with the if_changed macro. With GNU Make 4.4, it will work as a side-effect of commit fabb03eac412b5ea19f1a97be31dc8c6fa7fc047 I asked about this behavior some time ago in GNU Make ML. https://lists.gnu.org/archive/html/help-make/2024-08/msg00001.html or https://savannah.gnu.org/bugs/index.php?66073 The combination of the grouped target and if_changed is working with GNU Make 4.4+, but I do not know if it is future promising. IMHO, I do not see much benefits for using the group target in this case because you can still generate flask.h and av_permissions.h separately. > > --- > base-commit: bf9aa14fc523d2763fc9a10672a709224e8fcaf4 > change-id: 20241120-selinux-clean-dfcd0e7a344b > > Best regards, > -- > Thomas Weißschuh <thomas.weissschuh@linutronix.de> > -- Best Regards Masahiro Yamada
On Wed, Nov 20, 2024 at 6:55 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > On Wed, Nov 20, 2024 at 6:15 PM Thomas Weißschuh > <thomas.weissschuh@linutronix.de> wrote: > > > > av_permissions.h is not declared as a target and therefore won't be > > added to clean-files automatically by kbuild. > > For details why it is not a target see the Makefile itself. > > > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > --- > > security/selinux/Makefile | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/security/selinux/Makefile b/security/selinux/Makefile > > index 86f0575f670da66a9dc57e13a236d6a5551af38e..58129a7c8cfa08f9caf5444f7df776f41056b77a 100644 > > --- a/security/selinux/Makefile > > +++ b/security/selinux/Makefile > > @@ -41,5 +41,8 @@ targets += flask.h > > $(obj)/flask.h: $(obj)/genheaders FORCE > > $(call if_changed,genhdrs) > > > > +# see the note above, remove this line > > +clean-files += av_permissions.h > > + > > hostprogs := genheaders > > HOST_EXTRACFLAGS += -I$(srctree)/security/selinux/include > > Presumably, the attached fixup.diff (comment in 'targets' assignment) > would align with the intention of the maintainer of this Makefile > because you can do > > targets += $(genhdrs) > > without the need of the grouped target feature. > 'make clean' removes files listed in 'targets'. [SIDE NOTE: thank you for the comments/patch, but please don't post patches as attachments, it adds extra steps to review/reply which I find annoying] This approach looks a bit cleaner to me. I thought there was a reason why we only included flask.h in $targets, but I can't remember it well enough to know what it was or even if my memory is correct on this :) > BTW, the NOTE in this Makefile is not true. > https://github.com/torvalds/linux/blob/v6.12/security/selinux/Makefile#L7 > > Even if you use GNU Make 4.3, the grouped target does not work with > the if_changed macro. > > With GNU Make 4.4, it will work as a side-effect of commit > fabb03eac412b5ea19f1a97be31dc8c6fa7fc047 Patches correcting the Makefile comments are welcome, especially if what the comments are suggesting for future revisions is incorrect :) > IMHO, I do not see much benefits for using the group target in this case > because you can still generate flask.h and av_permissions.h > separately. True, although a change in the classmap.h file is going to result in both being generated; generating only one file is going to be very rare and not something a normal developer would see. -- paul-moore.com
Hi, On Wed, Nov 20, 2024 at 08:55:00PM +0900, Masahiro Yamada wrote: > On Wed, Nov 20, 2024 at 6:15 PM Thomas Weißschuh > <thomas.weissschuh@linutronix.de> wrote: > > > > av_permissions.h is not declared as a target and therefore won't be > > added to clean-files automatically by kbuild. > > For details why it is not a target see the Makefile itself. > > > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > --- > > security/selinux/Makefile | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/security/selinux/Makefile b/security/selinux/Makefile > > index 86f0575f670da66a9dc57e13a236d6a5551af38e..58129a7c8cfa08f9caf5444f7df776f41056b77a 100644 > > --- a/security/selinux/Makefile > > +++ b/security/selinux/Makefile > > @@ -41,5 +41,8 @@ targets += flask.h > > $(obj)/flask.h: $(obj)/genheaders FORCE > > $(call if_changed,genhdrs) > > > > +# see the note above, remove this line > > +clean-files += av_permissions.h > > + > > hostprogs := genheaders > > HOST_EXTRACFLAGS += -I$(srctree)/security/selinux/include > > > > Presumably, the attached fixup.diff (comment in 'targets' assignment) > would align with the intention of the maintainer of this Makefile > because you can do > > targets += $(genhdrs) > > without the need of the grouped target feature. > 'make clean' removes files listed in 'targets'. > > > > BTW, the NOTE in this Makefile is not true. > https://github.com/torvalds/linux/blob/v6.12/security/selinux/Makefile#L7 > > > Even if you use GNU Make 4.3, the grouped target does not work with > the if_changed macro. > > With GNU Make 4.4, it will work as a side-effect of commit > fabb03eac412b5ea19f1a97be31dc8c6fa7fc047 > > > I asked about this behavior some time ago in GNU Make ML. > > https://lists.gnu.org/archive/html/help-make/2024-08/msg00001.html > or > https://savannah.gnu.org/bugs/index.php?66073 > > > The combination of the grouped target and if_changed > is working with GNU Make 4.4+, but I do not know if > it is future promising. Thanks for all the insights! > IMHO, I do not see much benefits for using the group target in this case > because you can still generate flask.h and av_permissions.h > separately. I'm fine either way. @Selinux maintainers: What do you prefer? Also feel free to just commit whatever you think is best. Thomas
© 2016 - 2024 Red Hat, Inc.