Makefile: Handle unexpected empty var. values

Fixes #14021

Substitution values built from `$(shell ...)` output can easily be empty
due to the shell's default `pipefail` behavior.  This can also hide
non-zero exit codes, similarly resulting in empty values being set.
While not a perfect fix, the situation is improved by using the
`err_if_empty` function in all cases where empty values would be
unexpected.  Remove the definitions for `GIT_BRANCH` and
`GIT_BRANCH_CLEAN` which don't seem to actually be used anywhere
(including in code).

Add a simple release-test to verify `podman info` outputs a non-empty
value for "GitCommit".

Signed-off-by: Chris Evich <cevich@redhat.com>

2 files changed, 21 insertions, 18 deletions

diff --git a/Makefile b/Makefile
index 2995aed65..3810459b0 100644
--- a/Makefile
+++ b/Makefile
@@ -29,8 +29,6 @@ EPOCH_TEST_COMMIT ?= $(shell git merge-base $${DEST_BRANCH:-main} HEAD)
 HEAD ?= HEAD
 PROJECT := github.com/containers/podman
 GIT_BASE_BRANCH ?= origin/main
-GIT_BRANCH ?= $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null)
-GIT_BRANCH_CLEAN ?= $(shell echo $(GIT_BRANCH) | sed -e "s/[^[:alnum:]]/-/g")
 LIBPOD_INSTANCE := libpod_dev
 PREFIX ?= /usr/local
 BINDIR ?= ${PREFIX}/bin
@@ -80,18 +78,18 @@ FISHINSTALLDIR=${PREFIX}/share/fish/vendor_completions.d
 SELINUXOPT ?= $(shell test -x /usr/sbin/selinuxenabled && selinuxenabled && echo -Z)
 
 COMMIT_NO ?= $(shell git rev-parse HEAD 2> /dev/null || true)
-GIT_COMMIT ?= $(if $(shell git status --porcelain --untracked-files=no),${COMMIT_NO}-dirty,${COMMIT_NO})
+GIT_COMMIT ?= $(if $(shell git status --porcelain --untracked-files=no),$(call err_if_empty,COMMIT_NO)-dirty,$(COMMIT_NO))
 DATE_FMT = %s
 ifdef SOURCE_DATE_EPOCH
-	BUILD_INFO ?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" "+$(DATE_FMT)" 2>/dev/null || date -u -r "$(SOURCE_DATE_EPOCH)" "+$(DATE_FMT)" 2>/dev/null || date -u "+$(DATE_FMT)")
+	BUILD_INFO ?= $(shell date -u -d "@$(call err_if_empty,SOURCE_DATE_EPOCH)" "+$(DATE_FMT)" 2>/dev/null || date -u -r "$(SOURCE_DATE_EPOCH)" "+$(DATE_FMT)" 2>/dev/null || date -u "+$(DATE_FMT)")
 else
 	BUILD_INFO ?= $(shell date "+$(DATE_FMT)")
 endif
 LIBPOD := ${PROJECT}/v4/libpod
 GOFLAGS ?= -trimpath
 LDFLAGS_PODMAN ?= \
-	-X $(LIBPOD)/define.gitCommit=$(GIT_COMMIT) \
-	-X $(LIBPOD)/define.buildInfo=$(BUILD_INFO) \
+	$(if $(GIT_COMMIT),-X $(LIBPOD)/define.gitCommit=$(GIT_COMMIT),) \
+	$(if $(BUILD_INFO),-X $(LIBPOD)/define.buildInfo=$(BUILD_INFO),) \
 	-X $(LIBPOD)/config._installPrefix=$(PREFIX) \
 	-X $(LIBPOD)/config._etcDir=$(ETCDIR) \
 	-X github.com/containers/common/pkg/config.additionalHelperBinariesDir=$(HELPER_BINARIES_DIR)\
@@ -107,7 +105,7 @@ GINKGOTIMEOUT ?= -timeout=90m
 
 # Conditional required to produce empty-output if binary not built yet.
 RELEASE_VERSION = $(shell if test -x test/version/version; then test/version/version; fi)
-RELEASE_NUMBER = $(shell echo "$(RELEASE_VERSION)" | sed -e 's/^v\(.*\)/\1/')
+RELEASE_NUMBER = $(shell echo "$(call err_if_empty,RELEASE_VERSION)" | sed -e 's/^v\(.*\)/\1/')
 
 # If non-empty, logs all output from server during remote system testing
 PODMAN_SERVER_LOG ?=
@@ -138,7 +136,7 @@ err_if_empty = $(if $(strip $($(1))),$(strip $($(1))),$(error Required variable
 CGO_ENABLED ?= 1
 # Default to the native OS type and architecture unless otherwise specified
 NATIVE_GOOS := $(shell env -u GOOS $(GO) env GOOS)
-GOOS ?= $(NATIVE_GOOS)
+GOOS ?= $(call err_if_empty,NATIVE_GOOS)
 # Default to the native architecture type
 NATIVE_GOARCH := $(shell env -u GOARCH $(GO) env GOARCH)
 GOARCH ?= $(NATIVE_GOARCH)
@@ -158,7 +156,7 @@ export GOOS GOARCH CGO_ENABLED BINSFX SRCBINDIR
 # Need to use CGO for mDNS resolution, but cross builds need CGO disabled
 # See https://github.com/golang/go/issues/12524 for details
 DARWIN_GCO := 0
-ifeq ($(NATIVE_GOOS),darwin)
+ifeq ($(call err_if_empty,NATIVE_GOOS),darwin)
 ifdef HOMEBREW_PREFIX
 	DARWIN_GCO := 1
 endif
@@ -189,8 +187,8 @@ binaries: podman podman-remote rootlessport ## Build podman, podman-remote and r
 # at reference-time (due to `=` and not `=:`).
 _HLP_TGTS_RX = '^[[:print:]]+:.*?\#\# .*$$'
 _HLP_TGTS_CMD = grep -E $(_HLP_TGTS_RX) $(MAKEFILE_LIST)
-_HLP_TGTS_LEN = $(shell $(_HLP_TGTS_CMD) | cut -d : -f 1 | wc -L)
-_HLPFMT = "%-$(_HLP_TGTS_LEN)s %s\n"
+_HLP_TGTS_LEN = $(shell $(call err_if_empty,_HLP_TGTS_CMD) | cut -d : -f 1 | wc -L)
+_HLPFMT = "%-$(call err_if_empty,_HLP_TGTS_LEN)s %s\n"
 .PHONY: help
 help: ## (Default) Print listing of key targets with their descriptions
 	@printf $(_HLPFMT) "Target:" "Description:"
@@ -250,7 +248,7 @@ validate: lint .gitvalidation validate.completions man-page-check swagger-check
 .PHONY: build-all-new-commits
 build-all-new-commits:
 	# Validate that all the commits build on top of $(GIT_BASE_BRANCH)
-	git rebase $(GIT_BASE_BRANCH) -x "$(MAKE)"
+	git rebase $(call err_if_empty,GIT_BASE_BRANCH) -x "$(MAKE)"
 
 .PHONY: vendor
 vendor:
@@ -441,7 +439,7 @@ docs: $(MANPAGES) ## Generate documentation
 
 # docs/remote-docs.sh requires a locally executable 'podman-remote' binary
 # in addition to the target-archetecture binary (if any).
-podman-remote-%-docs: podman-remote-$(NATIVE_GOOS)
+podman-remote-%-docs: podman-remote-$(call err_if_empty,NATIVE_GOOS)
 	$(eval GOOS := $*)
 	$(MAKE) docs $(MANPAGES)
 	rm -rf docs/build/remote
@@ -639,7 +637,7 @@ podman-release-%.tar.gz: test/version/version
 	$(eval SUBDIR := podman-v$(call err_if_empty,RELEASE_NUMBER))
 	$(eval _DSTARGS := "DESTDIR=$(TMPDIR)/$(SUBDIR)" "PREFIX=/usr")
 	$(eval GOARCH := $*)
-	mkdir -p "$(TMPDIR)/$(SUBDIR)"
+	mkdir -p "$(call err_if_empty,TMPDIR)/$(SUBDIR)"
 	$(MAKE) GOOS=$(GOOS) GOARCH=$(NATIVE_GOARCH) \
 		clean-binaries docs podman-remote-$(GOOS)-docs
 	if [[ "$(GOARCH)" != "$(NATIVE_GOARCH)" ]]; then \
@@ -660,7 +658,7 @@ podman-remote-release-%.zip: test/version/version ## Build podman-remote for %=$
 	$(eval GOOS := $(firstword $(subst _, ,$*)))
 	$(eval GOARCH := $(lastword $(subst _, ,$*)))
 	$(eval _GOPLAT := GOOS=$(call err_if_empty,GOOS) GOARCH=$(call err_if_empty,GOARCH))
-	mkdir -p "$(TMPDIR)/$(SUBDIR)"
+	mkdir -p "$(call err_if_empty,TMPDIR)/$(SUBDIR)"
 	$(MAKE) GOOS=$(GOOS) GOARCH=$(NATIVE_GOARCH) \
 		clean-binaries podman-remote-$(GOOS)-docs
 	if [[ "$(GOARCH)" != "$(NATIVE_GOARCH)" ]]; then \
@@ -679,8 +677,8 @@ podman-remote-release-%.zip: test/version/version ## Build podman-remote for %=$
 
 .PHONY: podman.msi
 podman.msi: test/version/version  ## Build podman-remote, package for installation on Windows
-	$(MAKE) podman-v$(RELEASE_NUMBER).msi
-podman-v$(RELEASE_NUMBER).msi: podman-remote-windows podman-remote-windows-docs podman-winpath win-sshproxy
+	$(MAKE) podman-v$(call err_if_empty,RELEASE_NUMBER).msi
+podman-v%.msi: test/version/version podman-remote-windows podman-remote-windows-docs podman-winpath win-sshproxy
 	$(eval DOCFILE := docs/build/remote/windows)
 	find $(DOCFILE) -print | \
 		wixl-heat --var var.ManSourceDir --component-group ManFiles \
@@ -715,7 +713,7 @@ package:  ## Build rpm packages
 # a full path to test installed podman or you risk to call another executable.
 .PHONY: package-install
 package-install: package  ## Install rpm packages
-	sudo ${PKG_MANAGER} -y install ${HOME}/rpmbuild/RPMS/*/*.rpm
+	sudo $(call err_if_empty,PKG_MANAGER) -y install ${HOME}/rpmbuild/RPMS/*/*.rpm
 	/usr/bin/podman version
 	/usr/bin/podman info  # will catch a broken conmon
 
diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh
index c871f1f54..b9f43f395 100755
--- a/contrib/cirrus/runner.sh
+++ b/contrib/cirrus/runner.sh
@@ -312,6 +312,11 @@ function _run_release() {
     if [[ -n "$dev" ]]; then
         die "Releases must never contain '-dev' in output of 'podman info' ($dev)"
     fi
+
+    commit=$(bin/podman info --format='{{.Version.GitCommit}}' | tr -d '[:space:]')
+    if [[ -z "$commit" ]]; then
+        die "Releases must contain a non-empty Version.GitCommit in 'podman info'"
+    fi
     msg "All OK"
 }