* ensure good PATH variable

* move names of some common environment variables

5 files changed, 137 insertions, 27 deletions

diff --git a/client/go/defaults/defaults.go b/client/go/defaults/defaults.go
index b67900e84be..fdbc898ef76 100644
--- a/client/go/defaults/defaults.go
+++ b/client/go/defaults/defaults.go
@@ -10,12 +10,13 @@ import (
 	"strings"
 
 	"github.com/vespa-engine/vespa/client/go/trace"
+	"github.com/vespa-engine/vespa/client/go/util"
 )
 
 const (
-	ENV_VESPA_HOME = "VESPA_HOME"
-	ENV_VESPA_USER = "VESPA_USER"
-	ENV_VESPA_HOST = "VESPA_HOSTNAME"
+	ENV_VESPA_HOME = util.ENV_VESPA_HOME
+	ENV_VESPA_HOST = util.ENV_VESPA_HOSTNAME
+	ENV_VESPA_USER = util.ENV_VESPA_USER
 
 	ENV_CONFIGSERVERS     = "VESPA_CONFIGSERVERS"
 	ENV_ADDR_CONFIGSERVER = "addr_configserver"
diff --git a/client/go/script-utils/startcbinary/common_env.go b/client/go/script-utils/startcbinary/common_env.go
index 144a30b2ecb..ca3c187d36a 100644
--- a/client/go/script-utils/startcbinary/common_env.go
+++ b/client/go/script-utils/startcbinary/common_env.go
@@ -8,16 +8,18 @@ import (
 	"strings"
 
 	"github.com/vespa-engine/vespa/client/go/trace"
+	"github.com/vespa-engine/vespa/client/go/util"
 	"github.com/vespa-engine/vespa/client/go/vespa"
 )
 
 const (
-	ENV_LD_PRELOAD                   = "LD_PRELOAD"
-	ENV_STD_THREAD_PREVENT_TRY_CATCH = "STD_THREAD_PREVENT_TRY_CATCH"
-	ENV_GLIBCXX_FORCE_NEW            = "GLIBCXX_FORCE_NEW"
-	ENV_JAVA_HOME                    = "JAVA_HOME"
-	ENV_LD_LIBRARY_PATH              = "LD_LIBRARY_PATH"
-	ENV_MALLOC_ARENA_MAX             = "MALLOC_ARENA_MAX"
+	ENV_JAVA_HOME        = util.ENV_JAVA_HOME
+	ENV_LD_LIBRARY_PATH  = util.ENV_LD_LIBRARY_PATH
+	ENV_LD_PRELOAD       = util.ENV_LD_PRELOAD
+	ENV_MALLOC_ARENA_MAX = util.ENV_MALLOC_ARENA_MAX
+	ENV_PATH             = util.ENV_PATH
+	ENV_ROOT             = util.ENV_ROOT
+	ENV_VESPA_USER       = util.ENV_VESPA_USER
 
 	ENV_VESPA_AFFINITY_CPU_SOCKET    = "VESPA_AFFINITY_CPU_SOCKET"
 	ENV_VESPA_LOAD_CODE_AS_HUGEPAGES = "VESPA_LOAD_CODE_AS_HUGEPAGES"
@@ -25,7 +27,6 @@ const (
 	ENV_VESPA_MALLOC_MADVISE_LIMIT   = "VESPA_MALLOC_MADVISE_LIMIT"
 	ENV_VESPA_NO_NUMACTL             = "VESPA_NO_NUMACTL"
 	ENV_VESPA_TIMER_HZ               = "VESPA_TIMER_HZ"
-	ENV_VESPA_USER                   = "VESPA_USER"
 	ENV_VESPA_USE_HUGEPAGES          = "VESPA_USE_HUGEPAGES"
 	ENV_VESPA_USE_HUGEPAGES_LIST     = "VESPA_USE_HUGEPAGES_LIST"
 	ENV_VESPA_USE_MADVISE_LIST       = "VESPA_USE_MADVISE_LIST"
@@ -37,14 +38,14 @@ const (
 	ENV_VESPA_VALGRIND_OPT           = "VESPA_VALGRIND_OPT"
 
 	// backwards compatibility variables:
-	ENV_HUGEPAGES_LIST      = "HUGEPAGES_LIST"
-	ENV_MADVISE_LIST        = "MADVISE_LIST"
-	ENV_NO_VESPAMALLOC_LIST = "NO_VESPAMALLOC_LIST"
-	ENV_PATH                = "PATH"
-	ENV_ROOT                = "ROOT"
-	ENV_VESPAMALLOCDST_LIST = "VESPAMALLOCDST_LIST"
-	ENV_VESPAMALLOCD_LIST   = "VESPAMALLOCD_LIST"
-	ENV_VESPAMALLOC_LIST    = "VESPAMALLOCD_LIST"
+	ENV_GLIBCXX_FORCE_NEW            = "GLIBCXX_FORCE_NEW"
+	ENV_HUGEPAGES_LIST               = "HUGEPAGES_LIST"
+	ENV_MADVISE_LIST                 = "MADVISE_LIST"
+	ENV_NO_VESPAMALLOC_LIST          = "NO_VESPAMALLOC_LIST"
+	ENV_STD_THREAD_PREVENT_TRY_CATCH = "STD_THREAD_PREVENT_TRY_CATCH"
+	ENV_VESPAMALLOCDST_LIST          = "VESPAMALLOCDST_LIST"
+	ENV_VESPAMALLOCD_LIST            = "VESPAMALLOCD_LIST"
+	ENV_VESPAMALLOC_LIST             = "VESPAMALLOCD_LIST"
 )
 
 func (spec *ProgSpec) considerFallback(varName, varValue string) {
diff --git a/client/go/util/env_vars.go b/client/go/util/env_vars.go
new file mode 100644
index 00000000000..7225dd972df
--- /dev/null
+++ b/client/go/util/env_vars.go
@@ -0,0 +1,18 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Author: arnej
+
+package util
+
+// well-known environment variable names
+
+const (
+	ENV_JAVA_HOME        = "JAVA_HOME"
+	ENV_LD_LIBRARY_PATH  = "LD_LIBRARY_PATH"
+	ENV_LD_PRELOAD       = "LD_PRELOAD"
+	ENV_MALLOC_ARENA_MAX = "MALLOC_ARENA_MAX"
+	ENV_PATH             = "PATH"
+	ENV_ROOT             = "ROOT"
+	ENV_VESPA_HOME       = "VESPA_HOME"
+	ENV_VESPA_HOSTNAME   = "VESPA_HOSTNAME"
+	ENV_VESPA_USER       = "VESPA_USER"
+)
diff --git a/client/go/vespa/load_env.go b/client/go/vespa/load_env.go
index 0824e729dcb..c8f66b4fc24 100644
--- a/client/go/vespa/load_env.go
+++ b/client/go/vespa/load_env.go
@@ -10,20 +10,33 @@ import (
 	"os"
 	"strings"
 
+	"github.com/vespa-engine/vespa/client/go/trace"
 	"github.com/vespa-engine/vespa/client/go/util"
 )
 
+const (
+	ENV_JAVA_HOME       = util.ENV_JAVA_HOME
+	ENV_PATH            = util.ENV_PATH
+	ENV_VESPA_HOME      = util.ENV_VESPA_HOME
+	ENV_VESPA_USER      = util.ENV_VESPA_USER
+	CURRENT_GCC_TOOLSET = "/opt/rh/gcc-toolset-11/root/usr/bin"
+)
+
 // backwards-compatible parsing of default-env.txt
 func LoadDefaultEnv() error {
-	return loadDefaultEnvTo(new(osEnvReceiver))
+	receiver := new(osEnvReceiver)
+	err := loadDefaultEnvTo(receiver)
+	ensureGoodPath(receiver)
+	return err
 }
 
 // parse default-env.txt, then dump export statements for "sh" to stdout
 func ExportDefaultEnvToSh() error {
 	holder := newShellEnvExporter()
 	err := loadDefaultEnvTo(holder)
-	holder.fallbackVar("VESPA_HOME", FindHome())
-	holder.fallbackVar("VESPA_USER", FindVespaUser())
+	holder.fallbackVar(ENV_VESPA_HOME, FindHome())
+	holder.fallbackVar(ENV_VESPA_USER, FindVespaUser())
+	ensureGoodPath(holder)
 	holder.dump()
 	return err
 }
@@ -32,6 +45,7 @@ type loadEnvReceiver interface {
 	fallbackVar(varName, varVal string)
 	overrideVar(varName, varVal string)
 	unsetVar(varName string)
+	currentValue(varName string) string
 }
 
 type osEnvReceiver struct {
@@ -48,6 +62,9 @@ func (p *osEnvReceiver) overrideVar(varName, varVal string) {
 func (p *osEnvReceiver) unsetVar(varName string) {
 	os.Unsetenv(varName)
 }
+func (p *osEnvReceiver) currentValue(varName string) string {
+	return os.Getenv(varName)
+}
 
 func loadDefaultEnvTo(r loadEnvReceiver) error {
 	const defEnvTxt = "/conf/vespa/default-env.txt"
@@ -155,10 +172,9 @@ func newShellEnvExporter() *shellEnvExporter {
 	}
 }
 func (p *shellEnvExporter) fallbackVar(varName, varVal string) {
-	if p.exportVars[varName] == "" {
-		if os.Getenv(varName) == "" || os.Getenv(varName) == varVal || p.unsetVars[varName] != "" {
-			p.overrideVar(varName, varVal)
-		}
+	old := p.currentValue(varName)
+	if old == "" || old == varVal {
+		p.overrideVar(varName, varVal)
 	}
 }
 func (p *shellEnvExporter) overrideVar(varName, varVal string) {
@@ -169,6 +185,15 @@ func (p *shellEnvExporter) unsetVar(varName string) {
 	delete(p.exportVars, varName)
 	p.unsetVars[varName] = "unset"
 }
+func (p *shellEnvExporter) currentValue(varName string) string {
+	if p.unsetVars[varName] != "" {
+		return ""
+	}
+	if val, ok := p.exportVars[varName]; ok {
+		return val
+	}
+	return os.Getenv(varName)
+}
 
 func shellQuote(s string) string {
 	l := 0
@@ -233,3 +258,48 @@ func (p *shellEnvExporter) dump() {
 		fmt.Printf("unset %s\n", vn)
 	}
 }
+
+type pathBuilder struct {
+	curPath []string
+}
+
+func (builder *pathBuilder) applyTo(receiver loadEnvReceiver) {
+	newPath := strings.Join(builder.curPath, ":")
+	trace.Trace("updating PATH in environment =>", newPath)
+	receiver.overrideVar(ENV_PATH, newPath)
+}
+
+func (builder *pathBuilder) appendPath(p string) {
+	if !util.IsDirectory(p) {
+		return
+	}
+	for _, elem := range builder.curPath {
+		if elem == p {
+			return
+		}
+	}
+	builder.curPath = append(builder.curPath, p)
+}
+
+func ensureGoodPath(receiver loadEnvReceiver) {
+	var builder pathBuilder
+	builder.curPath = make([]string, 0, 15)
+	builder.appendPath(FindHome() + "/bin")
+	builder.appendPath(FindHome() + "/bin64")
+	// Prefer newer gdb and pstack:
+	builder.appendPath("/opt/rh/gcc-toolset-11/root/usr/bin")
+	// how to find the "java" program?
+	if javaHome := os.Getenv(ENV_JAVA_HOME); javaHome != "" {
+		builder.appendPath(javaHome + "/bin")
+	}
+	envPath := receiver.currentValue(ENV_PATH)
+	for _, p := range strings.Split(envPath, ":") {
+		builder.appendPath(p)
+	}
+	builder.appendPath("/opt/vespa-deps/bin")
+	builder.appendPath("/usr/local/bin")
+	builder.appendPath("/usr/local/sbin")
+	builder.appendPath("/usr/bin")
+	builder.appendPath("/usr/sbin")
+	builder.applyTo(receiver)
+}
diff --git a/client/go/vespa/load_env_test.go b/client/go/vespa/load_env_test.go
index e48a950b0b8..fd2b2862c2f 100644
--- a/client/go/vespa/load_env_test.go
+++ b/client/go/vespa/load_env_test.go
@@ -4,24 +4,32 @@ package vespa
 import (
 	"fmt"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/vespa-engine/vespa/client/go/trace"
 )
 
-func setup(t *testing.T, contents string) {
-	tmp := t.TempDir() + "/load_env_test.tmp"
+func setup(t *testing.T, contents string) string {
+	td := t.TempDir()
+	tmp := td + "/load_env_test.tmp"
 	vdir := tmp + "/vespa"
+	bdir := vdir + "/bin"
 	cdir := vdir + "/conf/vespa"
 	envf := cdir + "/default-env.txt"
 	err := os.MkdirAll(cdir, 0755)
 	assert.Nil(t, err)
 	t.Setenv("VESPA_HOME", vdir)
+	err = os.MkdirAll(bdir, 0755)
+	assert.Nil(t, err)
 	err = os.WriteFile(envf, []byte(contents), 0644)
 	assert.Nil(t, err)
+	return tmp
 }
 
 func TestLoadEnvSimple(t *testing.T) {
+	trace.AdjustVerbosity(0)
 	t.Setenv("VESPA_FOO", "was foo")
 	t.Setenv("VESPA_BAR", "was bar")
 	t.Setenv("VESPA_FOOBAR", "foobar")
@@ -161,3 +169,15 @@ unset XYZ
 	err = ExportDefaultEnvToSh()
 	assert.Nil(t, err)
 }
+
+func TestLoadEnvNop(t *testing.T) {
+	td := setup(t, "")
+	t.Setenv("PATH", td)
+	err := LoadDefaultEnv()
+	assert.Nil(t, err)
+	// check results
+	path := os.Getenv("PATH")
+	fmt.Println("got path:", path)
+	assert.True(t, strings.Contains(path, td+"/vespa/bin:"))
+	assert.True(t, strings.Contains(path, ":"+td))
+}