From 3d9ca9b2a7ebf4c52999e676334abe6b3ce515aa Mon Sep 17 00:00:00 2001
From: Scott Robinson <ssr@squareup.com>
Date: Wed, 31 May 2023 19:47:19 +1000
Subject: [PATCH] ops: don't re-apply already applied operations

Hermit environment variable operations can be overrode by the user after
an environment has already been opened. If this is the case, then we
shouldn't re-apply the operations; we should instead expressed respect
the wish of a mangled environment by the user.

This is relevant for constructs like Python virutal environments.

Context: https://square.slack.com/archives/C01RTEH4MFV/p1685522543591719
---
 env_test.go        |  7 ++-----
 envars/ops.go      | 17 ++++++++++++++++-
 envars/ops_test.go | 12 ++++++++++++
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/env_test.go b/env_test.go
index 0565f1ebf..e6b9437c6 100644
--- a/env_test.go
+++ b/env_test.go
@@ -5,7 +5,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -417,10 +416,8 @@ func TestEnv_EphemeralVariableSubstitutionOverride(t *testing.T) {
 
 func opsContains[T any](t *testing.T, slice []T, needle T) {
 	t.Helper()
-	for _, el := range slice {
-		if reflect.DeepEqual(el, needle) {
-			return
-		}
+	if envars.OpsContains(slice, needle) {
+		return
 	}
 	t.Fatalf("%v does not contain %v", slice, needle)
 }
diff --git a/envars/ops.go b/envars/ops.go
index 283132fac..e1da92990 100644
--- a/envars/ops.go
+++ b/envars/ops.go
@@ -39,9 +39,15 @@ func (e Envars) System() []string {
 //
 // Envars are not modified.
 func (e Envars) Apply(envRoot string, ops Ops) *Transform {
+	existingOps := Ops{}
+	if rawExistingOps, err := UnmarshalOps([]byte(e["HERMIT_ENV_OPS"])); err == nil {
+		existingOps = rawExistingOps
+	}
 	transform := transform(envRoot, e)
 	for _, op := range ops {
-		op.Apply(transform)
+		if !OpsContains(existingOps, op) {
+			op.Apply(transform)
+		}
 	}
 	return transform
 }
@@ -389,6 +395,15 @@ func makeRevertKey(transform *Transform, op Op) string {
 	return fmt.Sprintf("_HERMIT_OLD_%s_%X", op.Envar(), hash.Sum(nil))
 }
 
+func OpsContains[T any](slice []T, needle T) bool {
+	for _, el := range slice {
+		if reflect.DeepEqual(el, needle) {
+			return true
+		}
+	}
+	return false
+}
+
 // transform returns a Transform with e as the base.
 func transform(envRoot string, e Envars) *Transform {
 	return &Transform{
diff --git a/envars/ops_test.go b/envars/ops_test.go
index a8a16058f..932e59856 100644
--- a/envars/ops_test.go
+++ b/envars/ops_test.go
@@ -118,6 +118,18 @@ func TestIssue47(t *testing.T) {
 	assert.Equal(t, original, reverted)
 }
 
+func TestSkipEnvarReapply(t *testing.T) {
+	original := Envars{
+		"PATH":           "/bin:/usr/bin",
+		"HERMIT_ENV_OPS": "[{\"p\":{\"n\":\"PATH\",\"v\":\"/usr/bin\"}}]",
+	}
+	ops := Ops{
+		&Prepend{Name: "PATH", Value: "/usr/bin"},
+	}
+	actual := original.Apply("/home/user/project", ops).Combined()
+	assert.Equal(t, original, actual)
+}
+
 func TestEncodeDecodeOps(t *testing.T) {
 	actual := Ops{
 		&Append{"APPEND", "${APPEND}:text"},