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

bash escaping #4

Closed
eternal-flame-AD opened this issue Mar 2, 2019 · 11 comments
Closed

bash escaping #4

eternal-flame-AD opened this issue Mar 2, 2019 · 11 comments

Comments

@eternal-flame-AD
Copy link
Member

make GOTIFY_VERSION="$TARGET" FILE_SUFFIX="(for-gotify-$TARGET)" build;

This parenthesis should be escaped. See https://travis-ci.com/eternal-flame-AD/gotify-netlify/builds/102894043#L611

@jmattheis
Copy link
Member

Fixed this, the build plugin sadly doesn't work with the gotify/server binary:

λ  ~/src/playground/test  ./gotify-linux-amd64 
Starting Gotify version 2.0.0@2019-03-01-22:25:25
panic: error while loading plugin myplugin-linux-amd64-for-gotify-v2.0.0.so: plugin.Open("data/plugins/myplugin-linux-amd64-for-gotify-v2.0.0"): plugin was built with a different version of package github.com/gin-contrib/sse

goroutine 1 [running]:
github.com/gotify/server/router.Create(0xc000010640, 0xc00009c240, 0xc00017a000, 0xe, 0xc0000ae568)
	/proj/router/router.go:46 +0x3038
main.main()

Looking at the go.sum file both this plugin and gotify/server use:

github.com/gin-contrib/sse v0.0.0-20170109093832-22d885f9ecc7

@eternal-flame-AD Do you have any idea why it is like this?

@eternal-flame-AD
Copy link
Member Author

Yeah, the built package hash did not match:( It did not occur last time, please allow me some time to investigate to this.

@eternal-flame-AD
Copy link
Member Author

eternal-flame-AD commented Mar 2, 2019

I also hit this problem:(

I found that the plugin hash is different when built with -mod=vendor. This flag is added after merging the plugin PR so it did not happen during testing.

The following is a proof of concept:
In the plugin repo:

go build -buildmode=plugin

objdump -t -T gotify-netlify.so| grep go.link.pkghashbytes.github.com/gin-contrib/sse
0000000000c0b500 g    DO .rodata        0000000000000028  Base        go.link.pkghashbytes.github.com/gin-contrib/sse

readelf -x .rodata gotify-netlify.so| grep c0b500                                    
  0x00c0b500 64623739 64333338 35643238 37353664 db79d3385d28756d

In the server repo:

go build

objdump -t -T server| grep go.link.pkghashbytes.github.com/gin-contrib/sse
0000000001402c80 g     O .rodata        0000000000000028              go.link.pkghashbytes.github.com/gin-contrib/sse
0000000001402c80 g    DO .rodata        0000000000000028  Base        go.link.pkghashbytes.github.com/gin-contrib/sse

readelf -x .rodata server| grep 1402c80                                      
  0x01402c80 64623739 64333338 35643238 37353664 db79d3385d28756d
go build -mod=vendor 

objdump -t -T server| grep go.link.pkghashbytes.github.com/gin-contrib/sse 
0000000001402c80 g     O .rodata        0000000000000028              go.link.pkghashbytes.github.com/gin-contrib/sse
0000000001402c80 g    DO .rodata        0000000000000028  Base        go.link.pkghashbytes.github.com/gin-contrib/sse

readelf -x .rodata server| grep 1402c80    
  0x01402c80 30643132 38613663 30376338 66303361 0d128a6c07c8f03a

In addition:
adding also mod=vendor to the plugin repo doesn't seem to help, it seems that vendoring dependencies completely messed up the package hash

@eternal-flame-AD
Copy link
Member Author

I will add write a tool to add the package hash comparison to the CI process.

@jmattheis
Copy link
Member

This seems like a bug in Go or?

@eternal-flame-AD
Copy link
Member Author

I may need to look into the source code to take a look at how the package was hashed to see whether this was intentional. Please allow me some time to do this.

@eternal-flame-AD
Copy link
Member Author

Okay I have finally dug out the whole chain of events that caused the different package hash.

The package hash is generated in https://github.com/golang/go/blob/master/src/cmd/link/internal/ld/lib.go#L771 . Reading that part of the code we know that the hash is generated by hashing the first line and something between two double dollar signs.

Then I compared the compilation result of the .a files with -mod=vendor and without that flag. Hashed parts are highlighted. It is easy to spot the difference: the name of the vendor directory is included:(
image
image

So you think this is a bug that should be submitted to golang?

@eternal-flame-AD
Copy link
Member Author

Now everything makes sense. Adding mod=vendor both to the server and plugin will not solve the problem because the package path on the filesystem would be different. Only by using gomod or the traditional GOPATH mode that package path will be the same.

@jmattheis
Copy link
Member

jmattheis commented Mar 2, 2019

This sounds like a bug, could you add an issue for this on golang github?

Thanks for investigating this issue!

I added mod vendor mostly to prevent 502 errors https://travis-ci.org/gotify/server/builds/500530056#L3188 I guess it is because the build downloads the dependencies 5 times.

@jmattheis
Copy link
Member

Maybe we can mount the gopath in the docker image then it probably caches this too.

@eternal-flame-AD
Copy link
Member Author

Downloading dependencies for so many times does not look elegant. But I think the 502 is maybe just a temporary error, in the plugin template I downloaded dependencies 6 times and no 502 occurred in ~3 subsequent builds.

Still it worth trying mounting gopath/mod to prevent downloading dependencies repeatedly.

jmattheis added a commit to gotify/server that referenced this issue Mar 2, 2019
See gotify/plugin-template#4
tl;dr: Using vendor makes plugins incompatible.
jmattheis added a commit to gotify/server that referenced this issue Mar 2, 2019
See gotify/plugin-template#4
tl;dr: Using vendor makes plugins incompatible.
jmattheis added a commit to gotify/server that referenced this issue Mar 2, 2019
See gotify/plugin-template#4
tl;dr: Using vendor makes plugins incompatible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants