TestMixedMageImports fail on macOS in modules mode (current master) #224

misha-ridge · 2019-01-29T16:15:06Z

Hello.

Running go test ./... on macOS, commit 5dee1ad gives the following:

--- FAIL: TestMixedMageImports (0.17s)
    main_test.go:249: expected to exit with code 0, but got 1, stderr: build command-line-arguments: cannot find module for path _/Users/dottedmag/srcs/mage/mage/testdata/mixed_lib_files/subdir
        Error: error compiling magefiles
    main_test.go:254: expected "Targets:\n  build    \n" but got ""

The text was updated successfully, but these errors were encountered:

natefinch · 2019-02-04T04:12:07Z

Sorry, missed this when it was first filed. It works for me on my mac, but I'm not using modules, that may be the problem.

misha-ridge · 2019-02-04T07:00:10Z

Right, the test passes if I run it from $GOPATH

nlowe · 2019-06-23T19:44:34Z

Note this doesn't seem to be mac specific. This also is causing failures on travis with Go: tip and locally with 1.12:

$ docker run -it --rm -v "$(pwd):/mage" -w /mage golang:1.12 go test -v -run TestMixedMageImports ./mage/...
=== RUN   TestMixedMageImports
--- FAIL: TestMixedMageImports (0.12s)
    main_test.go:249: expected to exit with code 0, but got 1, stderr: build command-line-arguments: cannot find module for path _/mage/mage/testdata/mixed_lib_files/subdir
        Error: error compiling magefiles
    main_test.go:254: expected "Targets:\n  build    \n" but got ""
FAIL
FAIL    github.com/magefile/mage/mage   0.132s

Throwing it in the proper place in $GOPATH seems to fix it:

docker run -it --rm -v "$(pwd):/go/src/github.com/magefile/mage" -w /go/src/github.com/magefile/mage golang:1.12 go test -v -run TestMixedMageImports ./mage/...
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.33s)
PASS
ok      github.com/magefile/mage/mage   0.347s

nlowe · 2019-06-23T19:56:09Z

So you can sort of work around this by adding go_import_path to the .travis.yml file, but something seems to have changed from 1.12 in dev that breaks it.

nlowe · 2019-06-23T20:15:22Z

Ah, so the definition of GO111MODULE changed in 1.13 (golang/go#31857). It's treated as if it were set to auto when unset. In 1.13+, this means if go is run in a directory with a go.mod file, or in a subdirectory of a directory that contains a go.mod file, the go modules behavior is enabled.

I was able to work around this with this patch:

diff --git a/.travis.yml b/.travis.yml
index f7e0703..895ed9b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -9,12 +9,15 @@ branches:
 # library, I'm not going to worry about older versions for now.
 go:
   - tip
+  - 1.12.x
   - 1.11.x
   - 1.10.x
   - 1.9.x
   - 1.8.x
   - 1.7.x
 
+go_import_path: github.com/magefile/mage
+
 # don't call go get ./... because this hides when deps are
 # not packaged into the vendor directory.
 install: true
diff --git a/mage/main_test.go b/mage/main_test.go
index 286557e..0d97535 100644
--- a/mage/main_test.go
+++ b/mage/main_test.go
@@ -236,6 +236,32 @@ func TestListMagefilesLib(t *testing.T) {
 }
 
 func TestMixedMageImports(t *testing.T) {
+	// Go 1.13+ changes how GO111MODULE behaves. In 1.13, it defaults
+	// to `auto`, which enables Go Module functionality if the working
+	// directory contains a `go.mod` file, or is a subdirectory of a
+	// directory that contains a `go.mod file`.
+	//
+	// This test should be fixed to work in modules mode, but for now
+	// we can work around it by forcing `GO111MODULE` to `off` for
+	// this specific test.
+	go111module, set := os.LookupEnv("GO111MODULE")
+
+	if err := os.Setenv("GO111MODULE", "off"); err != nil {
+		t.Fatalf("Failed to disable go modules for flaky test")
+	}
+
+	defer func() {
+		var err error
+		if set {
+			err = os.Setenv("GO111MODULE", go111module)
+		} else {
+			err = os.Unsetenv("GO111MODULE")
+		}
+		if err != nil {
+			t.Fatalf("Failed to reset go module status")
+		}
+	}()
+
 	stderr := &bytes.Buffer{}
 	stdout := &bytes.Buffer{}
 	inv := Invocation{

nlowe · 2019-06-25T01:47:28Z

What's weird is if I force that test to run with Verbose and Debug is it seems to actually find the target in the magefiles:

$ docker run -it --rm -v "$(pwd):/mage" -w /mage golang:1.12 go test -v -run TestMixedMageImports ./mage/...
=== RUN   TestMixedMageImports
DEBUG: 01:43:14.054702 found target Build
DEBUG: 01:43:14.054739 time parse Magefiles: 3.0577ms
--- FAIL: TestMixedMageImports (0.18s)
    main_test.go:278: expected to exit with code 0, but got 1, stderr: build command-line-arguments: cannot find module for path _/mage/mage/testdata/mixed_lib_files/subdir
        Error: error compiling magefiles
    main_test.go:283: expected "Targets:\n  build    \n" but got ""
FAIL
FAIL    github.com/magefile/mage/mage   0.193s

nlowe · 2019-06-25T02:53:48Z

Ooo, progress. I think I have a fix for this. I hacked in some of the extra debugging to get some more verbose output, but that didn't help too much, so you can ignore that.

Changing

mage/mage/testdata/mixed_lib_files/magefile.go

Line 5 in 5bc3a8a

import "./subdir"

to

import "github.com/magefile/mage/mage/testdata/mixed_lib_files/subdir"

works on golang/go master:

C:\Users\Nathan\projects\mage [test/fix-test-failure +0 ~3 -0 !]
λ  docker run -it --rm -v "$(pwd):/mage" -w /mage -e GO111MODULE=auto golang:master go test -v -run TestMixedMageImports ./mage/...
=== RUN   TestMixedMageImports
DEBUG: 02:50:40.833023 getting all non-mage files in ./testdata/mixed_lib_files
DEBUG: 02:50:40.848897 found non-mage files lib.go
DEBUG: 02:50:40.848913 marked file as non-mage: "lib.go"
DEBUG: 02:50:40.848915 getting all files plus mage files
DEBUG: 02:50:40.866672 time to scan for Magefiles: 33.6508ms
DEBUG: 02:50:40.866707 found magefiles: testdata/mixed_lib_files/mage_helpers.go, testdata/mixed_lib_files/magefile.go
DEBUG: 02:50:40.871552 output exe is  /tmp/065651766/c92da59d59823ebbd68ab563dbfaf0afe36ab312
DEBUG: 02:50:40.875778 build cache exists, will ignore any compiled binary
DEBUG: 02:50:40.875806 parsing files
DEBUG: 02:50:40.878311 found target Build
DEBUG: 02:50:40.878351 time parse Magefiles: 2.5399ms
DEBUG: 02:50:40.878358 Creating mainfile at testdata/mixed_lib_files/mage_output_file.go
DEBUG: 02:50:40.879989 writing new file at testdata/mixed_lib_files/mage_output_file.go
About to compile [testdata/mixed_lib_files/mage_helpers.go testdata/mixed_lib_files/magefile.go testdata/mixed_lib_files/mage_output_file.go]
DEBUG: 02:50:40.882821 compiling to /tmp/065651766/c92da59d59823ebbd68ab563dbfaf0afe36ab312
DEBUG: 02:50:40.882824 compiling using gocmd: go
DEBUG: 02:50:40.889598 running go build -o /tmp/065651766/c92da59d59823ebbd68ab563dbfaf0afe36ab312 mage_helpers.go magefile.go mage_output_file.go
DEBUG: 02:50:41.195399 time to compile Magefile: 305.7445ms
Running compiled binary
DEBUG: 02:50:41.196286 running binary /tmp/065651766/c92da59d59823ebbd68ab563dbfaf0afe36ab312
DEBUG: 02:50:41.196298 running magefile with mage vars:
MAGEFILE_CACHE=/tmp/065651766
MAGEFILE_VERBOSE=1
MAGEFILE_LIST=1
MAGEFILE_DEBUG=1
MAGEFILE_GOCMD=go
--- PASS: TestMixedMageImports (0.37s)
PASS
ok      github.com/magefile/mage/mage   0.379s
C:\Users\Nathan\projects\mage [test/fix-test-failure +0 ~4 -0 !]
λ  docker run -it --rm -v "$(pwd):/go/src/github.com/magefile/mage" -w /go/src/github.com/magefile/mage -e GO111MODULE=auto golang:master go test -v -run TestMixedMageImports ./mage/...
=== RUN   TestMixedMageImports
DEBUG: 02:50:47.814008 getting all non-mage files in ./testdata/mixed_lib_files
DEBUG: 02:50:47.830877 found non-mage files lib.go
DEBUG: 02:50:47.830911 marked file as non-mage: "lib.go"
DEBUG: 02:50:47.830914 getting all files plus mage files
DEBUG: 02:50:47.848404 time to scan for Magefiles: 34.3905ms
DEBUG: 02:50:47.848452 found magefiles: testdata/mixed_lib_files/mage_helpers.go, testdata/mixed_lib_files/magefile.go
DEBUG: 02:50:47.854094 output exe is  /tmp/583154524/c92da59d59823ebbd68ab563dbfaf0afe36ab312
DEBUG: 02:50:47.859194 build cache exists, will ignore any compiled binary
DEBUG: 02:50:47.859222 parsing files
DEBUG: 02:50:47.862175 found target Build
DEBUG: 02:50:47.862216 time parse Magefiles: 2.9874ms
DEBUG: 02:50:47.862223 Creating mainfile at testdata/mixed_lib_files/mage_output_file.go
DEBUG: 02:50:47.863877 writing new file at testdata/mixed_lib_files/mage_output_file.go
About to compile [testdata/mixed_lib_files/mage_helpers.go testdata/mixed_lib_files/magefile.go testdata/mixed_lib_files/mage_output_file.go]
DEBUG: 02:50:47.866445 compiling to /tmp/583154524/c92da59d59823ebbd68ab563dbfaf0afe36ab312
DEBUG: 02:50:47.866449 compiling using gocmd: go
DEBUG: 02:50:47.872456 running go build -o /tmp/583154524/c92da59d59823ebbd68ab563dbfaf0afe36ab312 mage_helpers.go magefile.go mage_output_file.go
DEBUG: 02:50:48.175216 time to compile Magefile: 302.7146ms
Running compiled binary
DEBUG: 02:50:48.176137 running binary /tmp/583154524/c92da59d59823ebbd68ab563dbfaf0afe36ab312
DEBUG: 02:50:48.176154 running magefile with mage vars:
MAGEFILE_CACHE=/tmp/583154524
MAGEFILE_VERBOSE=1
MAGEFILE_LIST=1
MAGEFILE_DEBUG=1
MAGEFILE_GOCMD=go
--- PASS: TestMixedMageImports (0.36s)
PASS
ok      github.com/magefile/mage/mage   0.376s

I think this is just due to go not liking relative imports, regardless of where the code is in the gopath.

nlowe · 2019-06-25T02:58:42Z

Yeah, I tested this on all versions tested by Travis and it works for all of them:

Test Results

C:\Users\Nathan\projects\mage [master ≡ +0 ~2 -0 !]
λ  @(@(7..12) | ForEach-Object { "1.$_" }) + "master" | foreach-object { "Testing golang $_"; docker run -it --rm -v "$(pwd):/go/src/github.com/magefile/mage" -w /go/src/github.com/magefile/mage "golang:$_" go test -v -run TestMixedMageImports ./mage/... }
Testing golang 1.7
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.41s)
PASS
ok      github.com/magefile/mage/mage   0.424s
Testing golang 1.8
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.36s)
PASS
ok      github.com/magefile/mage/mage   0.377s
Testing golang 1.9
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.36s)
PASS
ok      github.com/magefile/mage/mage   0.377s
Testing golang 1.10
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.30s)
PASS
ok      github.com/magefile/mage/mage   0.312s
Testing golang 1.11
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.33s)
PASS
ok      github.com/magefile/mage/mage   0.346s
Testing golang 1.12
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.36s)
PASS
ok      github.com/magefile/mage/mage   0.375s
Testing golang master
=== RUN   TestMixedMageImports
--- PASS: TestMixedMageImports (0.35s)
PASS
ok      github.com/magefile/mage/mage   0.365s

Now, the question is, is the test incorrect (in which case, apply the import patch), or should mage be more aggressive about GO111MODULE for specific versions of Go? I can probably submit a PR for either option.

nlowe · 2019-06-25T04:10:37Z

Althought, to be fair, I have no idea why

mage/mage/testdata/transitiveDeps/magefile.go

Line 5 in 5bc3a8a

import "./dep"

doesn't break

mage/mage/main_test.go

Lines 63 to 109 in 5bc3a8a

    
           func TestTransitiveDepCache(t *testing.T) { 
        
           	cache, err := internal.OutputDebug("go", "env", "gocache") 
        
           	if err != nil { 
        
           		t.Fatal(err) 
        
           	} 
        
           	if cache == "" { 
        
           		t.Skip("skipping gocache tests on go version without cache") 
        
           	} 
        
           	// Test that if we change a transitive dep, that we recompile 
        
           	stdout := &bytes.Buffer{} 
        
           	stderr := &bytes.Buffer{} 
        
           	inv := Invocation{ 
        
           		Stderr: stderr, 
        
           		Stdout: stdout, 
        
           		Dir:    "testdata/transitiveDeps", 
        
           		Args:   []string{"Run"}, 
        
           	} 
        
           	code := Invoke(inv) 
        
           	if code != 0 { 
        
           		t.Fatalf("got code %v, err: %s", code, stderr) 
        
           	} 
        
           	expected := "woof\n" 
        
           	if actual := stdout.String(); actual != expected { 
        
           		t.Fatalf("expected %q but got %q", expected, actual) 
        
           	} 
        
           	// ok, so baseline, the generated and cached binary should do "woof" 
        
           	// now change out the transitive dependency that does the output 
        
           	// so that it produces different output. 
        
           	if err := os.Rename("testdata/transitiveDeps/dep/dog.go", "testdata/transitiveDeps/dep/dog.notgo"); err != nil { 
        
           		t.Fatal(err) 
        
           	} 
        
           	defer os.Rename("testdata/transitiveDeps/dep/dog.notgo", "testdata/transitiveDeps/dep/dog.go") 
        
           	if err := os.Rename("testdata/transitiveDeps/dep/cat.notgo", "testdata/transitiveDeps/dep/cat.go"); err != nil { 
        
           		t.Fatal(err) 
        
           	} 
        
           	defer os.Rename("testdata/transitiveDeps/dep/cat.go", "testdata/transitiveDeps/dep/cat.notgo") 
        
           	stderr.Reset() 
        
           	stdout.Reset() 
        
           	code = Invoke(inv) 
        
           	if code != 0 { 
        
           		t.Fatalf("got code %v, err: %s", code, stderr) 
        
           	} 
        
           	expected = "meow\n" 
        
           	if actual := stdout.String(); actual != expected { 
        
           		t.Fatalf("expected %q but got %q", expected, actual) 
        
           	} 
        
           }

nlowe · 2019-06-25T04:15:16Z

Oh, it does. The gocache variable is case-sensitive:

diff --git a/mage/main_test.go b/mage/main_test.go
index 286557e..4110c34 100644
--- a/mage/main_test.go
+++ b/mage/main_test.go
@@ -61,7 +61,7 @@ func testmain(m *testing.M) int {
 }

 func TestTransitiveDepCache(t *testing.T) {
-       cache, err := internal.OutputDebug("go", "env", "gocache")
+       cache, err := internal.OutputDebug("go", "env", "GOCACHE")
        if err != nil {
                t.Fatal(err)
        }

λ  @(@(7..12) | ForEach-Object { "1.$_" }) + "master" | foreach-object { "Testing golang $_"; docker run -it --rm -v "$(pwd):/go/src/github.com/magefile/mage" -w /go/src/github.com/magefile/mage "golang:$_" go test -v -run TestTransitiveDepCache ./mage/... }
Testing golang 1.7
=== RUN   TestTransitiveDepCache
--- SKIP: TestTransitiveDepCache (0.07s)
        main_test.go:69: skipping gocache tests on go version without cache
PASS
ok      github.com/magefile/mage/mage   0.080s
Testing golang 1.8
=== RUN   TestTransitiveDepCache
--- SKIP: TestTransitiveDepCache (0.04s)
        main_test.go:69: skipping gocache tests on go version without cache
PASS
ok      github.com/magefile/mage/mage   0.058s
Testing golang 1.9
=== RUN   TestTransitiveDepCache
--- SKIP: TestTransitiveDepCache (0.03s)
        main_test.go:69: skipping gocache tests on go version without cache
PASS
ok      github.com/magefile/mage/mage   0.046s
Testing golang 1.10
=== RUN   TestTransitiveDepCache
--- PASS: TestTransitiveDepCache (0.62s)
PASS
ok      github.com/magefile/mage/mage   0.639s
Testing golang 1.11
=== RUN   TestTransitiveDepCache
--- PASS: TestTransitiveDepCache (0.70s)
PASS
ok      github.com/magefile/mage/mage   0.710s
Testing golang 1.12
=== RUN   TestTransitiveDepCache
--- PASS: TestTransitiveDepCache (0.72s)
PASS
ok      github.com/magefile/mage/mage   0.732s
Testing golang master
=== RUN   TestTransitiveDepCache
--- FAIL: TestTransitiveDepCache (0.05s)
    main_test.go:82: got code 1, err: Error determining list of magefiles: failed to list mage gofiles: exit status 1
FAIL
FAIL    github.com/magefile/mage/mage   0.063s
FAIL

Making that import absolute instead of relative also fixes that test (after fixing the casing of gocache). It's the only other instance I see of relative imports in the test files.

natefinch · 2019-08-22T01:33:22Z

Ok, so this should be fixed with current master of mage. They dropped support for relative imports with modules. That .... seems like a go1 incompatibility, but I'm not the boss of them, so 🤷‍♂️

misha-ridge · 2019-08-22T01:37:19Z

Great, thanks.

To be fair, their spec says "To avoid ambiguity, Go programs cannot use relative import paths within a work space", and with modules every directory is "within a work space".

misha-ridge changed the title ~~TestMixedMageImports fail on macOS (current master)~~ TestMixedMageImports fail on macOS in modules mode (current master) Feb 4, 2019

natefinch added the 🐜 bug label Apr 10, 2019

nlowe mentioned this issue Jun 23, 2019

New Flag: -nodeps: skip target dependencies when executing target #251

Closed

nlowe mentioned this issue Jun 25, 2019

TestTransitiveDepCache is always skipped and fails on Go 1.13+ #252

Closed

natefinch closed this as completed Aug 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TestMixedMageImports fail on macOS in modules mode (current master) #224

TestMixedMageImports fail on macOS in modules mode (current master) #224

misha-ridge commented Jan 29, 2019

natefinch commented Feb 4, 2019

misha-ridge commented Feb 4, 2019

nlowe commented Jun 23, 2019 •

edited

nlowe commented Jun 23, 2019

nlowe commented Jun 23, 2019 •

edited

nlowe commented Jun 25, 2019

nlowe commented Jun 25, 2019 •

edited

nlowe commented Jun 25, 2019 •

edited

nlowe commented Jun 25, 2019

nlowe commented Jun 25, 2019

natefinch commented Aug 22, 2019

misha-ridge commented Aug 22, 2019

TestMixedMageImports fail on macOS in modules mode (current master) #224

TestMixedMageImports fail on macOS in modules mode (current master) #224

Comments

misha-ridge commented Jan 29, 2019

natefinch commented Feb 4, 2019

misha-ridge commented Feb 4, 2019

nlowe commented Jun 23, 2019 • edited

nlowe commented Jun 23, 2019

nlowe commented Jun 23, 2019 • edited

nlowe commented Jun 25, 2019

nlowe commented Jun 25, 2019 • edited

nlowe commented Jun 25, 2019 • edited

nlowe commented Jun 25, 2019

nlowe commented Jun 25, 2019

natefinch commented Aug 22, 2019

misha-ridge commented Aug 22, 2019

nlowe commented Jun 23, 2019 •

edited

nlowe commented Jun 23, 2019 •

edited

nlowe commented Jun 25, 2019 •

edited

nlowe commented Jun 25, 2019 •

edited