diff mbox series

[OS-BUILD] Adding support for distro targets in Makefile

Message ID 20221123153202.1814324-1-dzickus@redhat.com (mailing list archive)
State New, archived
Headers show
Series [OS-BUILD] Adding support for distro targets in Makefile | expand

Commit Message

Don Zickus Nov. 23, 2022, 3:32 p.m. UTC 
Fedora adds a directory to its source git tree to provide packaging
information[1] specific for its distro.  We would like to propagate our
'help' section to the toplevel to be seen by 'make help' as it isn't
obvious to users to use 'make dist-help'[2].

Instead of keeping Fedora changes local, I am proposing a generic
mechanism for other distros to use if they would like.  The change looks
for a distro directory and leverages that Makefile if it exists.
Otherwise it is ignored.

[1] - https://gitlab.com/cki-project/kernel-ark/-/tree/os-build/redhat
[2] - https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/redhat/Makefile#L809

Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>

----
This patch is more of a conversation starter than anything. 

I think other distros might find this useful and examples of what we would
populate the directory with can be found in [1].

Thoughts?

---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Masahiro Yamada Nov. 23, 2022, 3:41 p.m. UTC | #1 
On Thu, Nov 24, 2022 at 12:32 AM Don Zickus <dzickus@redhat.com> wrote:
>
> Fedora adds a directory to its source git tree to provide packaging
> information[1] specific for its distro.  We would like to propagate our
> 'help' section to the toplevel to be seen by 'make help' as it isn't
> obvious to users to use 'make dist-help'[2].
>
> Instead of keeping Fedora changes local, I am proposing a generic
> mechanism for other distros to use if they would like.  The change looks
> for a distro directory and leverages that Makefile if it exists.
> Otherwise it is ignored.
>
> [1] - https://gitlab.com/cki-project/kernel-ark/-/tree/os-build/redhat
> [2] - https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/redhat/Makefile#L809
>
> Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
> Signed-off-by: Don Zickus <dzickus@redhat.com>
>
> ----
> This patch is more of a conversation starter than anything.
>
> I think other distros might find this useful and examples of what we would
> populate the directory with can be found in [1].
>
> Thoughts?

I do not like to merge any code that is irrelevant to the upstream.


> ---
>  Makefile | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 6f846b1f2618f..45fdb18dde46f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1635,6 +1635,16 @@ distclean: mrproper
>  %pkg: include/config/kernel.release FORCE
>         $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.package $@
>
> +# Distribution of the kernel
> +# ---------------------------------------------------------------------------
> +
> +dist%: FORCE
> +       @if ! test -f $(srctree)/distro/Makefile; then \
> +               echo 'No distribution targets defined'; \
> +               exit 1; \
> +       fi
> +       $(Q)$(MAKE) -C $(srctree)/distro $@
> +
>  # Brief documentation of the typical targets used
>  # ---------------------------------------------------------------------------
>
> @@ -1732,6 +1742,11 @@ help:
>         @echo  ''
>         @echo  'Kernel packaging:'
>         @$(MAKE) -f $(srctree)/scripts/Makefile.package help
> +       @if test -f $(srctree)/distro/Makefile; then \
> +               echo ''; \
> +               echo 'Distro targets:'; \
> +               $(MAKE) -C $(srctree)/distro dist-help; \
> +       fi
>         @echo  ''
>         @echo  'Documentation targets:'
>         @$(MAKE) -f $(srctree)/Documentation/Makefile dochelp
> --
> 2.38.1
>
Justin Forbes Nov. 23, 2022, 4:08 p.m. UTC | #2 
On Wed, Nov 23, 2022 at 9:59 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Nov 24, 2022 at 12:32 AM Don Zickus <dzickus@redhat.com> wrote:
> >
> > Fedora adds a directory to its source git tree to provide packaging
> > information[1] specific for its distro.  We would like to propagate our
> > 'help' section to the toplevel to be seen by 'make help' as it isn't
> > obvious to users to use 'make dist-help'[2].
> >
> > Instead of keeping Fedora changes local, I am proposing a generic
> > mechanism for other distros to use if they would like.  The change looks
> > for a distro directory and leverages that Makefile if it exists.
> > Otherwise it is ignored.
> >
> > [1] - https://gitlab.com/cki-project/kernel-ark/-/tree/os-build/redhat
> > [2] - https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/redhat/Makefile#L809
> >
> > Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
> > Signed-off-by: Don Zickus <dzickus@redhat.com>
> >
> > ----
> > This patch is more of a conversation starter than anything.
> >
> > I think other distros might find this useful and examples of what we would
> > populate the directory with can be found in [1].
> >
> > Thoughts?
>
> I do not like to merge any code that is irrelevant to the upstream.


While not directly relevant to the upstream tree, it is very relevant
to the way the tree is being used. More importantly, it allows a
"distro agnostic" mechanism without requiring distributions or other
developers/users with their own additional make targets to avoid
carrying a patch to make it all work.

Justin

>
> > ---
> >  Makefile | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 6f846b1f2618f..45fdb18dde46f 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1635,6 +1635,16 @@ distclean: mrproper
> >  %pkg: include/config/kernel.release FORCE
> >         $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.package $@
> >
> > +# Distribution of the kernel
> > +# ---------------------------------------------------------------------------
> > +
> > +dist%: FORCE
> > +       @if ! test -f $(srctree)/distro/Makefile; then \
> > +               echo 'No distribution targets defined'; \
> > +               exit 1; \
> > +       fi
> > +       $(Q)$(MAKE) -C $(srctree)/distro $@
> > +
> >  # Brief documentation of the typical targets used
> >  # ---------------------------------------------------------------------------
> >
> > @@ -1732,6 +1742,11 @@ help:
> >         @echo  ''
> >         @echo  'Kernel packaging:'
> >         @$(MAKE) -f $(srctree)/scripts/Makefile.package help
> > +       @if test -f $(srctree)/distro/Makefile; then \
> > +               echo ''; \
> > +               echo 'Distro targets:'; \
> > +               $(MAKE) -C $(srctree)/distro dist-help; \
> > +       fi
> >         @echo  ''
> >         @echo  'Documentation targets:'
> >         @$(MAKE) -f $(srctree)/Documentation/Makefile dochelp
> > --
> > 2.38.1
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
Masahiro Yamada Nov. 24, 2022, 12:23 a.m. UTC | #3 
On Thu, Nov 24, 2022 at 1:08 AM Justin Forbes <jforbes@fedoraproject.org> wrote:
>
> On Wed, Nov 23, 2022 at 9:59 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Thu, Nov 24, 2022 at 12:32 AM Don Zickus <dzickus@redhat.com> wrote:
> > >
> > > Fedora adds a directory to its source git tree to provide packaging
> > > information[1] specific for its distro.  We would like to propagate our
> > > 'help' section to the toplevel to be seen by 'make help' as it isn't
> > > obvious to users to use 'make dist-help'[2].
> > >
> > > Instead of keeping Fedora changes local, I am proposing a generic
> > > mechanism for other distros to use if they would like.  The change looks
> > > for a distro directory and leverages that Makefile if it exists.
> > > Otherwise it is ignored.
> > >
> > > [1] - https://gitlab.com/cki-project/kernel-ark/-/tree/os-build/redhat
> > > [2] - https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/redhat/Makefile#L809
> > >
> > > Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
> > > Signed-off-by: Don Zickus <dzickus@redhat.com>
> > >
> > > ----
> > > This patch is more of a conversation starter than anything.
> > >
> > > I think other distros might find this useful and examples of what we would
> > > populate the directory with can be found in [1].
> > >
> > > Thoughts?
> >
> > I do not like to merge any code that is irrelevant to the upstream.
>
>
> While not directly relevant to the upstream tree, it is very relevant
> to the way the tree is being used. More importantly, it allows a
> "distro agnostic" mechanism without requiring distributions or other
> developers/users with their own additional make targets to avoid
> carrying a patch to make it all work.
>
> Justin



This is another contract to downstream code.

Once this is merged, it will be difficult to change it
because the upstream community never knows how it is used
in downstream.


For example,

   $(Q)$(MAKE) -C $(srctree)/distro $@

violates the rule on how Kbuild works
(in the sense of the working directory).


   $(Q)$(MAKE) -f $(srctree)/distro/Makefile $@

is more aligned with how O= build works, but
we cannot _fix_ it later since this changes the working directory.

And, I assume people will come back again, for example,
to insert another hook into "make clean" for cleaning distro artifacts.

So, it is better to not merge code like this.






> >
> > > ---
> > >  Makefile | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 6f846b1f2618f..45fdb18dde46f 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1635,6 +1635,16 @@ distclean: mrproper
> > >  %pkg: include/config/kernel.release FORCE
> > >         $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.package $@
> > >
> > > +# Distribution of the kernel
> > > +# ---------------------------------------------------------------------------
> > > +
> > > +dist%: FORCE
> > > +       @if ! test -f $(srctree)/distro/Makefile; then \
> > > +               echo 'No distribution targets defined'; \
> > > +               exit 1; \
> > > +       fi
> > > +       $(Q)$(MAKE) -C $(srctree)/distro $@
> > > +
> > >  # Brief documentation of the typical targets used
> > >  # ---------------------------------------------------------------------------
> > >
> > > @@ -1732,6 +1742,11 @@ help:
> > >         @echo  ''
> > >         @echo  'Kernel packaging:'
> > >         @$(MAKE) -f $(srctree)/scripts/Makefile.package help
> > > +       @if test -f $(srctree)/distro/Makefile; then \
> > > +               echo ''; \
> > > +               echo 'Distro targets:'; \
> > > +               $(MAKE) -C $(srctree)/distro dist-help; \
> > > +       fi
> > >         @echo  ''
> > >         @echo  'Documentation targets:'
> > >         @$(MAKE) -f $(srctree)/Documentation/Makefile dochelp
> > > --
> > > 2.38.1
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
Nicolas Schier Nov. 24, 2022, 4:59 a.m. UTC | #4 
On Wed 23 Nov 2022 10:32:03 GMT, Don Zickus wrote:
> Fedora adds a directory to its source git tree to provide packaging
> information[1] specific for its distro.  We would like to propagate our
> 'help' section to the toplevel to be seen by 'make help' as it isn't
> obvious to users to use 'make dist-help'[2].
> 
> Instead of keeping Fedora changes local, I am proposing a generic
> mechanism for other distros to use if they would like.  The change looks
> for a distro directory and leverages that Makefile if it exists.
> Otherwise it is ignored.
> 
> [1] - https://gitlab.com/cki-project/kernel-ark/-/tree/os-build/redhat
> [2] - https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/redhat/Makefile#L809
> 
> Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
> Signed-off-by: Don Zickus <dzickus@redhat.com>
> 
> ----
> This patch is more of a conversation starter than anything. 
> 
> I think other distros might find this useful and examples of what we would
> populate the directory with can be found in [1].
> 
> Thoughts?
> 
> ---
>  Makefile | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 6f846b1f2618f..45fdb18dde46f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1635,6 +1635,16 @@ distclean: mrproper
>  %pkg: include/config/kernel.release FORCE
>  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.package $@
>  
> +# Distribution of the kernel
> +# ---------------------------------------------------------------------------
> +
> +dist%: FORCE
> +	@if ! test -f $(srctree)/distro/Makefile; then \
> +		echo 'No distribution targets defined'; \
> +		exit 1; \
> +	fi
> +	$(Q)$(MAKE) -C $(srctree)/distro $@
> +
>  # Brief documentation of the typical targets used
>  # ---------------------------------------------------------------------------
>  
> @@ -1732,6 +1742,11 @@ help:
>  	@echo  ''
>  	@echo  'Kernel packaging:'
>  	@$(MAKE) -f $(srctree)/scripts/Makefile.package help
> +	@if test -f $(srctree)/distro/Makefile; then \
> +		echo ''; \
> +		echo 'Distro targets:'; \
> +		$(MAKE) -C $(srctree)/distro dist-help; \
> +	fi
>  	@echo  ''
>  	@echo  'Documentation targets:'
>  	@$(MAKE) -f $(srctree)/Documentation/Makefile dochelp
> -- 
> 2.38.1

Have you tried to wrap upstream main Makefile?  E.g. you can add a 
GNUmakefile that simply forwards all targets to Makefile.  It could be 
used to add new targets, or to extends one or the other target from 
Makefile.

(Further, you could make use of MAKEFILES variable and force inclusion 
of a makefile that extends the upstream makefiles "on-the-fly", but as 
this is quite fragile, I cannot recommend it.)

Kind regards,
Nicolas
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 6f846b1f2618f..45fdb18dde46f 100644
--- a/Makefile
+++ b/Makefile
@@ -1635,6 +1635,16 @@  distclean: mrproper
 %pkg: include/config/kernel.release FORCE
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.package $@
 
+# Distribution of the kernel
+# ---------------------------------------------------------------------------
+
+dist%: FORCE
+	@if ! test -f $(srctree)/distro/Makefile; then \
+		echo 'No distribution targets defined'; \
+		exit 1; \
+	fi
+	$(Q)$(MAKE) -C $(srctree)/distro $@
+
 # Brief documentation of the typical targets used
 # ---------------------------------------------------------------------------
 
@@ -1732,6 +1742,11 @@  help:
 	@echo  ''
 	@echo  'Kernel packaging:'
 	@$(MAKE) -f $(srctree)/scripts/Makefile.package help
+	@if test -f $(srctree)/distro/Makefile; then \
+		echo ''; \
+		echo 'Distro targets:'; \
+		$(MAKE) -C $(srctree)/distro dist-help; \
+	fi
 	@echo  ''
 	@echo  'Documentation targets:'
 	@$(MAKE) -f $(srctree)/Documentation/Makefile dochelp