You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Anatol Pomozov (JIRA)" <ji...@apache.org> on 2010/11/22 20:57:22 UTC

[jira] Created: (THRIFT-1003) Polishing c_glib code

Polishing c_glib code
---------------------

                 Key: THRIFT-1003
                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
             Project: Thrift
          Issue Type: Improvement
            Reporter: Anatol Pomozov


attached patch contains following changes:

* Added Apache headers to c/h files
* Use gtester for running tests. We don't need -wrapper script anymore
* Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
* Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)

Given changes are mostly fixing low hanging fruits. It does not change any logic/api.

There are more chages needed, such as 
 * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
 * stop using _set_property (aka reflection) in constructors
 * check more careful about _ref and _unref handling 

but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (THRIFT-1003) Polishing c_glib code

Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1003?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Roger Meier resolved THRIFT-1003.
---------------------------------

       Resolution: Fixed
    Fix Version/s: 0.6

> Polishing c_glib code
> ---------------------
>
>                 Key: THRIFT-1003
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Anatol Pomozov
>            Assignee: Anatol Pomozov
>             Fix For: 0.6
>
>         Attachments: c_glib_fixes.diff
>
>
> attached patch contains following changes:
> * Added Apache headers to c/h files
> * Use gtester for running tests. We don't need -wrapper script anymore
> * Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
> * Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)
> Given changes are mostly fixing low hanging fruits. It does not change any logic/api.
> There are more chages needed, such as 
>  * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
>  * stop using _set_property (aka reflection) in constructors
>  * check more careful about _ref and _unref handling 
> but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-1003) Polishing c_glib code

Posted by "Anatol Pomozov (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-1003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935961#action_12935961 ] 

Anatol Pomozov commented on THRIFT-1003:
----------------------------------------

Fixed version you can find here https://github.com/anatol/thrift

And here is the fix for the dependency issue
https://github.com/anatol/thrift/commit/a9a25fe19cff1a849632b6f42ca50be28bdf22ee

I am not an automake guru (I just restored previous code there) but as
I understand gen-c_glib/t_test_debug_proto_test_types.h should be
generated before testdebugproto built. How to make it with automake?
Adding gen-c_glib/t_test_debug_proto_test_types.h to
testdebugproto_SOURCES does not make any difference - 'make check'
still fails. Is my fix a true fix for the problem?

PS Please feel free to optimize/refactor Makefile.am.


> Polishing c_glib code
> ---------------------
>
>                 Key: THRIFT-1003
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Anatol Pomozov
>         Attachments: c_glib_fixes.diff
>
>
> attached patch contains following changes:
> * Added Apache headers to c/h files
> * Use gtester for running tests. We don't need -wrapper script anymore
> * Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
> * Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)
> Given changes are mostly fixing low hanging fruits. It does not change any logic/api.
> There are more chages needed, such as 
>  * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
>  * stop using _set_property (aka reflection) in constructors
>  * check more careful about _ref and _unref handling 
> but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-1003) Polishing c_glib code

Posted by "Anatol Pomozov (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-1003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12934572#action_12934572 ] 

Anatol Pomozov commented on THRIFT-1003:
----------------------------------------

Also the patch fixes *.pc file (adds correct dependencies to glib-2.0)

> Polishing c_glib code
> ---------------------
>
>                 Key: THRIFT-1003
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Anatol Pomozov
>         Attachments: c_glib_fixes.diff
>
>
> attached patch contains following changes:
> * Added Apache headers to c/h files
> * Use gtester for running tests. We don't need -wrapper script anymore
> * Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
> * Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)
> Given changes are mostly fixing low hanging fruits. It does not change any logic/api.
> There are more chages needed, such as 
>  * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
>  * stop using _set_property (aka reflection) in constructors
>  * check more careful about _ref and _unref handling 
> but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-1003) Polishing c_glib code

Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-1003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935941#action_12935941 ] 

Roger Meier commented on THRIFT-1003:
-------------------------------------

patch fails with a minor issue:
{code}
./configure --with-c_glib  --without-csharp  --without-java  --without-erlang  --without-python  --without-perl   --without-php   --without-php_extension  --without-ruby  --without-haskell --without-cpp

gcc -o testsimpleserver testsimpleserver.o ../libthrift_c_glib_la-thrift_protocol.o ../libthrift_c_glib_la-thrift_transport.o ../libthrift_c_glib_la-thrift_transport_factory.o ../libthrift_c_glib_la-thrift_processor.o ../libthrift_c_glib_la-thrift_protocol_factory.o ../libthrift_c_glib_la-thrift_binary_protocol.o ../libthrift_c_glib_la-thrift_binary_protocol_factory.o ../libthrift_c_glib_la-thrift_socket.o ../libthrift_c_glib_la-thrift_server_transport.o ../libthrift_c_glib_la-thrift_server_socket.o ../libthrift_c_glib_la-thrift_server.o  /usr/lib/libgobject-2.0.so /usr/lib/libglib-2.0.so -lrt -lpthread  
gcc -DHAVE_CONFIG_H -I. -I../../..  -g -Wall -I../src -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include      -MT testdebugproto.o -MD -MP -MF .deps/testdebugproto.Tpo -c -o testdebugproto.o testdebugproto.c
testdebugproto.c:27:54: error: gen-c_glib/t_test_debug_proto_test_types.h: No such file or directory
{code}

to following step was required to build successfully
{code}
cd lib/c_glib/test/ ; make libtestgenc.la ; cd ../../../
{code}
if {{--with-cpp}},  I have to build the cpp test libs also by hand
{code}
cd lib/c_glib/test/ ; make libtestgencpp.la ; cd ../../../
{code}

It seems, that something in lib/c_glib/test/makefile.am is broken

> Polishing c_glib code
> ---------------------
>
>                 Key: THRIFT-1003
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Anatol Pomozov
>         Attachments: c_glib_fixes.diff
>
>
> attached patch contains following changes:
> * Added Apache headers to c/h files
> * Use gtester for running tests. We don't need -wrapper script anymore
> * Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
> * Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)
> Given changes are mostly fixing low hanging fruits. It does not change any logic/api.
> There are more chages needed, such as 
>  * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
>  * stop using _set_property (aka reflection) in constructors
>  * check more careful about _ref and _unref handling 
> but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-1003) Polishing c_glib code

Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-1003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935958#action_12935958 ] 

Roger Meier commented on THRIFT-1003:
-------------------------------------

my env on Debian Lenny
{code}
ltmain.sh (GNU libtool) 1.5.26 Debian 1.5.26-4+lenny1 (1.1220.2.493 2008/02/01 16:58:18)
automake (GNU automake) 1.10.1
autoconf (GNU Autoconf) 2.65
{code}

same behavior with new libtool(2.2.6b) from backports {{sudo apt-get -t lenny-backports install libtool}}

> Polishing c_glib code
> ---------------------
>
>                 Key: THRIFT-1003
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Anatol Pomozov
>         Attachments: c_glib_fixes.diff
>
>
> attached patch contains following changes:
> * Added Apache headers to c/h files
> * Use gtester for running tests. We don't need -wrapper script anymore
> * Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
> * Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)
> Given changes are mostly fixing low hanging fruits. It does not change any logic/api.
> There are more chages needed, such as 
>  * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
>  * stop using _set_property (aka reflection) in constructors
>  * check more careful about _ref and _unref handling 
> but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (THRIFT-1003) Polishing c_glib code

Posted by "Roger Meier (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1003?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Roger Meier reassigned THRIFT-1003:
-----------------------------------

    Assignee: Anatol Pomozov

committed!

> Polishing c_glib code
> ---------------------
>
>                 Key: THRIFT-1003
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Anatol Pomozov
>            Assignee: Anatol Pomozov
>             Fix For: 0.6
>
>         Attachments: c_glib_fixes.diff
>
>
> attached patch contains following changes:
> * Added Apache headers to c/h files
> * Use gtester for running tests. We don't need -wrapper script anymore
> * Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
> * Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)
> Given changes are mostly fixing low hanging fruits. It does not change any logic/api.
> There are more chages needed, such as 
>  * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
>  * stop using _set_property (aka reflection) in constructors
>  * check more careful about _ref and _unref handling 
> but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-1003) Polishing c_glib code

Posted by "Anatol Pomozov (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1003?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Anatol Pomozov updated THRIFT-1003:
-----------------------------------

    Attachment: c_glib_fixes.diff

> Polishing c_glib code
> ---------------------
>
>                 Key: THRIFT-1003
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1003
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Anatol Pomozov
>         Attachments: c_glib_fixes.diff
>
>
> attached patch contains following changes:
> * Added Apache headers to c/h files
> * Use gtester for running tests. We don't need -wrapper script anymore
> * Use one-line macros G_DEFINE_TYPE instead of 15-line class definition
> * Keep formatting closer to glib-like style (one line class definition macroses/remove trailing spaces)
> Given changes are mostly fixing low hanging fruits. It does not change any logic/api.
> There are more chages needed, such as 
>  * using CLASS_TYPE_new functions instead of g_object_new(CLASS_TYPE) 
>  * stop using _set_property (aka reflection) in constructors
>  * check more careful about _ref and _unref handling 
> but this requires more careful refactoring so it will be later in a separate patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.