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

This reverts commit 94a955f7bac56cb8f524f43a7773038e6e341585, reversing
changes made to 708408a8ad99f942c9cfd40f43ec11b961d31846.

2 files changed, 22 insertions, 158 deletions

diff --git a/Makefile b/Makefile
index 494a66d7d..9ae657240 100644
--- a/Makefile
+++ b/Makefile
@@ -71,11 +71,6 @@ 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
@@ -223,7 +218,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    = $(addprefix ${BUILD_DIR}/bin/,$(notdir $(shell find ${SOURCE_DIR}/cmd -mindepth 1 -maxdepth 1 -type d -print)))
+GITALY_EXECUTABLES    = $(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)
 
@@ -237,6 +232,7 @@ 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}
@@ -255,9 +251,6 @@ 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>...]"
@@ -283,6 +276,24 @@ 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
@@ -502,29 +513,6 @@ ${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
@@ -615,8 +603,9 @@ ${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}GOBIN=${TOOLS_DIR} go install ${TOOL_PACKAGE}@${TOOL_VERSION}
+	${Q}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
deleted file mode 100644
index 0eab44149..000000000
--- a/tools/replace-buildid/main.go
+++ /dev/null
@@ -1,125 +0,0 @@
-// 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
-}