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

Build using gb #28

Closed
leifmadsen opened this issue Nov 6, 2015 · 35 comments
Closed

Build using gb #28

leifmadsen opened this issue Nov 6, 2015 · 35 comments

Comments

@leifmadsen
Copy link

This is a feature request / enhancement (and I do plan to look into it and see about submitting a pull request), but the current way of building is slightly awkward. I think it might be better to install this through the gb tool to handle dependency gathering.

https://github.com/constabulary/gb

@nati
Copy link
Contributor

nati commented Nov 7, 2015

Hi

Thank you for your suggestion!
We will very welcome your PR :)

@leifmadsen
Copy link
Author

I will do my best to work on it this week :)

@leifmadsen
Copy link
Author

Started work on this, it not compiles via GB and I confirm that the resulting binary starts :)

I have to go get ready for a dinner date, but I'll play some more tonight or tomorrow and get a pull request up for review!

@nati
Copy link
Contributor

nati commented Nov 7, 2015

👍

@leifmadsen
Copy link
Author

Hey @nati could you create a new branch against master that I can create a pull request to? This is a fairly invasive change, and while it works right now, there is still work to be done on the documentation, wercker.yml and such.

In order to do this in steps, it would be great to have a development or separate "implements_issue_28" branch or something. That way I can open the pull request against that rather than master for now. Thanks!

@nati
Copy link
Contributor

nati commented Nov 9, 2015

Thanks. please use this one
https://github.com/cloudwan/gohan/tree/implements_issue_28

@leifmadsen
Copy link
Author

Thanks! Pull request 29 (#29) is now open for this issue.

nati added a commit that referenced this issue Nov 11, 2015
@nati
Copy link
Contributor

nati commented Nov 11, 2015

ok three commits get merged
https://github.com/cloudwan/gohan/tree/implements_issue_28

TODO

(1) how to get code coverage
(2) how to pass ginkgo param for gb test
(3) cross compile
(4) remove items

@leifmadsen
Copy link
Author

@nati do you have the source of the Dockerfile for the nati/gohan_builder image? I found this: https://hub.docker.com/r/nati/gohan_builder/

But there is no Dockerfile attached. I'm going to try and fix the wercker builds for code coverage.

@leifmadsen
Copy link
Author

@nati I found something about Go 1.5 that has experimental vendoring now. I'm wondering if that actually makes a lot more sense to use, as

  1. it's likely to become a more defacto way of dealing with vendoring and
  2. it likely makes this a lot simpler to implement, rather than refactoring a whole bunch of things, like ginko, wercker, etc

I'm playing around now, and might make a separate pull request if I get it all going. It seems to be pretty straight forward.

@nati
Copy link
Contributor

nati commented Nov 11, 2015

Hi!

[1] Docker file
Sorry, i didn't created a docker file for that Dockerfile
It is basically docker xgo image + some local modification (remove android build because it isn't working on werker).
https://github.com/karalabe/xgo

You need build this image in your local because of issue #30.
karalabe/xgo#30

Because of we are using sqlite3, it is little bit hard to cross compile rather than pure go project.

[2] Vendoring
Yes.
I was thinking to use that vendoring after it graduated from experimental phase.
However, let use it if it is already useful and solves our issue :)

Note that xgo isn't supporting it yet. we may need modify xgo shell script by ourselves.
karalabe/xgo#31

@leifmadsen
Copy link
Author

@nati I spent all afternoon on the built in vendoring tool, and I'm highly annoyed by it :) Basically you use glide, but the way glide works, is that you have a glide.yaml file, which is fine (similar to Godeps) and then it imports into the vendor/ directory. All is fine there.

But the issue is that you can't add the vendor/ contents into your git repo (like you can with gb) because they are all git clones. This means that we now have to rely on an external tool like glide to get all the deps.

@leifmadsen
Copy link
Author

@nati per [1], do you think you could update the Docker image then to have 'gb' installed and made available in the $PATH? It should just require you to load the image, run:

go get github.com/constabulary/gb/...

and then make sure gb is available to be run via $PATH.

Once you do that, I assume previously you just applied your change to the image, and pushed it up. Maybe you could do the same, and then I can start working on the wercker.yml and get more code coverage back.

So far I have gb test running and working fine.

@nati
Copy link
Contributor

nati commented Nov 11, 2015

I got it!

I found a way to use existing tool also.

That's a good question. For most you should be able to emulate GOPATH with

    GOPATH=$PROJECT:$PROJECT/vendor godoc

constabulary/gb#42

I'll try this way

@leifmadsen
Copy link
Author

@nati disregard for now on the Docker image update. I have an idea!

@nati
Copy link
Contributor

nati commented Nov 11, 2015

Thanks.
the PR, right?

@leifmadsen
Copy link
Author

@nati yea I'm working through getting wercker going again with gb and no changes to the Docker image are required. I've gotten as far as the very last step in the wercker process (the gb test part) and I can't get it working.

I'm getting this issue:

2015-11-12 03:26:26.534211 N | etcdserver: added local member ce2a822cea30bfca [http://localhost:2380 http://localhost:7001] to cluster 7e27652122e8b2ae
FATAL: command "test" failed: failed to resolve import path "../../../go/src/github.com/cloudwan/gohan/src/db": "../../../go/src/github.com/cloudwan/gohan/src/db" is not a valid import path
2015-11-12 03:26:26.538940 N | osutil: received terminated signal, shutting down...
2015-11-12 03:26:26.539021 I | etcdserver: aborting publish because server is stopped
make: *** [test] Error 1

I'm not really sure why the import path is being pulled in like that. I have to work on something else for a bit, but when my co-worker gets back in later this morning we're going to see if we can fix that. Otherwise, we're getting pretty close I think.

@leifmadsen
Copy link
Author

Of course I'll submit documentation updates once everything is working so that we can get this merged down. I'll just leave those for the last PR.

@nati
Copy link
Contributor

nati commented Nov 12, 2015

I faced that issue too.
You need to add standdard gopath in gopath.

like GOPATH=$GOPATH:$PROJECT:$PROJECT/vendor

@leifmadsen
Copy link
Author

@nati thanks, going to try again with just the standard go test tool with that little trick and see what happens.

@leifmadsen
Copy link
Author

@nati ok latest change to my PR has wercker mostly working, but it's failing on the actual tests being run. I'm not sure if this is because the run_test is incorrect still, or there are legitimate tests failing?

Can you look at the result of this run and see if you have any ideas?

https://app.wercker.com/#buildstep/5644e211282b527a6b1e2750

@nati
Copy link
Contributor

nati commented Nov 12, 2015

Thanks! I'll fix that issue

@nati
Copy link
Contributor

nati commented Nov 12, 2015

OK build works.
Now cross compiling is only TODO :)

@leifmadsen
Copy link
Author

@nati NICE! Where does the cross-compiling happen?

@nati
Copy link
Contributor

nati commented Nov 13, 2015

yeah. xgo is highly depends on "go get" ...

@leifmadsen
Copy link
Author

@nati you can do cross compile with gb as well. I haven't done it before, but we might be able to switch xgo for gb commands. I just haven't been able to find where you're doing that...

@leifmadsen
Copy link
Author

BTW, when I say gb can cross-compile, I mean that Go 1.5 can cross-compile, and we can just use gb with a couple simple flags to do the same:

GOOS=linux GOARCH=arm gb build

@leifmadsen
Copy link
Author

I found where the script lives; it's on the Docker image directly. Just going through it, and it looks like we might be able to tweak that build.sh perhaps and then push the changes back up to hub.docker.com.

@nati
Copy link
Contributor

nati commented Nov 13, 2015

Thanks!
I'll try to tweak build.sh.

@nati
Copy link
Contributor

nati commented Nov 13, 2015

Aha.. I found we don't need xgo cross compile. gb simply takes care of it!
I could compile gohan even if in my local. awesome.

I'll push another code soon.

@nati
Copy link
Contributor

nati commented Nov 13, 2015

it looks like example app stop working with this change. Let me find a way to use gohan as library

@nati
Copy link
Contributor

nati commented Nov 13, 2015

hmm it looks like there is no nice way to use gb project as a library.
constabulary/gb#49

We would like to keep support to go based extension without modifying gohan mainline..

@leifmadsen do you have any good idea?
may be, we can try experimental vendor support

@leifmadsen
Copy link
Author

@nati can you elaborate on what you're looking for? I'm not sure I quite follow where gohan is getting used as a library?

I tried out the native vendor support but wasn't really all that impressed. Perhaps we could do experimental vendor support in combination with the Godeps stuff you already had.

If gb isn't going to work, then I think there are still some documentation and other updates that could make building the project much easier. That was the primary motivation here, since both my colleague and I found building gohan to be very tricky. The purpose here was simply to make it less tricky.

Thoughts?

@nati
Copy link
Contributor

nati commented Nov 17, 2015

@leifmadsen

This is a example for lib mode.
https://github.com/cloudwan/gohan/blob/master/exampleapp/exampleapp.go
You can add your custom route, and extensions implemented by go.

Could you also let us what problems you faced?
This one won't work for you?
http://gohan.cloudwan.io/gohan/installation.html#install-from-source

@leifmadsen
Copy link
Author

I've opened a PR for documentation updates, and I can continue to refine the documentation there. For me, simply running a go get github.com/cloudwan/gohan results in a binary as I would expect for a Go project, so we're good here.

I'm going to close this out and will continue to work against master with documentation updates, and file any other bugs against the recommended build method. Thanks!

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

No branches or pull requests

2 participants