Skip to content

Make default DataFile work with GOPATH with multiple entries.#132

Open
vadimsht wants to merge 1 commit intogoogle:masterfrom
vadimsht:datafile
Open

Make default DataFile work with GOPATH with multiple entries.#132
vadimsht wants to merge 1 commit intogoogle:masterfrom
vadimsht:datafile

Conversation

@vadimsht
Copy link
Copy Markdown

@vadimsht vadimsht commented Feb 1, 2019

It's not a big deal if this pull request is rejected, since DataFile is overridable and I can override it in my projects instead (it uses multiple GOPATHs).


func BenchmarkParse(b *testing.B) {
filename := dataFile("syntax", "testdata/scan.star")
filename := starlarktest.DataFile("syntax", "testdata/scan.star")
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is already used by this file in TestParseErrors above.

@josharian
Copy link
Copy Markdown
Collaborator

I don't understand why this routine is necessary at all. By default, go test runs executables with the cwd set to its package path, so you should be able to simply load the relative path testdata/something.star and have it work.

@alandonovan
Copy link
Copy Markdown
Contributor

go is not the only build system.

@vadimsht
Copy link
Copy Markdown
Author

vadimsht commented Feb 1, 2019

I can explain why I'm sending this: my project uses starlarktest package, including assert.star it has. Starlark-go code is "vendored" into parallel GOPATH (i.e. all my sources are in /.../go and all third party code, including starlark-go, are in automatically managed /.../go/.vendor). This results in build.Default.GOPATH being literally /.../go;/.../go/.vendor which confused DataFile.

@mcuadros
Copy link
Copy Markdown

Since the introduction of the go modules, now this is even more complex since is really hard found the path where is the source code. I believe that the easiest way to solve this is embed the source code of assert.star in Go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants