Setting_test.go failure in windows

Grafana Plugin Development
1

I’m seeing an error with the built-in test script for the setting module - but only under windows. It seems to be related to the forward-slash/back-slash ‘feature’ of windows, but so far, I haven’t been able to get to the exact root.

Specifically, running go run build.go test under linux works perfectly, but under windows, gives these 5 errors:
…x.xxx.
Failures:

  * C:/Users/mdmoo/go/src/github.com/grafana/grafana/pkg/setting/setting_test.go
  Line 60:
  Expected: '/tmp/data'
  Actual:   '..\..\tmp\data'
  (Should be equal)

  * C:/Users/mdmoo/go/src/github.com/grafana/grafana/pkg/setting/setting_test.go
  Line 83:
  Expected: '/tmp/override'
  Actual:   '..\..\tmp\override'
  (Should be equal)

  * C:/Users/mdmoo/go/src/github.com/grafana/grafana/pkg/setting/setting_test.go
  Line 93:
  Expected: '/tmp/data'
  Actual:   '..\..\tmp\data'
  (Should be equal)

  * C:/Users/mdmoo/go/src/github.com/grafana/grafana/pkg/setting/setting_test.go
  Line 103:
  Expected: '/tmp/env_override'
  Actual:   '..\..\tmp\env_override'
  (Should be equal)


16 total assertions

--- FAIL: TestLoadingSettings (0.24s)
FAIL
FAIL    github.com/grafana/grafana/pkg/setting  0.381s
?       github.com/grafana/grafana/pkg/social   [no test files]
ok      github.com/grafana/grafana/pkg/tsdb     0.473s
ok      github.com/grafana/grafana/pkg/tsdb/graphite    0.101s
ok      github.com/grafana/grafana/pkg/tsdb/influxdb    0.167s
ok      github.com/grafana/grafana/pkg/tsdb/mqe 0.100s
ok      github.com/grafana/grafana/pkg/tsdb/mysql       0.125s
ok      github.com/grafana/grafana/pkg/tsdb/opentsdb    0.058s
ok      github.com/grafana/grafana/pkg/tsdb/prometheus  0.052s
ok      github.com/grafana/grafana/pkg/util     0.126s
exit status 1
exit status 1

I have a clue though. The problem seems to be that setting.go resolves the paths by calling makeAbsolute() which prepends the homepath. Hence the “…/…/” Why this doesn’t do the same thing under linux is a mystery.

The second issue is the translation from forward-slashes to back-slashes, which seems to imply that the file directories are being resolved by the OS, resetting the setting property. I don’t see where this happens though.

2

Nice catch. Looks like we only run the frontend tests for our Windows CI build. As we none of the core team develop on Windows (go development was just too painful before Windows 10), we would never have seen this.

3

On a related theme - the windows versions of
go run build.go test
go run build.go package
go run build.go pkg-rpm
go run build.go pkg-deb

all fail with a message like this:

C:\Users\mdmoo\go\src\github.com\grafana\grafana>go run build.go package
Version: 4.4.0-pre1, Linux Version: 4.4.0, Package Iteration:
1497739176pre1
grunt([release --pkgVer=4.4.0-1497739176pre1])
./node_modules/.bin/grunt release --pkgVer=4.4.0-1497739176pre1
‘.’ is not recognized as an internal or external command,
operable program or batch file.
exit status 1
exit status 1

The problem is in the grunt function in build.go (line 347) and once again
involves forward-slashes and back-slashes.
Apparently, while you can windows understands unix-style forward-slashes
while accessing files, it does not do so when parsing executable paths
(weird, but apparently true)

The fix is to switch the path prefix depending on goos, like this:

func grunt(params …string) {

if goos == “windows” {
runPrint(“.\node_modules\.bin\grunt”, params…)
} else {
runPrint(“./node_modules/.bin/grunt”, params…)
}
}

This seems a bit trivial to put in a pull request.

Of course, there are still other stumbling blocks to making the go run
build.go package process work properly under windows, and maybe this was
never intended to work this way in the first place. For the record,
'package" in build.go calls createLinuxPackages, which ultimately calls
createPackage which calls runPrint 6 times to “mkdir”. RunPrint, as I’m
sure you know is a wrapper around exec.Command Excuse my ignorance of go,
but the source for exec.go says is doesn’t work for windows. There is a
exec_windows.go package, but this is not installed in a default regular
windows go installation and I can’t figure out how to include it. Even if
this could be solved, the bigger problem is that the ‘mkdir’ command is
built into cmd.exe; there is no mkdir.exe for exec to find.

I started down this path, imagining that go run build.go package was going
to create an installable package for windows, but now I look more carefully

  • I don’t think this does anyway. How are windows grafana installation
    packages built?
4

Update - I see the zip files are being created in the Dist folder - apparently part of the grunt release call inside go run build.go package - before building the LinuxPackages. This is so cool - and maybe should be documented somewhere. :slight_smile:

How it all works is still a mystery. Something to do with the Uglify step, I think.

5

Ah yes, I see tasks\distribute_task.js, registering the compress:release build-post-process, together with compress.js under tasks\options - this starts to make sense.

I still think the fix to the grunt() function in build.go is necessary to make the windows build work to this point.

6

May I ask why you are trying to build the linux packages on Windows? We do not use the go run build.go package command to generate the Windows build.

I just setup Grafana on Windows myself and I’ll update the docs with how to install golang with the correct path (it has changed a little in go 1.8) and how to install node-gyp dependencies without getting the python path error. I will also fix the failing tests (or the code if it is a bug) in the settings package.

1 Like
7

You’re right - I don’t need to create linux packages under windows.

But we still need to call go run build.go package, even for windows. The
windows package zip file gets created by the
grunt(gruntBuildArg(“release”)…) call before the createLinuxPackages()
step.

and for this call to work at all, we need to modify the grunt interface as
I mentioned before:

func grunt(params …string) {

if goos == “windows” {
runPrint(“.\node_modules\.bin\grunt”, params…)
} else {
runPrint(“./node_modules/.bin/grunt”, params…)
}
}

Since the createLinuxPackages step fails under windows (and is not
necessary or easily fixed), it would make sense to make this step
conditional, like this:

case “package”:
grunt(gruntBuildArg(“release”)…)
if goos != “windows” {
createLinuxPackages()
}

This works under both Linux and windows, without any error messages :slight_smile:

Or am I missing something…

8

We can absolutely make this change.

At the moment, we just run grunt release and zip the files. Here is how the appveyor (the service we use to test our windows builds) does it: https://github.com/grafana/grafana/blob/master/appveyor.yml#L30-L39

9

Yes, but you will need

func grunt(params …string) {

if goos == “windows” {
runPrint(“.\node_modules\.bin\grunt”, params…)
} else {
runPrint(“./node_modules/.bin/grunt”, params…)
}
}

too.

10

Yes, having it working locally. Will push to master soon.

Just curious but why are making a windows build? Is it something to do with the singlestat enhancement?

11

Yes, more or less. As you suggested, the enhancements I made to Singlestat
would be better turned into a stand-alone panel. This makes perfect sense,
though the more I dig into this, the more I appreciate how complex the
SingleStat panel actually is.

In the very short term, I wanted to ship a demo version based on the
enhanced built-in singlestat so my client could try it out, and the obvious
way to do this is to distribute a full windows grafana build. Thanks to
your help, this is now done.

As I said before, I’m learning about Node/Angular/Golang/Grunt/NPM
development as I’m doing this, so the process is quite a learning
experience. I’m making progress though.

I started by doing a simple copy-paste of singlestat from
C:\Users\mdmoo\go\src\github.com
\grafana\grafana\public\app\plugins\panel\singlestat
to
C:\Users\mdmoo\go\src\github.com
\grafana\grafana\data\plugins\annunciator-panel
then renaming all the references to SingleStat etc.
I ran into (to me) surmountable problems with the typescape compilation -
lots of problems with relative paths in included files and apparent version
conflicts (rxjs.Observable.d.ts expects a parameter etc…).
I noticed that none of the contributed panels use typescape, so maybe I’m
not alone in this. Nevermind, I now plan on making the panel from scratch
directly in javascript and angular.

I’ll let you know how this turns out.

Regards, and thanks again for the help

12

Here is my branch for the windows changes which I will merge to master in a while. Just need to test it on Linux as well as Windows first. https://github.com/grafana/grafana/tree/windows_build

13

Tested - as far as I can see, this works perfectly. :slight_smile: