diff mbox

[1/1] package/lua: do not install man files

Message ID 1442876342-10841-1-git-send-email-joerg.krause@embedded.rocks
State Rejected
Headers show

Commit Message

Jörg Krause Sept. 21, 2015, 10:59 p.m. UTC
As written in the holy manual:
http://buildroot.org/downloads/manual/manual.html#faq-no-doc-on-target

Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks>
---
 package/lua/lua.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Petazzoni Sept. 22, 2015, 2:01 p.m. UTC | #1
Hello Jörg,

On Tue, 22 Sep 2015 00:59:02 +0200, Jörg Krause wrote:
> As written in the holy manual:
> http://buildroot.org/downloads/manual/manual.html#faq-no-doc-on-target
> 
> Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks>
> ---
>  package/lua/lua.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/lua/lua.mk b/package/lua/lua.mk
> index 1279b76..af5cef3 100644
> --- a/package/lua/lua.mk
> +++ b/package/lua/lua.mk
> @@ -87,7 +87,7 @@ define HOST_LUA_BUILD_CMDS
>  endef
>  
>  define LUA_INSTALL_STAGING_CMDS
> -	$(MAKE) INSTALL_TOP="$(STAGING_DIR)/usr" -C $(@D) install
> +	$(MAKE) INSTALL_TOP="$(STAGING_DIR)/usr" INSTALL_MAN= -C $(@D) install
>  	$(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \
>  		$(STAGING_DIR)/usr/lib/pkgconfig/lua.pc
>  endef
> @@ -97,7 +97,7 @@ define LUA_INSTALL_TARGET_CMDS
>  endef
>  
>  define HOST_LUA_INSTALL_CMDS
> -	$(MAKE) INSTALL_TOP="$(HOST_DIR)/usr" -C $(@D) install
> +	$(MAKE) INSTALL_TOP="$(HOST_DIR)/usr" INSTALL_MAN= -C $(@D) install
>  	$(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \
>  		$(HOST_DIR)/usr/lib/pkgconfig/lua.pc
>  endef

This is in fact not needed. Buildroot is already removing the man pages
in the target-finalize step of the main Makefile. I have checked, the
Lua man pages are installed in standard locations, so they are properly
removed by this generic removal step.

For this reason, I am not sure it is really worth to add more
complexity to a package to avoid doing something that is anyway already
handled at some later point in a generic way. It would be worth if that
thing was actually taking a significant amount of build time, or
required additional dependencies. But it's not the case here, so i
would be tempted to leave the Lua package as is, and not take this
patch.

Opinions?

Thomas
Jörg Krause Sept. 22, 2015, 6:56 p.m. UTC | #2
On Di, 2015-09-22 at 16:01 +0200, Thomas Petazzoni wrote:
> Hello Jörg,
> 
> On Tue, 22 Sep 2015 00:59:02 +0200, Jörg Krause wrote:
> > As written in the holy manual:
> > http://buildroot.org/downloads/manual/manual.html#faq-no-doc-on-tar
> > get
> > 
> > Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks>
> > ---
> >  package/lua/lua.mk | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/package/lua/lua.mk b/package/lua/lua.mk
> > index 1279b76..af5cef3 100644
> > --- a/package/lua/lua.mk
> > +++ b/package/lua/lua.mk
> > @@ -87,7 +87,7 @@ define HOST_LUA_BUILD_CMDS
> >  endef
> >  
> >  define LUA_INSTALL_STAGING_CMDS
> > -	$(MAKE) INSTALL_TOP="$(STAGING_DIR)/usr" -C $(@D) install
> > +	$(MAKE) INSTALL_TOP="$(STAGING_DIR)/usr" INSTALL_MAN= -C
> > $(@D) install
> >  	$(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \
> >  		$(STAGING_DIR)/usr/lib/pkgconfig/lua.pc
> >  endef
> > @@ -97,7 +97,7 @@ define LUA_INSTALL_TARGET_CMDS
> >  endef
> >  
> >  define HOST_LUA_INSTALL_CMDS
> > -	$(MAKE) INSTALL_TOP="$(HOST_DIR)/usr" -C $(@D) install
> > +	$(MAKE) INSTALL_TOP="$(HOST_DIR)/usr" INSTALL_MAN= -C
> > $(@D) install
> >  	$(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \
> >  		$(HOST_DIR)/usr/lib/pkgconfig/lua.pc
> >  endef
> 
> This is in fact not needed. Buildroot is already removing the man
> pages
> in the target-finalize step of the main Makefile. I have checked, the
> Lua man pages are installed in standard locations, so they are
> properly
> removed by this generic removal step.
> 
> For this reason, I am not sure it is really worth to add more
> complexity to a package to avoid doing something that is anyway
> already
> handled at some later point in a generic way. It would be worth if
> that
> thing was actually taking a significant amount of build time, or
> required additional dependencies. But it's not the case here, so i
> would be tempted to leave the Lua package as is, and not take this
> patch.
> 
> Opinions?
> 

I see! I didn't know about this. I just build the lua package (without
make all) and saw the man files in my target tree.

Many thanks for clarification! I'll mark this patch as rejected...

Jörg
diff mbox

Patch

diff --git a/package/lua/lua.mk b/package/lua/lua.mk
index 1279b76..af5cef3 100644
--- a/package/lua/lua.mk
+++ b/package/lua/lua.mk
@@ -87,7 +87,7 @@  define HOST_LUA_BUILD_CMDS
 endef
 
 define LUA_INSTALL_STAGING_CMDS
-	$(MAKE) INSTALL_TOP="$(STAGING_DIR)/usr" -C $(@D) install
+	$(MAKE) INSTALL_TOP="$(STAGING_DIR)/usr" INSTALL_MAN= -C $(@D) install
 	$(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \
 		$(STAGING_DIR)/usr/lib/pkgconfig/lua.pc
 endef
@@ -97,7 +97,7 @@  define LUA_INSTALL_TARGET_CMDS
 endef
 
 define HOST_LUA_INSTALL_CMDS
-	$(MAKE) INSTALL_TOP="$(HOST_DIR)/usr" -C $(@D) install
+	$(MAKE) INSTALL_TOP="$(HOST_DIR)/usr" INSTALL_MAN= -C $(@D) install
 	$(INSTALL) -m 0644 -D $(@D)/etc/lua.pc \
 		$(HOST_DIR)/usr/lib/pkgconfig/lua.pc
 endef