Skip to content

Commit 7522bc3

Browse files
committed
Fix multibuild with a package path
Builds with a specified package path were failing at source discovery time. We need to scan the list of inputs to find the source dir, and build the sources list from that, also prepending the package path to each source path so that subsequently scanning for configuration can work. Note that for the time being we explicitly disallow building multiple paths, as that requires more work. See #14. Fixes: #13
1 parent 00ccc84 commit 7522bc3

2 files changed

Lines changed: 243 additions & 55 deletions

File tree

cmd/multibuild/integration_test.go

Lines changed: 171 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ func TestHelp(t *testing.T) {
1717
cmd := exec.Command("go", "build", "-o", bin)
1818
cmd.Stdout = os.Stdout
1919
cmd.Stderr = os.Stderr
20-
cmd.Env = append(os.Environ(), "CGO_ENABLED=0")
2120
if err := cmd.Run(); err != nil {
2221
t.Fatalf("build failed: %v", err)
2322
}
@@ -36,7 +35,6 @@ multibuild-specific options:
3635
for _, test := range []string{"-h", "--help"} {
3736
t.Run(test, func(t *testing.T) {
3837
cmd = exec.Command(bin, test)
39-
cmd.Env = append(os.Environ(), "CGO_ENABLED=0")
4038

4139
out, err := cmd.CombinedOutput()
4240
if err != nil {
@@ -58,14 +56,13 @@ multibuild-specific options:
5856
}
5957
}
6058

61-
func TestMultibuild(t *testing.T) {
59+
func TestMultibuildWithConfiguration(t *testing.T) {
6260
binTmp := t.TempDir()
6361
bin := filepath.Join(binTmp, "multibuild")
6462

6563
cmd := exec.Command("go", "build", "-o", bin)
6664
cmd.Stdout = os.Stdout
6765
cmd.Stderr = os.Stderr
68-
cmd.Env = append(os.Environ(), "CGO_ENABLED=0")
6966
if err := cmd.Run(); err != nil {
7067
t.Fatalf("build failed: %v", err)
7168
}
@@ -196,3 +193,173 @@ func main() {
196193
})
197194
}
198195
}
196+
197+
func TestMultibuildDifferentStyles(t *testing.T) {
198+
type testCase struct {
199+
name string
200+
numPackages int
201+
numBinariesPerPkg int
202+
runDir string
203+
args []string
204+
expectErr bool
205+
expectedBinaries []string
206+
}
207+
208+
goos := runtime.GOOS
209+
goarch := runtime.GOARCH
210+
211+
// TODO: A little too much magic generation in this test, but unsure how else to structure it.
212+
// TODO: We presently only test building inside a single module. That's probably OK, or do we need to test more?
213+
// TODO: We don't have tests to cover multiple source files that aren't binaries, and we should.
214+
testCases := []testCase{
215+
{
216+
// tests "multibuild" with no arguments should produce binaries
217+
name: "build in source dir",
218+
numPackages: 1,
219+
numBinariesPerPkg: 1,
220+
runDir: "pkg1",
221+
args: []string{},
222+
expectErr: false,
223+
expectedBinaries: []string{
224+
fmt.Sprintf("pkg1-%s-%s", goos, goarch),
225+
},
226+
},
227+
{
228+
// tests "multibuild pkg/" should produce binaries
229+
name: "build via path/",
230+
numPackages: 1,
231+
numBinariesPerPkg: 1,
232+
runDir: ".",
233+
args: []string{"./pkg1"},
234+
expectErr: false,
235+
expectedBinaries: []string{
236+
fmt.Sprintf("pkg1-%s-%s", goos, goarch),
237+
},
238+
},
239+
{
240+
// tests "multibuild pkg/main1.go" should produce binaries
241+
name: "build via single .go file",
242+
numPackages: 1,
243+
numBinariesPerPkg: 1,
244+
runDir: ".",
245+
args: []string{"pkg1/main1.go"},
246+
expectErr: false,
247+
expectedBinaries: []string{
248+
fmt.Sprintf("pkg1-%s-%s", goos, goarch),
249+
},
250+
},
251+
{
252+
// tests that currently, building two binaries should fail
253+
name: "build two binaries by file",
254+
numPackages: 1,
255+
numBinariesPerPkg: 2,
256+
runDir: ".",
257+
args: []string{"pkg1/main1.go", "pkg1/main2.go"},
258+
expectErr: true,
259+
expectedBinaries: []string{},
260+
},
261+
{
262+
// tests that currently, building two packages should fail
263+
name: "build two packages by path/",
264+
numPackages: 2,
265+
numBinariesPerPkg: 1,
266+
runDir: ".",
267+
args: []string{"pkg1", "pkg2"},
268+
expectErr: true,
269+
expectedBinaries: []string{},
270+
},
271+
}
272+
273+
tmpRoot := t.TempDir()
274+
bin := filepath.Join(tmpRoot, "multibuild")
275+
276+
cmd := exec.Command("go", "build", "-o", bin)
277+
cmd.Stdout = os.Stdout
278+
cmd.Stderr = os.Stderr
279+
if err := cmd.Run(); err != nil {
280+
t.Fatalf("build failed: %v", err)
281+
}
282+
283+
for _, tc := range testCases {
284+
t.Run(tc.name, func(t *testing.T) {
285+
// Setup packages and binaries
286+
gover := runtime.Version() // "go1.24..."
287+
if gover[0:2] != "go" { // check for, and skip the "go" prefix
288+
t.Fatalf("unexpected go version: %s", gover)
289+
}
290+
gover = gover[2:]
291+
baseMod := fmt.Sprintf("module %s\n\ngo %s\n", "testmod", gover)
292+
if err := os.WriteFile(filepath.Join(tmpRoot, "go.mod"), []byte(baseMod), 0644); err != nil {
293+
t.Fatalf("failed to write go.mod: %v", err)
294+
}
295+
296+
for p := 1; p <= tc.numPackages; p++ {
297+
pkgDir := filepath.Join(tmpRoot, fmt.Sprintf("pkg%d", p))
298+
os.RemoveAll(pkgDir)
299+
300+
if err := os.Mkdir(pkgDir, 0755); err != nil {
301+
t.Fatalf("failed to mkdir: %v", err)
302+
}
303+
for b := 1; b <= tc.numBinariesPerPkg; b++ {
304+
mainSource := fmt.Sprintf(`package main
305+
import "fmt"
306+
func main() { fmt.Println("Hello from main%d in pkg%d") }
307+
`, b, p)
308+
309+
mainPath := filepath.Join(pkgDir, fmt.Sprintf("main%d.go", b))
310+
if err := os.WriteFile(mainPath, []byte(mainSource), 0644); err != nil {
311+
t.Fatalf("failed to write %s: %v", mainPath, err)
312+
}
313+
// Add multibuild config to the first file in each package
314+
if b == 1 {
315+
config := `//go:multibuild:include=` + goos + `/` + goarch + "\n"
316+
config += "//go:multibuild:output=${TARGET}-${GOOS}-${GOARCH}\n"
317+
buf, err := os.ReadFile(mainPath)
318+
if err != nil {
319+
t.Fatalf("failed to read file to inject config")
320+
}
321+
if err := os.WriteFile(mainPath, []byte(config+string(buf)), 0644); err != nil {
322+
t.Fatalf("failed to write config: %v", err)
323+
}
324+
}
325+
}
326+
}
327+
328+
var runDir string
329+
if tc.runDir == "." {
330+
runDir = tmpRoot
331+
} else {
332+
runDir = filepath.Join(tmpRoot, tc.runDir)
333+
}
334+
335+
cmd := exec.Command(bin, tc.args...)
336+
cmd.Dir = runDir
337+
out, err := cmd.CombinedOutput()
338+
339+
if tc.expectErr {
340+
if err == nil {
341+
t.Fatalf("expected error, got success:\nOutput:\n%s", string(out))
342+
}
343+
} else {
344+
if err != nil {
345+
t.Fatalf("expected success, got error: %s\nOutput:\n%s", err, string(out))
346+
}
347+
348+
if err != nil {
349+
t.Fatalf("expected success, got error: %v\nOutput:\n%s", err, string(out))
350+
}
351+
for _, binRel := range tc.expectedBinaries {
352+
var binPath string
353+
if tc.runDir == "." {
354+
binPath = filepath.Join(tmpRoot, binRel)
355+
} else {
356+
binPath = filepath.Join(runDir, binRel)
357+
}
358+
if _, err := os.Stat(binPath); err != nil {
359+
t.Errorf("expected binary %q not found", binPath)
360+
}
361+
}
362+
}
363+
})
364+
}
365+
}

cmd/multibuild/multibuild.go

Lines changed: 72 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@ import (
1616
"os"
1717
"os/exec"
1818
"path/filepath"
19-
"slices"
2019
"strings"
2120
"sync"
2221
)
2322

2423
// Discovers all source files for this package.
2524
// This is smarter than Walk() looking for *.go, because it will obey build constraints.
26-
func sourcesList() ([]string, error) {
27-
cmd := exec.Command("go", "list", "-compiled", "-json=CompiledGoFiles")
25+
func sourcesList(packagePath string) ([]string, error) {
26+
cmd := exec.Command("go", "list", "-compiled", "-json=CompiledGoFiles", packagePath)
27+
2828
var buf bytes.Buffer
2929
cmd.Stdout = &buf
3030
cmd.Stderr = os.Stderr
@@ -40,6 +40,13 @@ func sourcesList() ([]string, error) {
4040
return nil, fmt.Errorf("unmarshal: %w", err)
4141
}
4242

43+
// We must prepend packagePath to each of the paths to scan, so that
44+
// we can actually find the paths in the case where we are building
45+
// a package from an unexpected location.
46+
for idx, p := range v.CompiledGoFiles {
47+
v.CompiledGoFiles[idx] = filepath.Join(packagePath, p)
48+
}
49+
4350
return v.CompiledGoFiles, nil
4451
}
4552

@@ -60,44 +67,6 @@ func targetList() ([]target, error) {
6067
}), nil
6168
}
6269

63-
// Returns the binary name/path that `go build` would produce.
64-
func determineTargetName(args []string) (string, error) {
65-
for i := range args {
66-
arg := args[i]
67-
68-
if arg == "-o" && i+1 < len(args) {
69-
return args[i+1], nil
70-
}
71-
72-
if after, ok := strings.CutPrefix(arg, "-o="); ok {
73-
return after, nil
74-
}
75-
}
76-
77-
var nonflags []string
78-
for _, arg := range args {
79-
if !strings.HasPrefix(arg, "-") {
80-
nonflags = append(nonflags, arg)
81-
}
82-
}
83-
84-
if len(nonflags) == 1 {
85-
target := nonflags[0]
86-
87-
if strings.HasSuffix(target, ".go") {
88-
return strings.TrimSuffix(filepath.Base(target), ".go"), nil
89-
}
90-
91-
return filepath.Base(target), nil
92-
}
93-
94-
wd, err := os.Getwd()
95-
if err != nil {
96-
return "", err
97-
}
98-
return filepath.Base(wd), nil
99-
}
100-
10170
func displayUsageAndExit(self string) {
10271
fmt.Fprintf(os.Stderr, "usage: %s [-o output] [build flags] [packages]\n", self)
10372
fmt.Fprintln(os.Stderr, "multibuild is a thin wrapper around 'go build'.")
@@ -126,14 +95,25 @@ func displayTargetsAndExit(targets []target) {
12695
}
12796

12897
func main() {
129-
self := filepath.Base(os.Args[0])
130-
args := os.Args[1:]
98+
self := filepath.Base(os.Args[0]) // current binary name
99+
args := os.Args[1:] // remaining args
100+
expectOutput := false // seen -o, waiting for the rest
101+
output := "" // -o or -o=
102+
var nonflags []string // non-flag arguments
131103
displayConfig := false
132104
displayTargets := false
133105
verbose := false
134106

135107
for _, arg := range args {
136108
switch {
109+
case expectOutput:
110+
output = arg
111+
expectOutput = false
112+
case arg == "-o":
113+
expectOutput = true
114+
case strings.HasPrefix(arg, "-o="):
115+
output = strings.TrimPrefix(arg, "-o=")
116+
137117
case arg == "-h" || arg == "--help":
138118
displayUsageAndExit(self)
139119
case arg == "-v":
@@ -144,18 +124,59 @@ func main() {
144124
displayTargets = true
145125
case strings.HasPrefix(arg, "--multibuild"):
146126
fatal("multibuild: unrecognized argument %q", arg)
127+
case !strings.HasPrefix(arg, "-"):
128+
nonflags = append(nonflags, arg)
147129
}
148130
}
149131

150-
output, err := determineTargetName(args)
151-
if err != nil {
152-
fatal("multibuild: failed to get target name: %s", err)
132+
var sources []string
133+
packagePath := "" // the path being built
134+
if output == "" {
135+
switch len(nonflags) {
136+
case 0:
137+
// implicit case: multibuild on the current dir -> multibuild .
138+
packagePath = "."
139+
wd, err := os.Getwd()
140+
if err != nil {
141+
fatal("multibuild: failed to get cwd: %s", err)
142+
}
143+
output = filepath.Base(wd)
144+
case 1:
145+
t := nonflags[0]
146+
if strings.HasSuffix(t, ".go") {
147+
// multibuild cmd/foo.go
148+
packagePath = filepath.Dir(t)
149+
output = strings.TrimSuffix(filepath.Base(t), ".go")
150+
sources = append(sources, t)
151+
} else {
152+
// multibuild cmd/foo
153+
packagePath = t
154+
output = filepath.Base(t)
155+
}
156+
default:
157+
// For now, I'm cowardly refusing to handle this.
158+
// I think we need to refactor some to handle two cases:
159+
// - specifying a list of .go sources in a single ultimate package
160+
// - specifying a list of packages
161+
//
162+
// The former is handled quite easily I think, the latter will
163+
// require some additional thought and handling, as it's essentially
164+
// another level of looping on top of what we have now.
165+
//
166+
// We will need to discover sources for each package, scan independently,
167+
// and build independently.
168+
fatal("multibuild: cannot build multiple packages")
169+
}
153170
}
154171

155-
sources, err := sourcesList()
156-
if err != nil {
157-
fatal("multibuild: failed to discover sources: %s", err)
172+
if len(sources) == 0 {
173+
var err error
174+
sources, err = sourcesList(packagePath)
175+
if err != nil {
176+
fatal("multibuild: failed to discover sources: %s", err)
177+
}
158178
}
179+
159180
opts, err := scanBuildDir(sources)
160181
if err != nil {
161182
fatal("multibuild: failed to scan sources: %s", err)
@@ -203,8 +224,8 @@ func main() {
203224
out += ".exe"
204225
}
205226

206-
buildArgs := slices.Clone(args)
207-
buildArgs = append(buildArgs, "-o", out)
227+
buildArgs := []string{"-o", out}
228+
buildArgs = append(buildArgs, args...)
208229

209230
wg.Add(1) // acquire for global
210231
go func(goos, goarch string, buildArgs []string) {

0 commit comments

Comments
 (0)