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

[Nicolas Schier]

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@xxxxxxxxxx>
> ---
>  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

-- 
epost|xmpp: nicolas@xxxxxxxxx          irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
     -- frykten for herren er opphav til kunnskap --

Attachment: signature.asc
Description: PGP signature