Merge branch 'pks-makefile-workaround-build-id-rebuilding' into 'master'

Makefile: Fix rebuilding Go binaries in place to add GNU build ID

See merge request gitlab-org/gitaly!4544

2 files changed, 158 insertions, 22 deletions

diff --git a/Makefile b/Makefile
index 9ae657240..494a66d7d 100644
--- a/Makefile
+++ b/Makefile
@@ -71,6 +71,11 @@ GO_LDFLAGS        := -X ${GITALY_PACKAGE}/internal/version.version=${GITALY_VERS
 SERVER_BUILD_TAGS := tracer_static,tracer_static_jaeger,tracer_static_stackdriver,continuous_profiler_stackdriver
 GIT2GO_BUILD_TAGS := static,system_libgit2
 
+# Temporary GNU build ID used as a placeholder value so that we can replace it
+# with our own one after binaries have been built. This is the ASCII encoding
+# of the string "TEMP_GITALY_BUILD_ID".
+TEMPORARY_BUILD_ID := 54454D505F474954414C595F4255494C445F4944
+
 ifeq (${FIPS_MODE}, 1)
     SERVER_BUILD_TAGS := ${SERVER_BUILD_TAGS},fips
     GIT2GO_BUILD_TAGS := ${GIT2GO_BUILD_TAGS},fips
@@ -218,7 +223,7 @@ TEST_REPO_GIT    := ${TEST_REPO_DIR}/gitlab-git-test.git
 BENCHMARK_REPO   := ${TEST_REPO_DIR}/benchmark.git
 
 # All executables provided by Gitaly
-GITALY_EXECUTABLES    = $(notdir $(shell find ${SOURCE_DIR}/cmd -mindepth 1 -maxdepth 1 -type d -print))
+GITALY_EXECUTABLES    = $(addprefix ${BUILD_DIR}/bin/,$(notdir $(shell find ${SOURCE_DIR}/cmd -mindepth 1 -maxdepth 1 -type d -print)))
 # Find all Go source files.
 find_go_sources       = $(shell find ${SOURCE_DIR} -type d \( -name ruby -o -name vendor -o -name testdata -o -name '_*' -o -path '*/proto/go/gitalypb' \) -prune -o -type f -name '*.go' -not -name '*.pb.go' -print | sort -u)
 
@@ -232,7 +237,6 @@ run_go_tests = PATH='${SOURCE_DIR}/internal/testhelper/testdata/home/bin:${PATH}
     ${GOTESTSUM} --format ${TEST_FORMAT} --junitfile ${TEST_REPORT} -- -ldflags '${GO_LDFLAGS}' -tags '${SERVER_BUILD_TAGS},${GIT2GO_BUILD_TAGS}' ${TEST_OPTIONS} ${TEST_PACKAGES}
 
 unexport GOROOT
-export GOBIN                      = ${BUILD_DIR}/bin
 export GOCACHE                   ?= ${BUILD_DIR}/cache
 export GOPROXY                   ?= https://proxy.golang.org
 export PATH                      := ${BUILD_DIR}/bin:${PATH}
@@ -251,6 +255,9 @@ export CGO_LDFLAGS_ALLOW          = -D_THREAD_SAFE
 ## Default target which builds Gitaly.
 all: build
 
+.PHONY: .FORCE
+.FORCE:
+
 ## Print help about available targets and variables.
 help:
 	@echo "usage: make [<target>...] [<variable>=<value>...]"
@@ -276,24 +283,6 @@ help:
 ## Build Go binaries and install required Ruby Gems.
 build: ${SOURCE_DIR}/.ruby-bundle ${GITALY_EXECUTABLES}
 
-gitaly:            GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
-praefect:          GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
-gitaly-git2go-v14: GO_BUILD_TAGS = ${GIT2GO_BUILD_TAGS}
-gitaly-git2go-v14: libgit2
-
-.PHONY: ${GITALY_EXECUTABLES}
-${GITALY_EXECUTABLES}:
-	${Q}go install -ldflags '${GO_LDFLAGS}' -tags "${GO_BUILD_TAGS}" $(addprefix ${GITALY_PACKAGE}/cmd/, $@)
-	@ # To compute a unique and deterministic value for GNU build-id, we build the Go binary a second time.
-	@ # From the first build, we extract its unique and deterministic Go build-id, and use that to derive
-	@ # comparably unique and deterministic GNU build-id to inject into the final binary.
-	@ # If we cannot extract a Go build-id, we punt and fallback to using a random 32-byte hex string.
-	@ # This fallback is unique but non-deterministic, making it sufficient to avoid generating the
-	@ # GNU build-id from the empty string and causing guaranteed collisions.
-	${Q}GO_BUILD_ID=$$( go tool buildid $(addprefix ${BUILD_DIR}/bin/, $@) || openssl rand -hex 32 ) && \
-	GNU_BUILD_ID=$$( echo $$GO_BUILD_ID | sha1sum | cut -d' ' -f1 ) && \
-	go install -ldflags '${GO_LDFLAGS}'" -B 0x$$GNU_BUILD_ID" -tags "${GO_BUILD_TAGS}" $(addprefix ${GITALY_PACKAGE}/cmd/, $@)
-
 .PHONY: install
 ## Install Gitaly binaries. The target directory can be modified by setting PREFIX and DESTDIR.
 install: build
@@ -513,6 +502,29 @@ ${TOOLS_DIR}: | ${BUILD_DIR}
 ${DEPENDENCY_DIR}: | ${BUILD_DIR}
 	${Q}mkdir -p ${DEPENDENCY_DIR}
 
+${BUILD_DIR}/bin/%: ${BUILD_DIR}/intermediate/% | ${BUILD_DIR}/bin
+	@ # To compute a unique and deterministic value for GNU build-id, we use an
+	@ # intermediate binary which has a fixed build ID of "TEMP_GITALY_BUILD_ID",
+	@ # which we replace with a deterministic build ID derived from the Go build ID.
+	@ # If we cannot extract a Go build-id, we punt and fallback to using a random 32-byte hex string.
+	@ # This fallback is unique but non-deterministic, making it sufficient to avoid generating the
+	@ # GNU build-id from the empty string and causing guaranteed collisions.
+	${Q}GO_BUILD_ID=$$(go tool buildid "$<" || openssl rand -hex 32) && \
+	GNU_BUILD_ID=$$(echo $$GO_BUILD_ID | sha1sum | cut -d' ' -f1) && \
+	go run "${SOURCE_DIR}"/tools/replace-buildid \
+		-input "$<" -input-build-id "${TEMPORARY_BUILD_ID}" \
+		-output "$@" -output-build-id "$$GNU_BUILD_ID"
+
+${BUILD_DIR}/intermediate/gitaly:            GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
+${BUILD_DIR}/intermediate/praefect:          GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
+${BUILD_DIR}/intermediate/gitaly-git2go-v14: GO_BUILD_TAGS = ${GIT2GO_BUILD_TAGS}
+${BUILD_DIR}/intermediate/gitaly-git2go-v14: libgit2
+${BUILD_DIR}/intermediate/%: .FORCE
+	@ # We're building intermediate binaries first which contain a fixed build ID
+	@ # of "TEMP_GITALY_BUILD_ID". In the final binary we replace this build ID with
+	@ # the computed build ID for this binary.
+	${Q}go build -o "$@" -ldflags '-B 0x${TEMPORARY_BUILD_ID} ${GO_LDFLAGS}' -tags "${GO_BUILD_TAGS}" $(addprefix ${SOURCE_DIR}/cmd/,$(@F))
+
 # This is a build hack to avoid excessive rebuilding of targets. Instead of
 # depending on the Makefile, we start to depend on tool versions as defined in
 # the Makefile. Like this, we only rebuild if the tool versions actually
@@ -603,9 +615,8 @@ ${PROTOC}: ${DEPENDENCY_DIR}/protoc.version | ${TOOLS_DIR}
 	${Q}cmake --build "${PROTOC_BUILD_DIR}" --target install -- -j $(shell nproc)
 	${Q}cp "${PROTOC_INSTALL_DIR}"/bin/protoc ${PROTOC}
 
-${TOOLS_DIR}/%: GOBIN = ${TOOLS_DIR}
 ${TOOLS_DIR}/%: ${TOOLS_DIR}/%.version
-	${Q}go install ${TOOL_PACKAGE}@${TOOL_VERSION}
+	${Q}GOBIN=${TOOLS_DIR} go install ${TOOL_PACKAGE}@${TOOL_VERSION}
 
 ${PROTOC_GEN_GITALY_LINT}: proto | ${TOOLS_DIR}
 	${Q}go build -o $@ ${SOURCE_DIR}/tools/protoc-gen-gitaly-lint
diff --git a/tools/replace-buildid/main.go b/tools/replace-buildid/main.go
new file mode 100644
index 000000000..0eab44149
--- /dev/null
+++ b/tools/replace-buildid/main.go
@@ -0,0 +1,125 @@
+// The `replace-buildid` tool is used to replace a build ID in an ELF binary with a new build ID.
+// Note that this tool is extremely naive: it simply takes the old input ID as string, verifies
+// that this ID is contained in the binary exactly once, and then replaces it. It has no knowledge
+// about ELF binaries whatsoever.
+//
+// This tool is mainly used to replace our static GNU build ID we set in our Makefile with a
+// derived build ID without having to build binaries twice.
+
+package main
+
+import (
+	"bytes"
+	"encoding/hex"
+	"flag"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+)
+
+func main() {
+	var inputPath, outputPath, inputBuildID, outputBuildID string
+
+	flag.StringVar(&inputPath, "input", "", "path to the binary whose GNU build ID should be replaced")
+	flag.StringVar(&outputPath, "output", "", "path whether the resulting binary should be placed")
+	flag.StringVar(&inputBuildID, "input-build-id", "", "static build ID to replace")
+	flag.StringVar(&outputBuildID, "output-build-id", "", "new build ID to replace old value with")
+	flag.Parse()
+
+	if err := replaceBuildID(inputPath, outputPath, inputBuildID, outputBuildID); err != nil {
+		fmt.Fprintf(os.Stderr, "%s\n", err)
+		os.Exit(1)
+	}
+}
+
+func replaceBuildID(inputPath, outputPath, inputBuildID, outputBuildID string) error {
+	if inputPath == "" {
+		return fmt.Errorf("missing input path")
+	}
+	if outputPath == "" {
+		return fmt.Errorf("missing output path")
+	}
+	if inputBuildID == "" {
+		return fmt.Errorf("missing output path")
+	}
+	if outputBuildID == "" {
+		return fmt.Errorf("missing output path")
+	}
+	if flag.NArg() > 0 {
+		return fmt.Errorf("extra arguments")
+	}
+
+	inputBuildIDDecoded, err := hex.DecodeString(inputBuildID)
+	if err != nil {
+		return fmt.Errorf("decoding input build ID: %w", err)
+	}
+
+	outputBuildIDDecoded, err := hex.DecodeString(outputBuildID)
+	if err != nil {
+		return fmt.Errorf("decoding output build ID: %w", err)
+	}
+
+	if len(inputBuildIDDecoded) != len(outputBuildIDDecoded) {
+		return fmt.Errorf("input and output build IDs do not have the same length")
+	}
+
+	data, err := readAndReplace(inputPath, inputBuildIDDecoded, outputBuildIDDecoded)
+	if err != nil {
+		return fmt.Errorf("could not replace build ID: %w", err)
+	}
+
+	if err := writeBinary(outputPath, data); err != nil {
+		return fmt.Errorf("writing binary: %w", err)
+	}
+
+	return nil
+}
+
+func readAndReplace(binaryPath string, inputBuildID, outputBuildID []byte) ([]byte, error) {
+	inputFile, err := os.Open(binaryPath)
+	if err != nil {
+		return nil, fmt.Errorf("opening input file: %w", err)
+	}
+	defer inputFile.Close()
+
+	data, err := io.ReadAll(inputFile)
+	if err != nil {
+		return nil, fmt.Errorf("reading input file: %w", err)
+	}
+
+	if occurrences := bytes.Count(data, inputBuildID); occurrences != 1 {
+		return nil, fmt.Errorf("exactly one match for old build ID expected, got %d", occurrences)
+	}
+
+	return bytes.ReplaceAll(data, inputBuildID, outputBuildID), nil
+}
+
+func writeBinary(binaryPath string, contents []byte) error {
+	f, err := os.CreateTemp(filepath.Dir(binaryPath), filepath.Base(binaryPath))
+	if err != nil {
+		return fmt.Errorf("could not create binary: %w", err)
+	}
+	defer func() {
+		_ = os.RemoveAll(f.Name())
+		f.Close()
+	}()
+
+	if err := f.Chmod(0o755); err != nil {
+		return fmt.Errorf("could not change permissions: %w", err)
+	}
+
+	if _, err := io.Copy(f, bytes.NewReader(contents)); err != nil {
+		return fmt.Errorf("could not write binary: %w", err)
+	}
+
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("could not close binary: %w", err)
+	}
+
+	if err := os.Rename(f.Name(), binaryPath); err != nil {
+		return fmt.Errorf("could not move binary into place: %w", err)
+	}
+
+	return nil
+}