Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: build panics or prints warning twice when import path pattern matches 0 packages #8165

Closed
dmitshur opened this issue Jun 7, 2014 · 5 comments
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jun 7, 2014

What does 'go version' print?

go version devel +00224712f89e Fri Jun 06 16:52:14 2014 -0400 linux/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. go list ...does_not_exist
2. go build ...does_not_exist
3. go build -o ./out ...does_not_exist

What happened?

The first command works correctly, printing a single warning.
The second command works, but prints the warning twice.
The third command prints a warning one time, and then panics with "runtime error:
index out of range".

1.

warning: "...does_not_exist" matched no packages

2.

warning: "...does_not_exist" matched no packages
warning: "...does_not_exist" matched no packages

3.

warning: "...does_not_exist" matched no packages
panic: runtime error: index out of range

goroutine 16 [running]:
runtime.panic(0x7f5740, 0xa7a67c)
    /home/dmitri/Dmitri/go/src/pkg/runtime/panic.c:279 +0xf5
main.runBuild(0xa774c0, 0xc20800e040, 0x1, 0x1)
    /home/dmitri/Dmitri/go/src/cmd/go/build.go:291 +0x442
main.main()
    /home/dmitri/Dmitri/go/src/cmd/go/main.go:162 +0x55a

goroutine 19 [finalizer wait]:
runtime.park(0x46de70, 0xa7ec10, 0xa7ce89)
    /home/dmitri/Dmitri/go/src/pkg/runtime/proc.c:1369 +0x89
runtime.parkunlock(0xa7ec10, 0xa7ce89)
    /home/dmitri/Dmitri/go/src/pkg/runtime/proc.c:1385 +0x3b
runfinq()
    /home/dmitri/Dmitri/go/src/pkg/runtime/mgc0.c:2644 +0xcf
runtime.goexit()
    /home/dmitri/Dmitri/go/src/pkg/runtime/proc.c:1445

goroutine 20 [syscall]:
os/signal.loop()
    /home/dmitri/Dmitri/go/src/pkg/os/signal/signal_unix.go:21 +0x1e
created by os/signal.init·1
    /home/dmitri/Dmitri/go/src/pkg/os/signal/signal_unix.go:27 +0x32

goroutine 21 [runnable]:
text/template/parse.lexText(0xc20804c100, 0x915150)
    /home/dmitri/Dmitri/go/src/pkg/text/template/parse/lex.go:228 +0x381
text/template/parse.(*lexer).run(0xc20804c100)
    /home/dmitri/Dmitri/go/src/pkg/text/template/parse/lex.go:198 +0x40
created by text/template/parse.lex
    /home/dmitri/Dmitri/go/src/pkg/text/template/parse/lex.go:191 +0x112

What should have happened instead?

All three commands should result in the same output, a single warning that the pattern
matched zero packages. There shouldn't be two identical warnings, and there shouldn't be
a panic.

Please provide any additional information below.
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-go1.4.

@dmitshur
Copy link
Contributor Author

dmitshur commented Jun 7, 2014

Comment 2:

The panic is very easy to fix, that panic stack trace points directly to the problem (`p
:= pkgs[0]` is done, but there's no check if `len(pkgs) == 0`).
However, I'm not sure what's a better correct behavior for `go build -o ./out
...does_not_exist`. There are two choices. Either a warning and no output like the other
commands.
Alternatively, maybe the best thing to do for `go build -o ./out ...does_not_exist` is
the same as for `go build -o ./out ...matches_multiple_pkgs`, which is:
fatalf("go build: cannot use -o with multiple packages")
It's a design decision of, if the user specifies -o option, should the command always
write _some_ output and fail with error if it can't (i.e. either matches multiple
packages, or no packages, or build error, etc.). Or should it write output if pattern
matches one package, but do nothing when pattern matches no packages.
Thinking about it more, I think I would prefer it to be more consistent and fail if
pattern matches multiple or 0 packages, which can be done by changing:
    if len(pkgs) > 1 {
        fatalf("go build: cannot use -o with multiple packages")
    }
to:
    if len(pkgs) != 1 {
        fatalf("go build: cannot use -o with multiple packages")
    } else if len(pkgs) < 1 {
        fatalf("go build: cannot use -o with no packages")
    }
Or something along those lines.
That way, if someone has a script that executes `go build -o binary_name ...some_pkg`,
that command will only return success status if ./binary_name is successfully written,
and hence it's more reliable.

@dmitshur
Copy link
Contributor Author

dmitshur commented Jun 7, 2014

Comment 3:

The panic is very easy to fix, that panic stack trace points directly to the problem (`p
:= pkgs[0]` is done, but there's no check if `len(pkgs) == 0`).
However, I'm not sure what's a better correct behavior for `go build -o ./out
...does_not_exist`. There are two choices. Either a warning and no output like the other
commands.
Alternatively, maybe the best thing to do for `go build -o ./out ...does_not_exist` is
the same as for `go build -o ./out ...matches_multiple_pkgs`, which is:
fatalf("go build: cannot use -o with multiple packages")
It's a design decision of, if the user specifies -o option, should the command always
write _some_ output and fail with error if it can't (i.e. either matches multiple
packages, or no packages, or build error, etc.). Or should it write output if pattern
matches one package, but do nothing when pattern matches no packages.
Thinking about it more, I think I would prefer it to be more consistent and fail if
pattern matches multiple or 0 packages, which can be done by changing:
    if len(pkgs) > 1 {
        fatalf("go build: cannot use -o with multiple packages")
    }
to:
    if len(pkgs) > 1 {
        fatalf("go build: cannot use -o with multiple packages")
    } else if len(pkgs) < 1 {
        fatalf("go build: cannot use -o with no packages")
    }
Or something along those lines.
That way, if someone has a script that executes `go build -o binary_name ...some_pkg`,
that command will only return success status if ./binary_name is successfully written,
and hence it's more consistent and reliable.

@gopherbot
Copy link

Comment 4:

CL https://golang.org/cl/107140043 mentions this issue.

@adg
Copy link
Contributor

adg commented Jul 9, 2014

Comment 5:

This issue was closed by revision 3e80141.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
Fixes golang#8165.

After this change, the panic is replaced by a message:

        $ go build -o out ...doesntexist
        warning: "...doesntexist" matched no packages
        no packages to build

The motivation to return 1 exit error code is to allow -o flag
to be used to guarantee that the output binary is written to
when exit status is 0. If someone uses an import path pattern
to specify a single package and suddenly that matches no packages,
it's better to return exit code 1 instead of silently doing nothing.
This is consistent with the case when -o flag is given and multiple
packages are matched.
It's also somewhat consistent with the current behavior with the
panic, except that gave return code 2. But it's similar in
that it's also non-zero (indicating failure).
I've changed the language to be similar to output of go test
when an import path pattern matches no packages (it also has a return status of
1):

        $ go test ...doesntexist
        warning: "...doesntexist" matched no packages
        no packages to test

LGTM=adg
R=golang-codereviews, josharian, gobot, adg
CC=golang-codereviews
https://golang.org/cl/107140043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
Fixes golang#8165.

After this change, the panic is replaced by a message:

        $ go build -o out ...doesntexist
        warning: "...doesntexist" matched no packages
        no packages to build

The motivation to return 1 exit error code is to allow -o flag
to be used to guarantee that the output binary is written to
when exit status is 0. If someone uses an import path pattern
to specify a single package and suddenly that matches no packages,
it's better to return exit code 1 instead of silently doing nothing.
This is consistent with the case when -o flag is given and multiple
packages are matched.
It's also somewhat consistent with the current behavior with the
panic, except that gave return code 2. But it's similar in
that it's also non-zero (indicating failure).
I've changed the language to be similar to output of go test
when an import path pattern matches no packages (it also has a return status of
1):

        $ go test ...doesntexist
        warning: "...doesntexist" matched no packages
        no packages to test

LGTM=adg
R=golang-codereviews, josharian, gobot, adg
CC=golang-codereviews
https://golang.org/cl/107140043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants