All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zdenek Kabelac <zkabelac@sourceware.org>
To: lvm-devel@redhat.com
Subject: main - makefiles: avoid piping
Date: Fri, 10 Feb 2023 16:53:29 +0000 (GMT)	[thread overview]
Message-ID: <20230210165329.811C13858C5F@sourceware.org> (raw)

Gitweb:        https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=a0437225aa799ed4685e60534fcbdc867fd26947
Commit:        a0437225aa799ed4685e60534fcbdc867fd26947
Parent:        a032e07578ecdb459470cced8e118ce57088d033
Author:        Zdenek Kabelac <zkabelac@redhat.com>
AuthorDate:    Thu Feb 9 22:33:31 2023 +0100
Committer:     Zdenek Kabelac <zkabelac@redhat.com>
CommitterDate: Fri Feb 10 17:50:27 2023 +0100

makefiles: avoid piping

Addressing problem for user of system without default bash shell.

As reported "set -o pipefail" is a bashism that causes the build to
fail when /bin/sh does not point to bash.
For example, this has been observed causing build failures
on Gentoo when /bin/sh is symlinked to /bin/dash.

Rule has been reworked and we started to use .DELETE_ON_ERROR to
ensure that with any errors during file generation, such invalid
file is automatically removed.

Rules were reworked to avoid using pipe and instead use temporary
files when necessary.
Printing lines with echo was replaced with POSIX recomended 'printf'
as proposed by reporter since handling of escape characters and
the "-n" flag for "echo" aren't portable across shells.

Also some build deps has been added.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1822280
Gentoo-bug: https://bugs.gentoo.org/682404
Gentoo-bug: https://bugs.gentoo.org/716496
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1822280

Reported-by: Michael Orlitzky <michael@orlitzky.com>

TODO: investage if the temporary files could be handled via some
intermediate target solution - ATM I couldn't make it work equally
well as current solution use shell 'trap' to remove temp file.
---
 include/Makefile.in | 16 ++++++++--------
 tools/Makefile.in   | 24 +++++++++++++-----------
 tools/license.inc   |  2 +-
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/include/Makefile.in b/include/Makefile.in
index fc184ed09..60b93f495 100644
--- a/include/Makefile.in
+++ b/include/Makefile.in
@@ -1,6 +1,6 @@
 #
 # Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
-# Copyright (C) 2004-2018 Red Hat, Inc. All rights reserved.
+# Copyright (C) 2004-2023 Red Hat, Inc. All rights reserved.
 #
 # This file is part of LVM2.
 #
@@ -18,23 +18,23 @@ top_builddir = @top_builddir@
 
 include $(top_builddir)/make.tmpl
 
-cmds.h:
+.DELETE_ON_ERROR:
+cmds.h: $(top_srcdir)/tools/command-lines.in $(top_srcdir)/tools/license.inc Makefile
 	@echo "    [GEN] $@"
-	$(Q) set -o pipefail && \
+	$(Q) \
 	( cat $(top_srcdir)/tools/license.inc && \
 	  echo "/* Do not edit. This file is generated by the Makefile. */" && \
 	  echo "cmd(CMD_NONE, none)" && \
-	  $(GREP) '^ID:' $(top_srcdir)/tools/command-lines.in | LC_ALL=C $(SORT) -u | $(AWK) '{print "cmd(" $$2 "_CMD, " $$2 ")"}' && \
+	  trap "$(RM) $@-t" EXIT INT QUIT TERM && \
+	  $(AWK) '/^ID:/ {print "cmd(" $$2 "_CMD, " $$2 ")"}' $< >$@-t && \
+	  LC_ALL=C $(SORT) -u $@-t && \
 	  echo "cmd(CMD_COUNT, count)" \
 	) > $@
 
 all: cmds.h
 
-clean:
-	rm -f cmds.h
-
 DISTCLEAN_TARGETS += configure.h lvm-version.h
-CLEAN_TARGETS += \
+CLEAN_TARGETS += cmds.h cmds.h-t \
  .symlinks \
  .symlinks_created \
  activate.h \
diff --git a/tools/Makefile.in b/tools/Makefile.in
index 4e196ab1e..2a663a5b6 100644
--- a/tools/Makefile.in
+++ b/tools/Makefile.in
@@ -1,6 +1,6 @@
 #
 # Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
-# Copyright (C) 2004-2012 Red Hat, Inc. All rights reserved.
+# Copyright (C) 2004-2023 Red Hat, Inc. All rights reserved.
 #
 # This file is part of LVM2.
 #
@@ -166,23 +166,25 @@ liblvm2cmd.$(LIB_SUFFIX).$(LIB_VERSION): liblvm2cmd.$(LIB_SUFFIX)
 	$(Q) $(CC) -E -P $< 2> /dev/null | \
 		$(EGREP) -v '^ *(|#.*|config|devtypes|dumpconfig|formats|fullreport|help|lastlog|lvmchange|lvpoll|pvdata|segtypes|systemid|tags|version) *$$' > .commands
 
-command-count.h: $(srcdir)/command-lines.in Makefile
+.DELETE_ON_ERROR:
+command-count.h: $(srcdir)/command-lines.in $(srcdir)/license.inc Makefile
 	@echo "    [GEN] $@"
-	$(Q) set -o pipefail && \
-	( cat $(top_srcdir)/tools/license.inc && \
+	$(Q) \
+	( cat $(srcdir)/license.inc && \
 	  echo "/* Do not edit. This file is generated by the Makefile. */" && \
-	  echo -n "#define COMMAND_COUNT " && \
-	  $(GREP) '^ID:' $< | $(WC) -l \
+	  printf "#define COMMAND_COUNT " && \
+	  $(GREP) -c '^ID:' $< \
 	) > $@
 
-command-lines-input.h: $(srcdir)/command-lines.in Makefile
+.DELETE_ON_ERROR:
+command-lines-input.h: $(srcdir)/command-lines.in $(srcdir)/license.inc Makefile
 	@echo "    [GEN] $@"
-	$(Q) set -o pipefail && \
+	$(Q) \
 	( cat $(srcdir)/license.inc && \
 	  echo "/* Do not edit. This file is generated by the Makefile. */" && \
-	  echo -en "static const char _command_input[] =\n\n\"" && \
-	  $(EGREP) -v '^#|\-\-\-|^$$' $(srcdir)/command-lines.in | $(AWK) 'BEGIN {ORS = "\\n\"\n\""} //' && \
-	  echo "\\n\\n\";" \
+	  printf "static const char _command_input[] =\n\n\"" && \
+	  $(AWK) 'BEGIN {ORS = "\\n\"\n\""} !/^#/ && !/---/ && !/^$$/' $(srcdir)/command-lines.in && \
+	  printf '\\n\\n";\n' \
 	) > $@
 
 $(SOURCES:%.c=%.d) $(SOURCES2:%.c=%.d): command-lines-input.h command-count.h
diff --git a/tools/license.inc b/tools/license.inc
index 5ba3e66d4..58e2441d8 100644
--- a/tools/license.inc
+++ b/tools/license.inc
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.  
- * Copyright (C) 2004-2017 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2023 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
  *


                 reply	other threads:[~2023-02-10 16:53 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230210165329.811C13858C5F@sourceware.org \
    --to=zkabelac@sourceware.org \
    --cc=lvm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.