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

go-build: bug if there are multiple packages in a directory #676

Closed
phst opened this issue Jun 30, 2015 · 13 comments

Comments

@phst
Copy link
Contributor

commented Jun 30, 2015

If a directory contains files belonging to more than one package (e.g. one file package a and one file package b), the go-build checker barfs:

Suspicious state from syntax checker go-build: Checker go-build returned non-zero exit code 1, but no errors from output: can't load package: package .: found packages a.go (a) and b.go (b) in /private/tmp

Checker definition probably flawed.
@phst

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2015

Actually I think that instead of adding a new error pattern the checker command could just be replaced by

("go" "build" "-o" temporary-file-name "--" source)

to compile only a single file.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2015

@phst I do not know go, but I was under the impression that there is just one go package per directory. Is it common for multiple go packages to be in the same directory?

@phst

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2015

It's not common, but possible. Flycheck might either choose to not support this use case (then the error pattern should be fixed to detect the situation), or support it (then the go tool requires an explicit file name argument).

@phst

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2015

However, my suggestion wouldn't work out of the box if the package consists of more than one file. Maybe just give up if a directory with multiple packages is detected? Otherwise this would require parsing all Go files in the current directory to find out to which package they belong.

@lunaryorn lunaryorn added this to the community backlog milestone Jul 6, 2015

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2015

@phst I think I'm the wrong person to discuss. I can't contribute any ideas, because I do not know Go, and I'll not fix this issue by myself, because fixing a corner case for a language that I neither know nor intent to ever use is not a priority for me. Whatever the fix turns out to be, you or someone else has to come up with a pull request or patch.

All that I can do is to set the basic requirements for such a patch: It must not break anything that Flycheck already supports, and it should not be overly complex.

In other words, if giving an explicit file name argument to go build breaks packages consisting of multiple files, then that's not an acceptable fix for me, and if you need a 100 lines of Lisp code to find out how to check the current buffer, then that's not acceptable either.

I think, it'll probably come down to adding a new error pattern or a specific :error-parser: to deal with this situation, and just error out in this case.

@lunaryorn lunaryorn added the kind: bug label Jul 6, 2015

@phst

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2015

Fair enough, I can produce a pull request for this.

@phst

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2015

What I would actually prefer is to disable the Go checker in that case, e.g. by returning an empty error list. The error from go build doesn't represent an error in the source file being checked, but a limitation in the go tool itself.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2015

@phst You can't disable a syntax checker by returning an empty error list. Flycheck will still use the checker, and just not report any errors. I don't think that this is a good solution, because it suggests that the buffer is free from errors when it actually might not be. I'd rather prefer to have an error that explicitly tells the user what's going on.

@phst

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2015

OK, but I think this should not be an error, rather a warning or an info message. (I think an error should be reserved for cases where the code is really known to be broken, not just unhandleable by the tool.)

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2015

OTOH, it's a situation that prevents the tool from working properly, i.e. an error condition, but I'm fine with either level. We just should not silently fail.

@phst

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2015

Just a quick update, I haven't forgotten about this, and I've started to create a patch, but I have little time right now, so this might take a while.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2015

@phst Take all the time you need, I'm in no hurry. Please feel free to ask further question if there's anything unclear, or if you need any help with Flycheck.

Thanks for your contribution and for your support!

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2015

@phst I'll close this issue as wontfix meanwhile, because I for my part will not actively work on it. It doesn't stand against a PR, so if you open one with a fix, I'll be happy to review and merge it.

@lunaryorn lunaryorn closed this Aug 1, 2015

@lunaryorn lunaryorn added status: wontfix and removed blocked labels Aug 1, 2015

phst added a commit to phst/flycheck that referenced this issue Mar 8, 2016

Catch error message for multiple Go packages
If there are multiple Go packages in a single directory, `go build` will
bail out.  Catch the error message printed by the tool and turn it into
an informational message because Flycheck can’t really syntax-check such
packages.

Because the error message doesn’t contain a line number, install an
error filter to fake a line number.  Otherwise the message would be
ignored.

Fixes flycheckGH-676 and closes flycheckGH-676.

phst added a commit to phst/flycheck that referenced this issue Mar 12, 2016

Catch error message for multiple Go packages
If there are multiple Go packages in a single directory, `go build` will
bail out.  Catch the error message printed by the tool and turn it into
an informational message because Flycheck can’t really syntax-check such
packages.

Because the error message doesn’t contain a line number, install an
error filter to fake a line number.  Otherwise the message would be
ignored.

Fixes flycheckGH-676.

lunaryorn added a commit that referenced this issue Mar 17, 2016

Catch error message for multiple Go packages
If there are multiple Go packages in a single directory, `go build` will
bail out.  Catch the error message printed by the tool and turn it into
an informational message because Flycheck can’t really syntax-check such
packages.

Because the error message doesn’t contain a line number, install an
error filter to fake a line number.  Otherwise the message would be
ignored.

Fixes GH-676 and closes GH-904
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.