can you give steps to show how to merge your latest code into thrift 0.8? #18

Open
AlexLuya opened this Issue Jun 26, 2012 · 18 comments

Comments

Projects
None yet
4 participants

It will save time of many user.And by the way,I tried to use "go install github.com/pomack/thrift4go/lib/go/thrift" to install this package,but get an error: can't load package: package github.com/pomack/thrift4go/lib/go/thrift: import "github.com/pomack/thrift4go/lib/go/thrift": cannot find package.Any mistakes I have made?

Owner

pomack commented Jun 26, 2012

Here is the one liner I use to download, compile, and install into my current directory's src and pkg:

GOPATH=pwd:$GOPATH go get github.com/pomack/thrift4go/lib/go/src/thrift

thanks,but how to merge your latest code into thrift 0.8

Owner

pomack commented Jun 26, 2012

The current files in the go1 branch are for use with thrift 0.8.
There are two files in the thrift4go/compiler/cpp (Makefile.am and t_go_generator.cc) that need to be updated in your local thrift repository.
You should also completely remove the lib/go from your thrift repository and copy/link to this thrift4go/lib/go

I replaced Makefile.am and t_go_generator.cc.
And replaced thrift-0.8.0/lib/go/thrift with thrift4go/lib/go/src/thrift,notice:not thrift4go/lib/go but thrift4go/lib/go/src/thrift,is this right?

then run ./configure,got

Building PHP Library ......... : no
Building Erlang Library ...... : yes

Building Go Library .......... : no (look at this line)

then replaced configure.ac,run ./configures again,same output as above

Owner

pomack commented Jun 26, 2012

With ./configure I'm pretty sure you need to use --with_go

My configure line at one point looked like:

./configure --prefix=/usr/local/encap/thrift-20110713 --with-boost=/usr/local/encap/boost-1.40.0 GOROOT=/Docs/extbuild/src/go-lang GOARCH=amd64 GOBIN=/usr/local/encap/go-release-branch.r58/bin PKG_CONFIG_PATH=/usr/local/bin/pkg-config --with-go --with-c_glib

do as your suggestion,same output,and try to check log with command line:more config.log|grep go,found this:

goinstall='/home/alex/go/bin/goinstall'

gomake='/home/alex/go/bin/gomake'

Any other thing should be edited?

Contributor

matttproud commented Jun 26, 2012

These may be useful to keep in mind:

Aalok, I'd be happy to work with you to get these mainlined, as a project I'm working on would benefit from this. Secondarily, I have a number of proposed generated Go code changes I want to make down the road, so the sooner any of this is mainlined, the better.

Maybe it'd be well for us to chat privately. Here's my e-mail: matt (dot) proud (at) gmail (dot) com

I am still not find a way to do merging,can you guys write a script to let me run it like this:./script_for_merge dir_of_latest_thrift4g0 dir_of_thrift0.8.I am not lazy,you know sometime,even to solve a simple problem,every user will spend two days,but creator will just take 15 minutes.

Even use configure.ac from:https://github.com/yanatan16/thrift4go,I still got:

..........................................
checking for gomake:/home/alex/go/gomake
checking for goinstall:/home/alex/go/goinstall

...........................................

,but I found:

GOGO=${GOBIN}/go
GOMAKE=${GOGO} build

GOINSTALL=${GOGO} install

I seems these lines don't take effect,I think this where problems comes from

AlexLuya commented Jul 2, 2012

I pretty sure it is because configure file error caused this problem,and found in your code base:

In configure.cc file,there are entries:

GOMAKE=${GOBIN}/gomake

GOINSTALL=${GOBIN}/goinstall

should be updated,but not,and in master/lib/go/Makefile,you did updating:

.......
install:
GOPATH=$(GOPATH) go install thrift
-/bin/cp -f pkg/$(GOOS)$(GOARCH)/thrift.* $(GOROOT)/pkg/$(GOOS)$(GOARCH)/

.........

Here goinstall has been updated to "go install",and if I install go r60 with "sudo apt-get install golang",then run ./configure without any other options,I got:Building Go Library .......... : yes.

I am also unable to build with current master.

Would it be possible to provide some basic instructions?

Contributor

matttproud commented Aug 6, 2012

Alec, since I was responsible for doing the rebasing of Aalok's work against Thrift 0.8.0, that responsibility would fall onto me, in part. Unfortunately I'm in the process of moving abroad (California -> Germany) right now this week. If you would be OK waiting about two weeks, I could provide a more detailed account. That said, here's what it involves in essence:

0.) Have Go version 1 or point release installed.
1.) Swapping Thrift's configure.ac with that which we have here.
2.) Replacing the compiler root of Thrift with what we have here. [Makefile.am may be out-of-sync. Please verify and report.]
3.) Replacing lib/Makefile.am from Thrift with what we have here.
4.) Deleting the lib/go directory of Thrift and replacing it totally with what we have here.
5.) Running the bootstrap command for Autotools such that configure and friends are regenerated.
6.) Run the newly-generated configure to compile and test.

Now, I am trying to remember whether I updated lib/go/Makefile.am to support automatic in-place library installation on one's system.

Hi Matt,

Thanks for the reply. I had done most of this before, and as you intimate, the problem seems to lie with compiler/cpp/Makefile.am being out of date:

compiler/cpp/Makefile.am:63: error: THRIFT_GEN_c_glib does not appear in AM_CONDITIONAL
compiler/cpp/Makefile.am:66: error: THRIFT_GEN_cpp does not appear in AM_CONDITIONAL
compiler/cpp/Makefile.am:69: error: THRIFT_GEN_java does not appear in AM_CONDITIONAL
...
Contributor

matttproud commented Aug 9, 2012

Prior to this move, I had been in the process of developing an integration test suite. It appears the importance hereof is self-evident now.

I kept the original Makefile.am and it seemed to work (ish).

I also needed to rename the thrifty.h include to thrifty.hh in thriftl.ll.

It still failed when installing the go library:

cp: -t: No such file or directory

I'm not sure how to fix that one without deep mojo.

Contributor

matttproud commented Aug 9, 2012

How about you make a clone of MASTER and replace our Makefile.am with Thrift 0.8.0's?

I might caution: I would be careful about the .h -> .hh. That could be vendor specific behavior.

Contributor

matttproud commented Aug 12, 2012

Until it is merged, consider giving #29 a look. :-) I just arrived in Europe and spent a jetlagged hour getting this fixed.

Owner

pomack commented Aug 12, 2012

This is merged now. Thanks Matt!

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