Repository: magefile/mage
Pull request #398 (2021-12-22 19:47:06)

feat: allow mage:import alias to be defined for multiple imports
When testing this locally, I haven't been able to trigger any collisions that isnt already flagged by the existing sanity checks. So far everything has just worked as expected, all existing tests are passing.  

Documentation wise, I couldnt find anything that actually specifies that the an import alias needs to be unique, so I dont feel we need to add something there

viktorvoltaire: @natefinch given our discussion on slack, should i proceed with writing a test where we expect error if target exist in both packages?
mirogta: @viktorvoltaire wow, nice one, why hadn't I think about this one? :-)

Looks good.

Documentation wise: Now I wonder if you have a specific real use case where this would help you and it might me beneficial for others to explain this use case and actually mention when/how this can be used?
viktorvoltaire: > Documentation wise: Now I wonder if you have a specific real use case where this would help you and it might me beneficial for others to explain this use case and actually mention when/how this can be used?

We use the namespaces to group commands, to make it easier to identify. For us it makes sense that imports from different packages should be able to be grouped together. I actually thought that this was possible before the issue showed itself for me :) Hence why I think the documentation is good as it is

viktorvoltaire: @natefinch what do you think? 

Patch:
diff --git a/parse/parse.go b/parse/parse.go
index 5ab0070..37df031 100644
--- a/parse/parse.go
+++ b/parse/parse.go
@@ -267,7 +267,7 @@ func Package(path string, files []string) (*PkgInfo, error) {
 
 func getNamedImports(gocmd string, pkgs map[string]string) ([]*Import, error) {
 	var imports []*Import
-	for alias, pkg := range pkgs {
+	for pkg, alias := range pkgs {
 		debug.Printf("getting import package %q, alias %q", pkg, alias)
 		imp, err := getImport(gocmd, pkg, alias)
 		if err != nil {
@@ -411,10 +411,7 @@ func setImports(gocmd string, pi *PkgInfo) error {
 				}
 				if alias != "" {
 					debug.Printf("found %s: %s (%s)", importTag, name, alias)
-					if importNames[alias] != "" {
-						return fmt.Errorf("duplicate import alias: %q", alias)
-					}
-					importNames[alias] = name
+					importNames[name] = alias
 				} else {
 					debug.Printf("found %s: %s", importTag, name)
 					rootImports = append(rootImports, name)


Test patch:
diff --git a/mage/import_test.go b/mage/import_test.go
index af7d5ff..83949b2 100644
--- a/mage/import_test.go
+++ b/mage/import_test.go
@@ -312,3 +312,57 @@ Error parsing magefiles: error running "go list -f {{.Dir}}||{{.Name}} github.co
 		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actualShortened)
 	}
 }
+
+func TestMageImportsSameNamespaceUniqueTargets(t *testing.T) {
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+	inv := Invocation{
+		Dir:    "./testdata/mageimport/samenamespace/uniquetargets",
+		Stdout: stdout,
+		Stderr: stderr,
+		List:   true,
+	}
+
+	code := Invoke(inv)
+	if code != 0 {
+		t.Fatalf("expected to exit with code 0, but got %v, stderr:\n%s", code, stderr)
+	}
+	actual := stdout.String()
+	expected := `
+Targets:
+  samenamespace:build1    
+  samenamespace:build2    
+`[1:]
+
+	if actual != expected {
+		t.Logf("expected: %q", expected)
+		t.Logf("  actual: %q", actual)
+		t.Fatalf("expected:\n%v\n\ngot:\n%v", expected, actual)
+	}
+}
+
+func TestMageImportsSameNamespaceDupTargets(t *testing.T) {
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+	inv := Invocation{
+		Dir:    "./testdata/mageimport/samenamespace/duptargets",
+		Stdout: stdout,
+		Stderr: stderr,
+		List:   true,
+	}
+
+	code := Invoke(inv)
+	if code != 1 {
+		t.Fatalf("expected to exit with code 1, but got %v, stderr:\n%s", code, stderr)
+	}
+	actual := stderr.String()
+	expected := `
+Error parsing magefiles: "samenamespace:build" target has multiple definitions: github.com/magefile/mage/mage/testdata/mageimport/samenamespace/duptargets/package1.Build, github.com/magefile/mage/mage/testdata/mageimport/samenamespace/duptargets/package2.Build
+
+`[1:]
+	if actual != expected {
+		t.Logf("expected: %q", expected)
+		t.Logf("  actual: %q", actual)
+		t.Fatalf("expected:\n%v\n\ngot:\n%v", expected, actual)
+	}
+}
diff --git a/mage/testdata/mageimport/samenamespace/duptargets/magefile.go b/mage/testdata/mageimport/samenamespace/duptargets/magefile.go
new file mode 100644
index 0000000..5c58530
--- /dev/null
+++ b/mage/testdata/mageimport/samenamespace/duptargets/magefile.go
@@ -0,0 +1,10 @@
+// +build mage
+
+package sametarget
+
+import (
+	// mage:import samenamespace
+	_ "github.com/magefile/mage/mage/testdata/mageimport/samenamespace/duptargets/package1"
+	// mage:import samenamespace
+	_ "github.com/magefile/mage/mage/testdata/mageimport/samenamespace/duptargets/package2"
+)
diff --git a/mage/testdata/mageimport/samenamespace/duptargets/package1/package1.go b/mage/testdata/mageimport/samenamespace/duptargets/package1/package1.go
new file mode 100644
index 0000000..3f634e7
--- /dev/null
+++ b/mage/testdata/mageimport/samenamespace/duptargets/package1/package1.go
@@ -0,0 +1,7 @@
+package package1
+
+import "fmt"
+
+func Build() {
+	fmt.Println("build")
+}
diff --git a/mage/testdata/mageimport/samenamespace/duptargets/package2/package2.go b/mage/testdata/mageimport/samenamespace/duptargets/package2/package2.go
new file mode 100644
index 0000000..1d7826f
--- /dev/null
+++ b/mage/testdata/mageimport/samenamespace/duptargets/package2/package2.go
@@ -0,0 +1,7 @@
+package package2
+
+import "fmt"
+
+func Build() {
+	fmt.Println("build")
+}
diff --git a/mage/testdata/mageimport/samenamespace/uniquetargets/magefile.go b/mage/testdata/mageimport/samenamespace/uniquetargets/magefile.go
new file mode 100644
index 0000000..aebacbb
--- /dev/null
+++ b/mage/testdata/mageimport/samenamespace/uniquetargets/magefile.go
@@ -0,0 +1,10 @@
+// +build mage
+
+package main
+
+import (
+	// mage:import samenamespace
+	_ "github.com/magefile/mage/mage/testdata/mageimport/samenamespace/uniquetargets/package1"
+	// mage:import samenamespace
+	_ "github.com/magefile/mage/mage/testdata/mageimport/samenamespace/uniquetargets/package2"
+)
diff --git a/mage/testdata/mageimport/samenamespace/uniquetargets/package1/package1.go b/mage/testdata/mageimport/samenamespace/uniquetargets/package1/package1.go
new file mode 100644
index 0000000..ce7488f
--- /dev/null
+++ b/mage/testdata/mageimport/samenamespace/uniquetargets/package1/package1.go
@@ -0,0 +1,7 @@
+package package1
+
+import "fmt"
+
+func Build1() {
+	fmt.Println("build")
+}
diff --git a/mage/testdata/mageimport/samenamespace/uniquetargets/package2/package2.go b/mage/testdata/mageimport/samenamespace/uniquetargets/package2/package2.go
new file mode 100644
index 0000000..7270997
--- /dev/null
+++ b/mage/testdata/mageimport/samenamespace/uniquetargets/package2/package2.go
@@ -0,0 +1,7 @@
+package package2
+
+import "fmt"
+
+func Build2() {
+	fmt.Println("build")
+}