Repository: go-delve/delve

Issue:
Build ID uses version.go blob hash and not built commit hash
When users install Delve using `go install github.com/go-delve/delve/cmd/dlv@latest`, the build ID is always is the blob hash of `pkg/version/version.go` given $Id$ expands to be the blob hash of the file it resides (see https://git-scm.com/docs/gitattributes). Generally build identifiers are the git commit hash that the binary is built/installed with, not the hash of a file's contents. Should the Build id in Version structure not be the git commit hash or should it actually be the blob hash of `pkg/version/version.go`

Discussion:

Solution patch:
diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go
index c7c8ee64..a95c8085 100644
--- a/pkg/proc/gdbserial/gdbserver.go
+++ b/pkg/proc/gdbserial/gdbserver.go
@@ -200,6 +200,7 @@ func (err *ErrBackendUnavailable) Error() string {
 type gdbRegisters struct {
 	regs     map[string]gdbRegister
 	regsInfo []gdbRegisterInfo
+	loaded   []bool
 	tls      uint64
 	gaddr    uint64
 	hasgaddr bool
@@ -1667,6 +1668,7 @@ func (regs *gdbRegisters) init(regsInfo []gdbRegisterInfo, arch *proc.Arch, regn
 	regs.regnames = regnames
 	regs.regs = make(map[string]gdbRegister)
 	regs.regsInfo = regsInfo
+	regs.loaded = make([]bool, len(regsInfo))
 
 	regsz := 0
 	for _, reginfo := range regsInfo {
@@ -1702,20 +1704,27 @@ func (t *gdbThread) reloadRegisters(regs map[uint64]uint64) error {
 				} else {
 					return err
 				}
+			} else {
+				for i := range t.regs.loaded {
+					t.regs.loaded[i] = true
+				}
 			}
 		}
 		if !t.p.gcmdok {
-			for _, reginfo := range t.p.conn.regsInfo {
+			for i, reginfo := range t.p.conn.regsInfo {
+				t.regs.loaded[i] = true
 				if err := t.p.conn.readRegister(t.strID, reginfo.Regnum, t.regs.regs[reginfo.Name].value); err != nil {
 					logflags.DebuggerLogger().Errorf("Could not read register %s: %v\n", reginfo.Name, err)
 					for i := range t.regs.regs[reginfo.Name].value {
 						t.regs.regs[reginfo.Name].value[i] = 0
 					}
+					t.regs.loaded[i] = false
 				}
 			}
 		}
 	} else {
-		for _, r := range t.p.conn.regsInfo {
+		for i, r := range t.p.conn.regsInfo {
+			t.regs.loaded[i] = true
 			if val, ok := regs[uint64(r.Regnum)]; ok {
 				switch r.Bitsize / 8 {
 				case 8:
@@ -1729,6 +1738,7 @@ func (t *gdbThread) reloadRegisters(regs map[uint64]uint64) error {
 					for i := range t.regs.regs[r.Name].value {
 						t.regs.regs[r.Name].value[i] = 0
 					}
+					t.regs.loaded[i] = false
 				}
 			}
 		}
@@ -1781,8 +1791,9 @@ func (t *gdbThread) writeRegisters() error {
 			return err
 		}
 	}
-	for _, r := range t.regs.regs {
-		if r.ignoreOnWrite {
+	for i, reginfo := range t.regs.regsInfo {
+		r := t.regs.regs[reginfo.Name]
+		if r.ignoreOnWrite || !t.regs.loaded[i] {
 			continue
 		}
 		if err := t.p.conn.writeRegister(t.strID, r.regnum, r.value); err != nil {
@@ -2199,6 +2210,7 @@ func (regs *gdbRegisters) Copy() (proc.Registers, error) {
 	savedRegs := &gdbRegisters{}
 	savedRegs.init(regs.regsInfo, regs.arch, regs.regnames)
 	copy(savedRegs.buf, regs.buf)
+	copy(savedRegs.loaded, regs.loaded)
 	return savedRegs, nil
 }
 
diff --git a/pkg/version/fixbuild.go b/pkg/version/fixbuild.go
index 7ddb22b6..8c67d1c6 100644
--- a/pkg/version/fixbuild.go
+++ b/pkg/version/fixbuild.go
@@ -2,21 +2,38 @@
 
 package version
 
-import "runtime/debug"
+import (
+	"runtime/debug"
+	"strings"
+)
 
 func init() {
 	fixBuild = buildInfoFixBuild
 }
 
 func buildInfoFixBuild(v *Version) {
+	// Return if v.Build already set, but not if it is Git ident expand file blob hash
+	if !strings.HasPrefix(v.Build, "$Id: ") && !strings.HasSuffix(v.Build, " $") {
+		return
+	}
+
 	info, ok := debug.ReadBuildInfo()
 	if !ok {
 		return
 	}
-	for i := range info.Settings {
-		if info.Settings[i].Key == "gitrevision" {
-			v.Build = info.Settings[i].Value
-			break
+
+	for _, setting := range info.Settings {
+		if setting.Key == "vcs.revision" {
+			v.Build = setting.Value
+			return
+		}
+	}
+
+	// If we didn't find vcs.revision, try the old key for backward compatibility
+	for _, setting := range info.Settings {
+		if setting.Key == "gitrevision" {
+			v.Build = setting.Value
+			return
 		}
 	}
 }



Test patch:
diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go
index b6c03a14..00be387a 100644
--- a/pkg/terminal/command_test.go
+++ b/pkg/terminal/command_test.go
@@ -1593,7 +1593,7 @@ func TestBreakPointFailWithCond(t *testing.T) {
 		assertNoError(t, term.client.FollowExec(true, ""), "FollowExec")
 		_, err := term.Exec("break spawnchild.go:11 if i == 1")
 		if err != nil {
-			t.Errorf("expect to set a suspened breakpoint")
+			t.Fatalf("expect to set a suspended breakpoint: %v", err)
 		}
 		bp, _ := term.client.GetBreakpoint(1)
 		if bp.Cond != "i == 1" {