Closed willfaught closed 8 years ago
Thanks @willfaught! Just one comment and lgtm.
This still seems to not work for some reason, just wanted to put up what I was thinking. This produces a GOPATH of /Users/Will/Developer/go/Users/Will/Developer/go/src/github.com/willfaught/antlr4/runtime-testsuite/target/classes/Go/src/github.com/willfaught/antlr4/runtime/Go/antlr
, which isn't right. The path separator doesn't seem to show up.
(Updated)
It doesn't work because of our go import path. Peter's old system used runtimePath
which would yield $CWD/runtime-testsuite/target/classes/Go
and below that it had the subdir src/antlr4
with the runtime sources, hence go could find the import path antlr4
there.
Now we use a complete import path, such that we have to put the whole repository under our $GOPATH/src/github.com/pboyer
. That makes runtime
obsolete, but now we messed up automated builds (with $GOPATH unset), because only go developers use go-specific project directory layouts.
As of now I am happy with that, but we need a better idea. :)
Whoops, misread the Javadoc. Using File.pathSeparator worked.
That makes runtime obsolete, but now we messed up automated builds (with $GOPATH unset), because only go developers use go-specific project directory layouts.
Can you give more detail? When/where are these builds happening with GOPATH unset?
One example, when a build w/o GOPATH happens is travis (see also #12), or whenever a non-go-developer tries this at home.
This is an example error:
dir /tmp/TestCompositeParsers-1464374124472/parser
exec stderrVacuum: parser/m_base_listener.go:4:8: cannot find package "github.com/pboyer/antlr4/runtime/Go/antlr" in any of:
/opt/go/src/github.com/pboyer/antlr4/runtime/Go/antlr (from $GOROOT)
/home/wjk/go/antlr/src/github.com/pboyer/antlr4/runtime-testsuite/target/classes/Go/src/github.com/pboyer/antlr4/runtime/Go/antlr (from $GOPATH)
/tmp/TestCompositeParsers-1464374124472/src/github.com/pboyer/antlr4/runtime/Go/antlr
Not sure what exec stderrVacuum
is doing, but can we change it to set GOPATH?
The tests use readers (stdoutVacuum
and stderrVacuum
that read STDERR and STDOUT in order to assert/compare each output with expected outputs. Because go
complains, there is unexpected stuff in stderrVacuum.
The spot where you did the null
fix is exactly where we can set the GOPATH
as desired. The thing is, when the "external" GOPATH
is empty and the user is a go developer, then s/he should set GOPATH
correctly. When the user is any non-go developer or the travis build host, there is no meaningful value that we can set. At least, I have no idea what to use. Yet.
My take is, this is a problem that needs to be addressed, but it is not related to this PR. It just became apparent at this point.
Ah, I think I see the issue. Basically, for all users, we can't ensure the repo appears in a structure like .../src/github.com/antlr/antlr4 such that we can just set GOPATH=...
I think it's reasonable that if you want to compile/test the Go target, you need Go installed and configured correctly. I wouldn't expect the C# target to work if I didn't have it installed.
Yes, that's right.
Having go installed is not enough. It's also mandatory that the the git checkout is already inside a go-eligible directory. I think that is a hurdle that we should remove somehow. At least I have no idea how this could work with travis otherwise.
In the case of Travis, we can set GOPATH to a temporary directory then go get the antlr4 repo. Or symlink the existing runtime lib into it.
On May 27, 2016, at 11:48 AM, Wolfgang Johannes Kohnen notifications@github.com wrote:
Yes, that's right.
Having go installed is not enough. It also mandatory that the the git checkout is already inside a go-eligible directory. I think that is a hurdle that we should remove somehow. At least I have no idea how this could work with travis otherwise.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
@pboyer I think your point has been addressed. As @wjkohnen said, enabling Travis builds is a separate issue.