diff mbox series

[v2,1/4] tools lib subcmd: Add install target

Message ID 20221122001125.765003-2-irogers@google.com
State New
Headers
Series objtool build improvements |

Commit Message

Ian Rogers Nov. 22, 2022, 12:11 a.m. UTC 
  This allows libsubcmd to be installed as a dependency.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/subcmd/Makefile | 49 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

Nicolas Schier Nov. 22, 2022, 6:48 a.m. UTC | #1 
On Mon 21 Nov 2022 16:11:22 GMT, Ian Rogers wrote:
> This allows libsubcmd to be installed as a dependency.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/lib/subcmd/Makefile | 49 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> index 8f1a09cdfd17..e96566f8991c 100644
> --- a/tools/lib/subcmd/Makefile
> +++ b/tools/lib/subcmd/Makefile
> @@ -17,6 +17,15 @@ RM = rm -f
>  
>  MAKEFLAGS += --no-print-directory
>  
> +INSTALL = install
> +
> +# Use DESTDIR for installing into a different root directory.
> +# This is useful for building a package. The program will be
> +# installed in this directory as if it was the root directory.
> +# Then the build tool can move it later.
> +DESTDIR ?=
> +DESTDIR_SQ = '$(subst ','\'',$(DESTDIR))'
> +
>  LIBFILE = $(OUTPUT)libsubcmd.a
>  
>  CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
> @@ -48,6 +57,18 @@ CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
>  
>  SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
>  
> +ifeq ($(LP64), 1)
> +  libdir_relative = lib64
> +else
> +  libdir_relative = lib
> +endif
> +
> +prefix ?=
> +libdir = $(prefix)/$(libdir_relative)
> +
> +# Shell quotes
> +libdir_SQ = $(subst ','\'',$(libdir))
> +
>  all:
>  
>  export srctree OUTPUT CC LD CFLAGS V
> @@ -61,6 +82,34 @@ $(SUBCMD_IN): FORCE
>  $(LIBFILE): $(SUBCMD_IN)
>  	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(SUBCMD_IN)
>  
> +define do_install_mkdir
> +	if [ ! -d '$(DESTDIR_SQ)$1' ]; then             \
> +		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \
> +	fi
> +endef
> +
> +define do_install
> +	if [ ! -d '$(DESTDIR_SQ)$2' ]; then             \
> +		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \
> +	fi;                                             \
> +	$(INSTALL) $1 $(if $3,-m $3,) '$(DESTDIR_SQ)$2'
> +endef
> +
> +install_lib: $(LIBFILE)
> +	$(call QUIET_INSTALL, $(LIBFILE)) \
> +		$(call do_install_mkdir,$(libdir_SQ)); \
> +		cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
> +

Sorry for being late.

Is there a specific reason why you do not use 'mkdir -p 
$(DESTDIR)/$(2); cp $(1) $(2)' or 'install $(addprefix -m,$(3)) -D $(1) 
$(2)' for the install rules (cp.  scripts/Makefile.{dtb,mod}inst)?

I think you could get rid of mkdir calls and the ..._SQ handling when 
using one of them.

> +install_headers:
> +	$(call QUIET_INSTALL, headers) \

Unlikely, but if one of the install commands fails, you probably want 
make to exit with an error.  Might you want to add 'set -e; \' here?

Kind regards,
Nicolas


> +		$(call do_install,exec-cmd.h,$(prefix)/include/subcmd,644); \
> +		$(call do_install,help.h,$(prefix)/include/subcmd,644); \
> +		$(call do_install,pager.h,$(prefix)/include/subcmd,644); \
> +		$(call do_install,parse-options.h,$(prefix)/include/subcmd,644); \
> +		$(call do_install,run-command.h,$(prefix)/include/subcmd,644);
> +
> +install: install_lib install_headers
> +
>  clean:
>  	$(call QUIET_CLEAN, libsubcmd) $(RM) $(LIBFILE); \
>  	find $(or $(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
> -- 
> 2.38.1.584.g0f3c55d4c2-goog
Ian Rogers Nov. 22, 2022, 6:19 p.m. UTC | #2 
On Mon, Nov 21, 2022 at 10:48 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On Mon 21 Nov 2022 16:11:22 GMT, Ian Rogers wrote:
> > This allows libsubcmd to be installed as a dependency.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/lib/subcmd/Makefile | 49 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >
> > diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> > index 8f1a09cdfd17..e96566f8991c 100644
> > --- a/tools/lib/subcmd/Makefile
> > +++ b/tools/lib/subcmd/Makefile
> > @@ -17,6 +17,15 @@ RM = rm -f
> >
> >  MAKEFLAGS += --no-print-directory
> >
> > +INSTALL = install
> > +
> > +# Use DESTDIR for installing into a different root directory.
> > +# This is useful for building a package. The program will be
> > +# installed in this directory as if it was the root directory.
> > +# Then the build tool can move it later.
> > +DESTDIR ?=
> > +DESTDIR_SQ = '$(subst ','\'',$(DESTDIR))'
> > +
> >  LIBFILE = $(OUTPUT)libsubcmd.a
> >
> >  CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
> > @@ -48,6 +57,18 @@ CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
> >
> >  SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
> >
> > +ifeq ($(LP64), 1)
> > +  libdir_relative = lib64
> > +else
> > +  libdir_relative = lib
> > +endif
> > +
> > +prefix ?=
> > +libdir = $(prefix)/$(libdir_relative)
> > +
> > +# Shell quotes
> > +libdir_SQ = $(subst ','\'',$(libdir))
> > +
> >  all:
> >
> >  export srctree OUTPUT CC LD CFLAGS V
> > @@ -61,6 +82,34 @@ $(SUBCMD_IN): FORCE
> >  $(LIBFILE): $(SUBCMD_IN)
> >       $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(SUBCMD_IN)
> >
> > +define do_install_mkdir
> > +     if [ ! -d '$(DESTDIR_SQ)$1' ]; then             \
> > +             $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \
> > +     fi
> > +endef
> > +
> > +define do_install
> > +     if [ ! -d '$(DESTDIR_SQ)$2' ]; then             \
> > +             $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \
> > +     fi;                                             \
> > +     $(INSTALL) $1 $(if $3,-m $3,) '$(DESTDIR_SQ)$2'
> > +endef
> > +
> > +install_lib: $(LIBFILE)
> > +     $(call QUIET_INSTALL, $(LIBFILE)) \
> > +             $(call do_install_mkdir,$(libdir_SQ)); \
> > +             cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
> > +
>
> Sorry for being late.
>
> Is there a specific reason why you do not use 'mkdir -p
> $(DESTDIR)/$(2); cp $(1) $(2)' or 'install $(addprefix -m,$(3)) -D $(1)
> $(2)' for the install rules (cp.  scripts/Makefile.{dtb,mod}inst)?
>
> I think you could get rid of mkdir calls and the ..._SQ handling when
> using one of them.

Thanks for the feedback! Perhaps we can merge this as being consistent
with existing approaches:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/perf/Makefile#n185
and then refactor as you say.

> > +install_headers:
> > +     $(call QUIET_INSTALL, headers) \
>
> Unlikely, but if one of the install commands fails, you probably want
> make to exit with an error.  Might you want to add 'set -e; \' here?

Possibly. Again, I think we should aim for consistency and do this as
a follow up.

Thanks,
Ian

> Kind regards,
> Nicolas
>
>
> > +             $(call do_install,exec-cmd.h,$(prefix)/include/subcmd,644); \
> > +             $(call do_install,help.h,$(prefix)/include/subcmd,644); \
> > +             $(call do_install,pager.h,$(prefix)/include/subcmd,644); \
> > +             $(call do_install,parse-options.h,$(prefix)/include/subcmd,644); \
> > +             $(call do_install,run-command.h,$(prefix)/include/subcmd,644);
> > +
> > +install: install_lib install_headers
> > +
> >  clean:
> >       $(call QUIET_CLEAN, libsubcmd) $(RM) $(LIBFILE); \
> >       find $(or $(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
> > --
> > 2.38.1.584.g0f3c55d4c2-goog
>
> --
> epost|xmpp: nicolas@fjasle.eu          irc://oftc.net/nsc
> ↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
>      -- frykten for herren er opphav til kunnskap --
diff mbox series

Patch

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 8f1a09cdfd17..e96566f8991c 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -17,6 +17,15 @@  RM = rm -f
 
 MAKEFLAGS += --no-print-directory
 
+INSTALL = install
+
+# Use DESTDIR for installing into a different root directory.
+# This is useful for building a package. The program will be
+# installed in this directory as if it was the root directory.
+# Then the build tool can move it later.
+DESTDIR ?=
+DESTDIR_SQ = '$(subst ','\'',$(DESTDIR))'
+
 LIBFILE = $(OUTPUT)libsubcmd.a
 
 CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
@@ -48,6 +57,18 @@  CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
 
 SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
 
+ifeq ($(LP64), 1)
+  libdir_relative = lib64
+else
+  libdir_relative = lib
+endif
+
+prefix ?=
+libdir = $(prefix)/$(libdir_relative)
+
+# Shell quotes
+libdir_SQ = $(subst ','\'',$(libdir))
+
 all:
 
 export srctree OUTPUT CC LD CFLAGS V
@@ -61,6 +82,34 @@  $(SUBCMD_IN): FORCE
 $(LIBFILE): $(SUBCMD_IN)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(SUBCMD_IN)
 
+define do_install_mkdir
+	if [ ! -d '$(DESTDIR_SQ)$1' ]; then             \
+		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \
+	fi
+endef
+
+define do_install
+	if [ ! -d '$(DESTDIR_SQ)$2' ]; then             \
+		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \
+	fi;                                             \
+	$(INSTALL) $1 $(if $3,-m $3,) '$(DESTDIR_SQ)$2'
+endef
+
+install_lib: $(LIBFILE)
+	$(call QUIET_INSTALL, $(LIBFILE)) \
+		$(call do_install_mkdir,$(libdir_SQ)); \
+		cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
+
+install_headers:
+	$(call QUIET_INSTALL, headers) \
+		$(call do_install,exec-cmd.h,$(prefix)/include/subcmd,644); \
+		$(call do_install,help.h,$(prefix)/include/subcmd,644); \
+		$(call do_install,pager.h,$(prefix)/include/subcmd,644); \
+		$(call do_install,parse-options.h,$(prefix)/include/subcmd,644); \
+		$(call do_install,run-command.h,$(prefix)/include/subcmd,644);
+
+install: install_lib install_headers
+
 clean:
 	$(call QUIET_CLEAN, libsubcmd) $(RM) $(LIBFILE); \
 	find $(or $(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)