From 9fc19e8bdf237bb67ef6f5971ee1fe22f6ab4827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Tue, 17 May 2022 22:20:28 +0100 Subject: [PATCH] support import paths ending with ".go" When splitFlagsFromFiles saw "-p foo/bar.go", it thought that was the first Go file, when in fact it's not. We didn't notice because such import paths are pretty rare, but they do exist, like github.com/nats-io/nats.go. Before the fix, the added test case fails as expected: > garble build -tags buildtag [stderr] # test/main/goextension.go open test/main/goextension.go: no such file or directory We could go through the trouble of teaching splitFlagsFromFiles about all of the flags understood by the compiler and linker, but that feels like far more code than the small alternative we went with. And I'm pretty sure the alternative will work pretty reliably for now. Fixes #539. --- main.go | 18 ++++++++++++++---- testdata/scripts/imports.txt | 12 ++++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/main.go b/main.go index 612df12..9ac2527 100644 --- a/main.go +++ b/main.go @@ -2023,13 +2023,23 @@ func filterForwardBuildFlags(flags []string) (filtered []string, firstUnknown st // // This function only makes sense for lower-level tool commands, such as // "compile" or "link", since their arguments are predictable. +// +// We iterate from the end rather than from the start, to better protect +// oursrelves from flag arguments that may look like paths, such as: +// +// compile [flags...] -p pkg/path.go [more flags...] file1.go file2.go +// +// For now, since those confusing flags are always followed by more flags, +// iterating in reverse order works around them entirely. func splitFlagsFromFiles(all []string, ext string) (flags, paths []string) { - for i, arg := range all { - if !strings.HasPrefix(arg, "-") && strings.HasSuffix(arg, ext) { - return all[:i:i], all[i:] + for i := len(all) - 1; i >= 0; i-- { + arg := all[i] + if strings.HasPrefix(arg, "-") || !strings.HasSuffix(arg, ext) { + cutoff := i + 1 // arg is a flag, not a path + return all[:cutoff:cutoff], all[cutoff:] } } - return all, nil + return nil, all } // flagValue retrieves the value of a flag such as "-foo", from strings in the diff --git a/testdata/scripts/imports.txt b/testdata/scripts/imports.txt index d588799..b9a4270 100644 --- a/testdata/scripts/imports.txt +++ b/testdata/scripts/imports.txt @@ -115,9 +115,13 @@ func init() { fmt.Println("buildtag init func") } -- differentpkg_unnamed.go -- package main -import "test/main/different-pkg-name" +import ( + "test/main/different-pkg-name" + "test/main/goextension.go" +) var _ = actualpkgname.Noop +var _ = goextension.Noop -- differentpkg_named.go -- package main @@ -159,6 +163,10 @@ package actualpkgname var Noop int +-- goextension.go/ext.go -- +package goextension + +var Noop int -- main.stdout -- buildtag init func imported var value @@ -167,4 +175,4 @@ x {3 {23} 0} Don't communicate by sharing memory, share memory by communicating. 42 -dummy \ No newline at end of file +dummy