Skip to content

Commit

Permalink
fix(goenv.GOMOD): never rely on the environment variable (#384)
Browse files Browse the repository at this point in the history
The value of `go env GOMOD` changes depending on the working directory
where the command is executed. Instead of relying on the environment
variable, always spawn `go env GOMOD` in an explicitly selected
directory.
  • Loading branch information
RomainMuller authored Nov 12, 2024
1 parent 9fabf32 commit e6d1010
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 33 deletions.
8 changes: 7 additions & 1 deletion internal/ensure/requiredversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"runtime"
"syscall"

"github.com/DataDog/orchestrion/internal/goenv"
"github.com/DataDog/orchestrion/internal/log"
"github.com/DataDog/orchestrion/internal/version"
"golang.org/x/tools/go/packages"
Expand Down Expand Up @@ -128,8 +129,13 @@ func requiredVersion(
// working directory is used. The version may be blank if a replace directive is in effect; in which
// case the path value may indicate the location of the source code that is being used instead.
func goModVersion(dir string) (moduleVersion string, moduleDir string, err error) {
gomod, err := goenv.GOMOD(dir)
if err != nil {
return "", "", err
}

cfg := &packages.Config{
Dir: dir,
Dir: filepath.Dir(gomod),
Mode: packages.NeedModule,
Logf: func(format string, args ...any) { log.Tracef(format+"\n", args...) },
}
Expand Down
8 changes: 3 additions & 5 deletions internal/ensure/requiredversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ func TestGoModVersion(t *testing.T) {
}

t.Run("no-go-mod", func(t *testing.T) {
tmp, err := os.MkdirTemp("", "ensure-*")
require.NoError(t, err, "failed to create temporary directory")
defer os.RemoveAll(tmp)
tmp := t.TempDir()

os.WriteFile(filepath.Join(tmp, "main.go"), []byte(`
package main
Expand All @@ -98,9 +96,9 @@ func TestGoModVersion(t *testing.T) {
`), 0o644)

require.NotPanics(t, func() {
_, _, err = goModVersion(tmp)
_, _, err := goModVersion(tmp)
require.ErrorIs(t, err, goenv.ErrNoGoMod)
})
require.ErrorContains(t, err, "go.mod file not found in current directory")
})
}

Expand Down
10 changes: 4 additions & 6 deletions internal/goenv/goenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ import (

var (
// ErrNoGoMod is returned when no GOMOD value could be identified.
ErrNoGoMod = errors.New("`go mod GOMOD` returned a blank string")
ErrNoGoMod = errors.New("`go env GOMOD` returned a blank string")
)

// GOMOD returns the current GOMOD environment variable (possibly from running `go env GOMOD`).
func GOMOD() (string, error) {
if goMod := os.Getenv("GOMOD"); goMod != "" {
return goMod, nil
}
// GOMOD returns the current GOMOD environment variable (from running `go env GOMOD`).
func GOMOD(dir string) (string, error) {
cmd := exec.Command("go", "env", "GOMOD")
cmd.Dir = dir
var stdout bytes.Buffer
cmd.Stdout = &stdout
if err := cmd.Run(); err != nil {
Expand Down
22 changes: 2 additions & 20 deletions internal/goenv/goenv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,21 @@
package goenv

import (
"os"
"testing"

"github.com/stretchr/testify/require"
)

func TestGOMOD(t *testing.T) {
t.Run("without GOMOD environment variable", func(t *testing.T) {
t.Setenv("GOMOD", "")

gomod, err := GOMOD()
gomod, err := GOMOD("")
require.NoError(t, err)
require.NotEmpty(t, gomod)
})

t.Run("no GOMOD can be found", func(t *testing.T) {
t.Setenv("GOMOD", "")

wd, _ := os.Getwd()
defer os.Chdir(wd)
os.Chdir(os.TempDir())

val, err := GOMOD()
val, err := GOMOD(t.TempDir())
require.Empty(t, val)
require.ErrorIs(t, err, ErrNoGoMod)
})

t.Run("with GOMOD environment variable", func(t *testing.T) {
expected := "/fake/path/to/go.mod"
t.Setenv("GOMOD", expected)

gomod, err := GOMOD()
require.NoError(t, err)
require.EqualValues(t, expected, gomod)
})
}
2 changes: 1 addition & 1 deletion internal/pin/pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func defaultOrchestrionToolGo() *dst.File {
// PinOrchestrion applies or update the orchestrion pin file in the current
// working directory, according to the supplied [Options].
func PinOrchestrion(opts Options) error {
goMod, err := goenv.GOMOD()
goMod, err := goenv.GOMOD("")
if err != nil {
return fmt.Errorf("getting GOMOD: %w", err)
}
Expand Down

0 comments on commit e6d1010

Please sign in to comment.