diff --git a/envars/ops.go b/envars/ops.go index 9392327b8..bf21df841 100644 --- a/envars/ops.go +++ b/envars/ops.go @@ -219,6 +219,9 @@ func (e *Prepend) Envar() string { return e.Name } // nolint: golint func (e *Prepend) Apply(transform *Transform) { // nolint: golint value, _ := transform.get(e.Name) prepend := transform.expand(e.Value) + if alreadyPrefixed(value, prepend) { + return + } out := splitAndDrop(value, prepend) out = append([]string{prepend}, out...) transform.set(e.Name, strings.Join(out, ":")) @@ -334,6 +337,25 @@ func (f *Force) Revert(transform *Transform) { // nolint: golint transform.unset(f.Name) } +// alreadyPrefixed reports whether all components of "prepend" (split by ":") +// already appear, in order, as a prefix of "value" (also split by ":"). +func alreadyPrefixed(value, prepend string) bool { + if value == "" || prepend == "" { + return false + } + prependParts := strings.Split(prepend, ":") + valueParts := strings.Split(value, ":") + if len(prependParts) > len(valueParts) { + return false + } + for i, p := range prependParts { + if valueParts[i] != p { + return false + } + } + return true +} + // Split "envar" by ":" and drop "value" from it. func splitAndDrop(envar string, value string) []string { parts := strings.Split(envar, ":") diff --git a/envars/ops_test.go b/envars/ops_test.go index a8a16058f..ce41730bd 100644 --- a/envars/ops_test.go +++ b/envars/ops_test.go @@ -118,6 +118,73 @@ func TestIssue47(t *testing.T) { assert.Equal(t, original, reverted) } +func TestAlreadyPrefixed(t *testing.T) { + tests := []struct { + name string + value string + prepend string + expected bool + }{ + {"SingleAtFront", "/usr/bin:/bin", "/usr/bin", true}, + {"SingleNotAtFront", "/bin:/usr/bin", "/usr/bin", false}, + {"MultiAtFront", "/a:/b:/bin", "/a:/b", true}, + {"MultiNotAtFront", "/bin:/a:/b", "/a:/b", false}, + {"EmptyValue", "", "/usr/bin", false}, + {"EmptyPrepend", "/bin", "", false}, + {"ExactMatch", "/usr/bin", "/usr/bin", true}, + {"PrependLongerThanValue", "/a", "/a:/b", false}, + {"PartialPrefixMismatch", "/a:/c:/bin", "/a:/b", false}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := alreadyPrefixed(test.value, test.prepend) + assert.Equal(t, test.expected, actual) + }) + } +} + +// Test that Prepend is a no-op when paths are already at the front. +// This simulates hermit exec when the caller (e.g. bloxlet) has already +// activated Hermit and set up PATH with all the correct paths. +func TestPrependNoOpWhenAlreadyAtFront(t *testing.T) { + // Single-element: PATH already has /hermit/bin at the front + env := Envars{ + "PATH": "/hermit/bin:/custom:/usr/bin", + } + ops := Ops{ + &Prepend{Name: "PATH", Value: "/hermit/bin"}, + } + actual := env.Apply("", ops).Combined() + assert.Equal(t, env, actual) + + // Revert should still work correctly + reverted := actual.Revert("", ops).Combined() + assert.Equal(t, Envars{"PATH": "/custom:/usr/bin"}, reverted) + + // Multi-element: PATH already has /a:/b at the front + env2 := Envars{ + "PATH": "/a:/b:/custom:/usr/bin", + } + ops2 := Ops{ + &Prepend{Name: "PATH", Value: "/a:/b"}, + } + actual2 := env2.Apply("", ops2).Combined() + assert.Equal(t, env2, actual2) + + reverted2 := actual2.Revert("", ops2).Combined() + assert.Equal(t, Envars{"PATH": "/custom:/usr/bin"}, reverted2) + + // Present but not at front: should still move to front + env3 := Envars{ + "PATH": "/bin:/usr/bin", + } + ops3 := Ops{ + &Prepend{Name: "PATH", Value: "/usr/bin"}, + } + actual3 := env3.Apply("", ops3).Combined() + assert.Equal(t, Envars{"PATH": "/usr/bin:/bin"}, actual3) +} + func TestEncodeDecodeOps(t *testing.T) { actual := Ops{ &Append{"APPEND", "${APPEND}:text"},