Thrift
  1. Thrift
  2. THRIFT-1600

Thrift Go Compiler and Library out of date with Go 1 Release.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.9.1
    • Component/s: Go - Compiler, Go - Library
    • Labels:
      None

      Description

      Go 1 is a major release of Go that will be stable in the long term. Read the Go 1 Release Notes for more information.
      http://golang.org/doc/go1.html

      The current (trunk) compiler and libraries are not compatible with Go1, because of significant changes in the language.

      The pomack/thrift4go seems a valid patch, but doesn't seem to be pulled into Apache Thrift. Possibly merge the development into one place? https://github.com/pomack/thrift4go

      1. partial.diff
        2 kB
        Atul S Vasu
      2. thrift-1600-go1-support.patch.gz
        118 kB
        Jed Smith

        Issue Links

          Activity

          Hide
          Jake Farrell added a comment -

          Please see http://thrift.apache.org/docs/HowToContribute/ for submitting patches

          Show
          Jake Farrell added a comment - Please see http://thrift.apache.org/docs/HowToContribute/ for submitting patches
          Hide
          Atul S Vasu added a comment -

          Several namespace related issues are fixed in this CL. I am not sure if that will be enough to cover though. Just a starting point, I'll wait for pomack to provide a complete patch.

          In any case we need to wait for
          http://code.google.com/p/go/issues/detail?id=3607

          to get fixed in Go. Note that using reflection to get field numbers is a bad idea, so getting rid of that will prevent us from having to wait for it.

          Show
          Atul S Vasu added a comment - Several namespace related issues are fixed in this CL. I am not sure if that will be enough to cover though. Just a starting point, I'll wait for pomack to provide a complete patch. In any case we need to wait for http://code.google.com/p/go/issues/detail?id=3607 to get fixed in Go. Note that using reflection to get field numbers is a bad idea, so getting rid of that will prevent us from having to wait for it.
          Hide
          Matt T. Proud added a comment - - edited

          Hi Jake and Atul,

          I have a pull (https://github.com/apache/thrift/pull/24) request open on Github to up-integrate pomack's newer compiler and Golang library support. I plan on making some improvements to generated Go code, so it'd be nice to get this mainlined soon.

          • Matt
          Show
          Matt T. Proud added a comment - - edited Hi Jake and Atul, I have a pull ( https://github.com/apache/thrift/pull/24 ) request open on Github to up-integrate pomack's newer compiler and Golang library support. I plan on making some improvements to generated Go code, so it'd be nice to get this mainlined soon. Matt
          Hide
          Aalok Shah added a comment -

          Hi Jake, Atul, and Matt,

          I am pomack on github and I wasn't aware of this JIRA (don't subscribe). A few people have reached out to me separately and I've helped them get up and running individually. The version of Go in the Apache Thrift distribution is from roughly Go-r59 or r60

          https://github.com/pomack/thrift4go works for Go1 with the following known caveats:

          1) An empty thrift file can cause errors because Go will complain about an empty compiled package.
          2) Specifying services without any types/structs causes "import not used" error messages. The thrift4go compiler creates an empty file for types.
          3) Referring to functions/types from an imported thrift file doesn't scope the functions/types in the generated code. A workaround is to change statements from "import a" to "import . a" but that can cause naming conflicts. This requires proper thrift4go compiler support.
          4) Generated Makefiles still use pre-Go1 Makefiles that need to be updated to use Go1 standards
          5) Always generates packages under directory thriftlib/

          I didn't want to push my changes out to the official thrift repository until these were resolved, but I don't have the time to work on these issues in the near future.

          Show
          Aalok Shah added a comment - Hi Jake, Atul, and Matt, I am pomack on github and I wasn't aware of this JIRA (don't subscribe). A few people have reached out to me separately and I've helped them get up and running individually. The version of Go in the Apache Thrift distribution is from roughly Go-r59 or r60 https://github.com/pomack/thrift4go works for Go1 with the following known caveats: 1) An empty thrift file can cause errors because Go will complain about an empty compiled package. 2) Specifying services without any types/structs causes "import not used" error messages. The thrift4go compiler creates an empty file for types. 3) Referring to functions/types from an imported thrift file doesn't scope the functions/types in the generated code. A workaround is to change statements from "import a" to "import . a" but that can cause naming conflicts. This requires proper thrift4go compiler support. 4) Generated Makefiles still use pre-Go1 Makefiles that need to be updated to use Go1 standards 5) Always generates packages under directory thriftlib/ I didn't want to push my changes out to the official thrift repository until these were resolved, but I don't have the time to work on these issues in the near future.
          Hide
          Matt T. Proud added a comment -

          Hi Aalok,

          I've dropped the pull request that I've cited above into Thrift proper, but I would like to see this mainlined as soon as reasonable. Would you be OK if I sent you a couple of fixes in the interim via pull requests into your branch? I'd like to help out however I can, but the last thing I'd want to do is step on your shoes. Please let me know how you'd like me to proceed.

          Cheers,

          Matt

          Show
          Matt T. Proud added a comment - Hi Aalok, I've dropped the pull request that I've cited above into Thrift proper, but I would like to see this mainlined as soon as reasonable. Would you be OK if I sent you a couple of fixes in the interim via pull requests into your branch? I'd like to help out however I can, but the last thing I'd want to do is step on your shoes. Please let me know how you'd like me to proceed. Cheers, Matt
          Hide
          Aalok Shah added a comment -

          Go ahead and send me the pull requests. I'm not currently working on it and would appreciate any help.

          Show
          Aalok Shah added a comment - Go ahead and send me the pull requests. I'm not currently working on it and would appreciate any help.
          Hide
          Jed Smith added a comment - - edited

          Hello,

          I have taken a stab at porting Aalok and Matt's Go 1 work to trunk, and it applied cleanly (by hand). I cleaned up the diff a lot because there were hundreds of lines whose whitespace was modified and which effectively created a no-op; I did not want to dirty the tree with all of that noise, with the exception of Go source which I know was run through gofmt, so I went ahead and pulled that in.

          There's good work here but it's being stagnated on Github, and I'm bummed out about that. I'm hoping to resurrect this bug and give it some life, potentially for 0.9.0 or a point release – Go 1 is pretty mature, now, and it'd be good to make Go a first-class language again for Thrift.

          M       configure.ac
          M       tutorial/go/src/GoServer.go
          M       tutorial/go/src/GoClient.go
          M       tutorial/go/src/main.go
          M       tutorial/go/src/CalculatorHandler.go
          M       tutorial/go/Make.deps
          M       tutorial/go/deps.bash
          M       tutorial/go/Makefile
          D       lib/go/thrift
          D       lib/go/thrift/tnonblocking_server.go
          D       lib/go/thrift/tjson_protocol_test.go
          D       lib/go/thrift/tjson_protocol.go
          D       lib/go/thrift/tset.go
          D       lib/go/thrift/texception_test.go
          D       lib/go/thrift/texception.go
          D       lib/go/thrift/tiostream_transport.go
          D       lib/go/thrift/_testmain.go
          D       lib/go/thrift/tprotocol_test.go
          D       lib/go/thrift/tnonblocking_server_socket.go
          D       lib/go/thrift/tmemory_buffer.go
          D       lib/go/thrift/tnumeric.go
          D       lib/go/thrift/tsimple_json_protocol_test.go
          D       lib/go/thrift/tbinary_protocol_test.go
          D       lib/go/thrift/tserver_test.go
          D       lib/go/thrift/tserver.go
          D       lib/go/thrift/tprocessor_factory.go
          D       lib/go/thrift/tcompact_protocol.go
          D       lib/go/thrift/tframed_transport_test.go
          D       lib/go/thrift/tframed_transport.go
          D       lib/go/thrift/tnonblocking_socket.go
          D       lib/go/thrift/thttp_client_test.go
          D       lib/go/thrift/tnonblocking_transport_test.go
          D       lib/go/thrift/tsimple_server.go
          D       lib/go/thrift/tprocessor.go
          D       lib/go/thrift/tapplication_exception_test.go
          D       lib/go/thrift/ttype.go
          D       lib/go/thrift/tcontainer.go
          D       lib/go/thrift/ttransport_factory.go
          D       lib/go/thrift/tstruct.go
          D       lib/go/thrift/Makefile
          D       lib/go/thrift/tprotocol_factory.go
          D       lib/go/thrift/tsocket.go
          D       lib/go/thrift/ttransport_test.go
          D       lib/go/thrift/ttransport.go
          D       lib/go/thrift/tiostream_transport_test.go
          D       lib/go/thrift/tmemory_buffer_test.go
          D       lib/go/thrift/tprotocol.go
          D       lib/go/thrift/tsimple_json_protocol.go
          D       lib/go/thrift/tbinary_protocol.go
          D       lib/go/thrift/tcompact_protocol_test.go
          D       lib/go/thrift/tbase.go
          D       lib/go/thrift/ttransport_exception.go
          D       lib/go/thrift/tlist.go
          D       lib/go/thrift/tmap.go
          D       lib/go/thrift/thttp_client.go
          D       lib/go/thrift/tserver_socket.go
          D       lib/go/thrift/tnonblocking_transport.go
          D       lib/go/thrift/tprotocol_exception.go
          D       lib/go/thrift/tapplication_exception.go
          D       lib/go/thrift/tserver_transport.go
          D       lib/go/thrift/tfield.go
          D       lib/go/thrift/tmessage.go
          D       lib/go/thrift/tmessagetype.go
          D       lib/go/thrift/tcompare.go
          A       lib/go/src
          A       lib/go/src/thrift
          A       lib/go/src/thrift/tnonblocking_server.go
          A       lib/go/src/thrift/tjson_protocol_test.go
          A       lib/go/src/thrift/tjson_protocol.go
          A       lib/go/src/thrift/tset.go
          A       lib/go/src/thrift/texception_test.go
          A       lib/go/src/thrift/texception.go
          A       lib/go/src/thrift/tiostream_transport.go
          A       lib/go/src/thrift/_testmain.go
          A       lib/go/src/thrift/tprotocol_test.go
          A       lib/go/src/thrift/tnonblocking_server_socket.go
          A       lib/go/src/thrift/tmemory_buffer.go
          A       lib/go/src/thrift/tnumeric.go
          A       lib/go/src/thrift/tsimple_json_protocol_test.go
          A       lib/go/src/thrift/tbinary_protocol_test.go
          A       lib/go/src/thrift/tserver_test.go
          A       lib/go/src/thrift/tserver.go
          A       lib/go/src/thrift/tprocessor_factory.go
          A       lib/go/src/thrift/tcompact_protocol.go
          A       lib/go/src/thrift/tframed_transport_test.go
          A       lib/go/src/thrift/tframed_transport.go
          A       lib/go/src/thrift/tnonblocking_socket.go
          A       lib/go/src/thrift/thttp_client_test.go
          A       lib/go/src/thrift/tmap_test.go
          A       lib/go/src/thrift/tnonblocking_transport_test.go
          A       lib/go/src/thrift/tsimple_server.go
          A       lib/go/src/thrift/tprocessor.go
          A       lib/go/src/thrift/tapplication_exception_test.go
          A       lib/go/src/thrift/ttype.go
          A       lib/go/src/thrift/tcontainer.go
          A       lib/go/src/thrift/ttransport_factory.go
          A       lib/go/src/thrift/tstruct.go
          A       lib/go/src/thrift/Makefile
          A       lib/go/src/thrift/tprotocol_factory.go
          A       lib/go/src/thrift/tsocket.go
          A       lib/go/src/thrift/ttransport_test.go
          A       lib/go/src/thrift/ttransport.go
          A       lib/go/src/thrift/tiostream_transport_test.go
          A       lib/go/src/thrift/tmemory_buffer_test.go
          A       lib/go/src/thrift/tprotocol.go
          A       lib/go/src/thrift/tsimple_json_protocol.go
          A       lib/go/src/thrift/tbinary_protocol.go
          A       lib/go/src/thrift/tcompact_protocol_test.go
          A       lib/go/src/thrift/tbase.go
          A       lib/go/src/thrift/ttransport_exception.go
          A       lib/go/src/thrift/tlist.go
          A       lib/go/src/thrift/tmap.go
          A       lib/go/src/thrift/thttp_client.go
          A       lib/go/src/thrift/tserver_socket.go
          A       lib/go/src/thrift/tnonblocking_transport.go
          A       lib/go/src/thrift/tprotocol_exception.go
          A       lib/go/src/thrift/tapplication_exception.go
          A       lib/go/src/thrift/tserver_transport.go
          A       lib/go/src/thrift/tfield.go
          A       lib/go/src/thrift/tmessage.go
          A       lib/go/src/thrift/tmessagetype.go
          A       lib/go/src/thrift/tcompare.go
          A       lib/go/Makefile.am
          M       lib/go/Makefile
          M       lib/Makefile.am
          M       compiler/cpp/src/generate/t_go_generator.cc
          

          This is an initial, rough cut intended to start discussion. I'm working on verifying it now but dealing with other build chain issues that are unrelated to this patch.

          Update: Built fine, mostly (unrelated errors bombed the build, but a thrift compiler was born). Generated some go. Seems to be OK.

          Show
          Jed Smith added a comment - - edited Hello, I have taken a stab at porting Aalok and Matt's Go 1 work to trunk, and it applied cleanly (by hand). I cleaned up the diff a lot because there were hundreds of lines whose whitespace was modified and which effectively created a no-op; I did not want to dirty the tree with all of that noise, with the exception of Go source which I know was run through gofmt, so I went ahead and pulled that in. There's good work here but it's being stagnated on Github, and I'm bummed out about that. I'm hoping to resurrect this bug and give it some life, potentially for 0.9.0 or a point release – Go 1 is pretty mature, now, and it'd be good to make Go a first-class language again for Thrift. M configure.ac M tutorial/go/src/GoServer.go M tutorial/go/src/GoClient.go M tutorial/go/src/main.go M tutorial/go/src/CalculatorHandler.go M tutorial/go/Make.deps M tutorial/go/deps.bash M tutorial/go/Makefile D lib/go/thrift D lib/go/thrift/tnonblocking_server.go D lib/go/thrift/tjson_protocol_test.go D lib/go/thrift/tjson_protocol.go D lib/go/thrift/tset.go D lib/go/thrift/texception_test.go D lib/go/thrift/texception.go D lib/go/thrift/tiostream_transport.go D lib/go/thrift/_testmain.go D lib/go/thrift/tprotocol_test.go D lib/go/thrift/tnonblocking_server_socket.go D lib/go/thrift/tmemory_buffer.go D lib/go/thrift/tnumeric.go D lib/go/thrift/tsimple_json_protocol_test.go D lib/go/thrift/tbinary_protocol_test.go D lib/go/thrift/tserver_test.go D lib/go/thrift/tserver.go D lib/go/thrift/tprocessor_factory.go D lib/go/thrift/tcompact_protocol.go D lib/go/thrift/tframed_transport_test.go D lib/go/thrift/tframed_transport.go D lib/go/thrift/tnonblocking_socket.go D lib/go/thrift/thttp_client_test.go D lib/go/thrift/tnonblocking_transport_test.go D lib/go/thrift/tsimple_server.go D lib/go/thrift/tprocessor.go D lib/go/thrift/tapplication_exception_test.go D lib/go/thrift/ttype.go D lib/go/thrift/tcontainer.go D lib/go/thrift/ttransport_factory.go D lib/go/thrift/tstruct.go D lib/go/thrift/Makefile D lib/go/thrift/tprotocol_factory.go D lib/go/thrift/tsocket.go D lib/go/thrift/ttransport_test.go D lib/go/thrift/ttransport.go D lib/go/thrift/tiostream_transport_test.go D lib/go/thrift/tmemory_buffer_test.go D lib/go/thrift/tprotocol.go D lib/go/thrift/tsimple_json_protocol.go D lib/go/thrift/tbinary_protocol.go D lib/go/thrift/tcompact_protocol_test.go D lib/go/thrift/tbase.go D lib/go/thrift/ttransport_exception.go D lib/go/thrift/tlist.go D lib/go/thrift/tmap.go D lib/go/thrift/thttp_client.go D lib/go/thrift/tserver_socket.go D lib/go/thrift/tnonblocking_transport.go D lib/go/thrift/tprotocol_exception.go D lib/go/thrift/tapplication_exception.go D lib/go/thrift/tserver_transport.go D lib/go/thrift/tfield.go D lib/go/thrift/tmessage.go D lib/go/thrift/tmessagetype.go D lib/go/thrift/tcompare.go A lib/go/src A lib/go/src/thrift A lib/go/src/thrift/tnonblocking_server.go A lib/go/src/thrift/tjson_protocol_test.go A lib/go/src/thrift/tjson_protocol.go A lib/go/src/thrift/tset.go A lib/go/src/thrift/texception_test.go A lib/go/src/thrift/texception.go A lib/go/src/thrift/tiostream_transport.go A lib/go/src/thrift/_testmain.go A lib/go/src/thrift/tprotocol_test.go A lib/go/src/thrift/tnonblocking_server_socket.go A lib/go/src/thrift/tmemory_buffer.go A lib/go/src/thrift/tnumeric.go A lib/go/src/thrift/tsimple_json_protocol_test.go A lib/go/src/thrift/tbinary_protocol_test.go A lib/go/src/thrift/tserver_test.go A lib/go/src/thrift/tserver.go A lib/go/src/thrift/tprocessor_factory.go A lib/go/src/thrift/tcompact_protocol.go A lib/go/src/thrift/tframed_transport_test.go A lib/go/src/thrift/tframed_transport.go A lib/go/src/thrift/tnonblocking_socket.go A lib/go/src/thrift/thttp_client_test.go A lib/go/src/thrift/tmap_test.go A lib/go/src/thrift/tnonblocking_transport_test.go A lib/go/src/thrift/tsimple_server.go A lib/go/src/thrift/tprocessor.go A lib/go/src/thrift/tapplication_exception_test.go A lib/go/src/thrift/ttype.go A lib/go/src/thrift/tcontainer.go A lib/go/src/thrift/ttransport_factory.go A lib/go/src/thrift/tstruct.go A lib/go/src/thrift/Makefile A lib/go/src/thrift/tprotocol_factory.go A lib/go/src/thrift/tsocket.go A lib/go/src/thrift/ttransport_test.go A lib/go/src/thrift/ttransport.go A lib/go/src/thrift/tiostream_transport_test.go A lib/go/src/thrift/tmemory_buffer_test.go A lib/go/src/thrift/tprotocol.go A lib/go/src/thrift/tsimple_json_protocol.go A lib/go/src/thrift/tbinary_protocol.go A lib/go/src/thrift/tcompact_protocol_test.go A lib/go/src/thrift/tbase.go A lib/go/src/thrift/ttransport_exception.go A lib/go/src/thrift/tlist.go A lib/go/src/thrift/tmap.go A lib/go/src/thrift/thttp_client.go A lib/go/src/thrift/tserver_socket.go A lib/go/src/thrift/tnonblocking_transport.go A lib/go/src/thrift/tprotocol_exception.go A lib/go/src/thrift/tapplication_exception.go A lib/go/src/thrift/tserver_transport.go A lib/go/src/thrift/tfield.go A lib/go/src/thrift/tmessage.go A lib/go/src/thrift/tmessagetype.go A lib/go/src/thrift/tcompare.go A lib/go/Makefile.am M lib/go/Makefile M lib/Makefile.am M compiler/cpp/src/generate/t_go_generator.cc This is an initial, rough cut intended to start discussion. I'm working on verifying it now but dealing with other build chain issues that are unrelated to this patch. Update: Built fine, mostly (unrelated errors bombed the build, but a thrift compiler was born). Generated some go. Seems to be OK.
          Hide
          Matt T. Proud added a comment -

          Hi Jed,

          If you are willing to wait a few days, I am in the process of conducting some major repairs to the way that Thrift4go generates enum messages. These are critical repairs; otherwise, Gossie, one of the major Go-based Cassandra libraries, does not work.

          • M.
          Show
          Matt T. Proud added a comment - Hi Jed, If you are willing to wait a few days, I am in the process of conducting some major repairs to the way that Thrift4go generates enum messages. These are critical repairs; otherwise, Gossie, one of the major Go-based Cassandra libraries, does not work. M.
          Hide
          Jed Smith added a comment -

          By all means – better right than right now. Just want to renew committer interest and kick the bug.

          Show
          Jed Smith added a comment - By all means – better right than right now. Just want to renew committer interest and kick the bug.
          Hide
          Jed Smith added a comment -

          Any progress, Matt?

          Show
          Jed Smith added a comment - Any progress, Matt?
          Hide
          Matt T. Proud added a comment -

          Hi Jed,

          My efforts have been at a real stand-still for the past few weeks. All of my upstream changes into Aalok Shah's thrift4go have been merged in. There is continuous integration as well as an extensive interoperability and generation validation test suites now. We should have strong confidence in the system now.

          To that end, Geert-Johan (https://github.com/GeertJohan/thrift4go) has discovered a couple of additional bugs in thrift4go that were unrelated to my changes but affect Thrift's support Go support for generated code in cases where ENUMs are used in the generic containers—e.g., MAP, LIST.

          Where do things stand now?

          1.) The current Go support in upstream Thrift is categorically incompatible with the accepted stable release of the Go platform. Taking what is in thrift4go should be satisfactory.

          2.) Geert-Johan is fixing some corner case problems: https://github.com/GeertJohan/thrift4go and is merging in his fixes into the canonical source. I would let him continue in the interim. Consider just merging in his subsequent changes into a point release, maybe.

          3.) My feeling is that the lack of useful Go support right now is actually hindering future development and improvement of the Thrift ecosystem in Go. If you snapshot what exists now, folks have an opportunity to at least use the thing now. That is a real win for everybody.

          Show
          Matt T. Proud added a comment - Hi Jed, My efforts have been at a real stand-still for the past few weeks. All of my upstream changes into Aalok Shah's thrift4go have been merged in. There is continuous integration as well as an extensive interoperability and generation validation test suites now. We should have strong confidence in the system now. To that end, Geert-Johan ( https://github.com/GeertJohan/thrift4go ) has discovered a couple of additional bugs in thrift4go that were unrelated to my changes but affect Thrift's support Go support for generated code in cases where ENUMs are used in the generic containers—e.g., MAP, LIST. Where do things stand now? 1.) The current Go support in upstream Thrift is categorically incompatible with the accepted stable release of the Go platform. Taking what is in thrift4go should be satisfactory. 2.) Geert-Johan is fixing some corner case problems: https://github.com/GeertJohan/thrift4go and is merging in his fixes into the canonical source. I would let him continue in the interim. Consider just merging in his subsequent changes into a point release, maybe. 3.) My feeling is that the lack of useful Go support right now is actually hindering future development and improvement of the Thrift ecosystem in Go. If you snapshot what exists now, folks have an opportunity to at least use the thing now. That is a real win for everybody.
          Hide
          Roger Meier added a comment -

          new patch ? or Jed's patch?

          Show
          Roger Meier added a comment - new patch ? or Jed's patch?
          Hide
          Jake Farrell added a comment -

          Any updates on this? would like to add libthrift-go to the list of available go packages for our next release

          Show
          Jake Farrell added a comment - Any updates on this? would like to add libthrift-go to the list of available go packages for our next release
          Hide
          Laurent Desegur added a comment - - edited

          A bunch of issues when trying to merge the thrift4go project into the Thrift 0.9. In particular, the go generator doesn't consolidate import statements causing the latest gofmt to complain. render_includes should be consolidated in one import directive while it's currently literring the files with import lines in between functions.

          Show
          Laurent Desegur added a comment - - edited A bunch of issues when trying to merge the thrift4go project into the Thrift 0.9. In particular, the go generator doesn't consolidate import statements causing the latest gofmt to complain. render_includes should be consolidated in one import directive while it's currently literring the files with import lines in between functions.
          Hide
          Travis Cline added a comment - - edited

          Attached are a current (thrift HEAD:691a16ac0443bc6) rebased patches to the Go client library and the tutorial code. This work represents the current state pomack's repository (4e1ad38)

          Show
          Travis Cline added a comment - - edited Attached are a current (thrift HEAD:691a16ac0443bc6) rebased patches to the Go client library and the tutorial code. This work represents the current state pomack's repository (4e1ad38)
          Hide
          Travis Cline added a comment -

          This patch fixes generated imports for Go code.

          Show
          Travis Cline added a comment - This patch fixes generated imports for Go code.
          Hide
          Laurent Desegur added a comment -

          Thanks Travis. I tried applying 0002 and got this:

          ➜ thrift git:(master) git apply --check ~/Downloads/0002-Integrate-changes-from-github.com-pomack-thrift4go.patch
          error: patch failed: compiler/cpp/src/generate/t_go_generator.cc:359
          error: compiler/cpp/src/generate/t_go_generator.cc: patch does not apply

          Show
          Laurent Desegur added a comment - Thanks Travis. I tried applying 0002 and got this: ➜ thrift git:(master) git apply --check ~/Downloads/0002-Integrate-changes-from-github.com-pomack-thrift4go.patch error: patch failed: compiler/cpp/src/generate/t_go_generator.cc:359 error: compiler/cpp/src/generate/t_go_generator.cc: patch does not apply
          Hide
          Travis Cline added a comment -

          Laurent: Sorry, that patch was applied after the patch on issue 1980. I should have produced these patches without that one in stream (though I consider that one important). Uploading new ones now.

          Show
          Travis Cline added a comment - Laurent: Sorry, that patch was applied after the patch on issue 1980. I should have produced these patches without that one in stream (though I consider that one important). Uploading new ones now.
          Hide
          Laurent Desegur added a comment -

          Just to be clear. I found the patch for issue 1980. Applied it with success then applied the one you included here and got an error still:
          ➜ thrift git:(master) ✗ git apply --check ~/Downloads/0002-Integrate-changes-from-github.com-pomack-thrift4go.patch
          error: patch failed: compiler/cpp/src/generate/t_go_generator.cc:417
          error: compiler/cpp/src/generate/t_go_generator.cc: patch does not apply

          Show
          Laurent Desegur added a comment - Just to be clear. I found the patch for issue 1980. Applied it with success then applied the one you included here and got an error still: ➜ thrift git:(master) ✗ git apply --check ~/Downloads/0002-Integrate-changes-from-github.com-pomack-thrift4go.patch error: patch failed: compiler/cpp/src/generate/t_go_generator.cc:417 error: compiler/cpp/src/generate/t_go_generator.cc: patch does not apply
          Hide
          Travis Cline added a comment -

          Laurent, I uploaded new patch here that should be applied /without/ the 1980 patch.

          Show
          Travis Cline added a comment - Laurent, I uploaded new patch here that should be applied /without/ the 1980 patch.
          Hide
          Laurent Desegur added a comment -

          Thanks Travis. This is great. However, now that the patches are applied, I still need to apply the fix for 1980 as goinstall isn't present and --with-go fails to build the go lib

          If I apply the patch attached to issue 1980 AFTER (minus the t_go_generator.cc changes I removed manually to avoid post merge conflict), I don't get with-go in configure.ac anymore (go is gone from the options).

          Travis: Is there a branch on apache.org/repos/asf/thrift.git or some place else if you don't have write access on apache (github?) with all the patches already applied so I can simply clone it? I sincerely appreciate your help.

          Show
          Laurent Desegur added a comment - Thanks Travis. This is great. However, now that the patches are applied, I still need to apply the fix for 1980 as goinstall isn't present and --with-go fails to build the go lib If I apply the patch attached to issue 1980 AFTER (minus the t_go_generator.cc changes I removed manually to avoid post merge conflict), I don't get with-go in configure.ac anymore (go is gone from the options). Travis: Is there a branch on apache.org/repos/asf/thrift.git or some place else if you don't have write access on apache (github?) with all the patches already applied so I can simply clone it? I sincerely appreciate your help.
          Hide
          Travis Cline added a comment -

          Laurent - yes, the go situation is a little convoluted (fixed soon hopefully!) My pomack integration doesn't include his configure.ac changes, sorry about that, I'll fix that up.

          Meanwhile I have some active changes here: (subject to rebasing) https://github.com/traviscline/thrift

          Show
          Travis Cline added a comment - Laurent - yes, the go situation is a little convoluted (fixed soon hopefully!) My pomack integration doesn't include his configure.ac changes, sorry about that, I'll fix that up. Meanwhile I have some active changes here: (subject to rebasing) https://github.com/traviscline/thrift
          Hide
          Laurent Desegur added a comment - - edited

          Thanks. Got it to compile and now I am going to try using it.

          Ended up pulling 3801c20e9fef28126a965f9824f33625b539f12f out of your repo (less error handling and formatting) and replaced my broken version of ./compiler/cpp/src/generate/t_go_generator.cc which seems to do the trick. At least it's generating files from the tutorial. Keeping fingers crossed...

          Thanks again for your support.

          Show
          Laurent Desegur added a comment - - edited Thanks. Got it to compile and now I am going to try using it. Ended up pulling 3801c20e9fef28126a965f9824f33625b539f12f out of your repo (less error handling and formatting) and replaced my broken version of ./compiler/cpp/src/generate/t_go_generator.cc which seems to do the trick. At least it's generating files from the tutorial. Keeping fingers crossed... Thanks again for your support.
          Hide
          Travis Cline added a comment -

          I'm actually going to retract my patches posted here, pending 1980 since they're a little intertwined.

          Show
          Travis Cline added a comment - I'm actually going to retract my patches posted here, pending 1980 since they're a little intertwined.
          Hide
          Travis Cline added a comment -

          This can be considered closed by THRIFT-2012

          Show
          Travis Cline added a comment - This can be considered closed by THRIFT-2012

            People

            • Assignee:
              Unassigned
              Reporter:
              Atul S Vasu
            • Votes:
              4 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development