Re: Add a protobuf-config script like the old gtk-config

2009-08-03 Thread Jeff Bailey
lgtm
New version is much nicer.  Suck about the crashing bug in pkg-config. =(

Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754


On Thu, Jul 30, 2009 at 3:00 PM, Kenton Varda  wrote:

> New patch:  Use pkg-config instead.  Much simpler.
>
>
> On Thu, Jul 30, 2009 at 10:05 AM, Jeff Bailey wrote:
>
>> I haven't done pkg-config stuff yet.  Maybe check in on #autotools in
>> freenode.  I'm usually there, as are other people.
>> Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754
>>
>>
>> On Thu, Jul 30, 2009 at 12:12 PM, Kenton Varda  wrote:
>>
>>> Yeargh, I'm behind the times.  pkg-config?  I guess I should be
>>> integrating with that rather than writing my own script?
>>>
>>>
>>> On Thu, Jul 30, 2009 at 9:05 AM, Jeff Bailey wrote:
>>>
 lgtm

 42 Packages which depend on Protocol Buffers should call this script
 automatically   43 as part of their own configure script.

 Provide an example with PKG_CONFIG or something like that.

 Otherwise, looks good.  Thanks!

 Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754


 On Thu, Jul 30, 2009 at 12:00 PM, Kenton Varda wrote:

> (New patch set uploaded.)
>
>
> On Thu, Jul 30, 2009 at 8:59 AM, Kenton Varda wrote:
>
>>
>>
>> On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey 
>> wrote:
>>
>>> *sigh*  It looks like the version at appspot.com isn't GA+ enabled,
>>> so I sign in and it thinks I'm not signed in.
>>> Anyhow, a few comments:
>>>
>>> Since it's generated by configure.ac, do you need it in bin_SCRIPTS?
>>>  I think that might cause it to get looked at twice.
>>>
>>
>> The purpose of putting it in bin_SCRIPTS is to make sure that it is
>> installed, which configure is not going to do automatically.  The 
>> automake
>> docs say that bin_SCRIPTS are by default not included in the dist, which 
>> is
>> what we want here (since configure generates it).
>>
>>
>>> You should pretty much always do a set -e at the top of a shell
>>> script to catch errors early on.
>>>
>>
>> Oops, fixed.
>>
>>
>>>
>>>  *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" !=
>>> ""; then
>>>
>>> Should those all be @pre...@?
>>>
>>
>> Yes.  :/
>>
>>
>>> Also, I think test -a might be a bashism in this case.
>>>
>>
>> Changed to "&& test".
>>
>>
>>> Same for this line:
>>>
>>>
>>>  *79* if test $full_library = true -o $explicit_library = false;
>>> then
>>>
>>
>> Done.
>>
>> Also, I added --ldflags as a separate option since LDFLAGS and LIBS
>> are traditionally separate.  Not sure why gtk-config itself does not do
>> this.
>>
>> Also also, I expanded the help text.
>>
>> Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since
>> I doubt anyone will correctly parse it otherwise (since people will code
>> against official releases which have no suffix).
>>
>
>

>>>
>>
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Add a protobuf-config script like the old gtk-config

2009-07-30 Thread Kenton Varda
New patch:  Use pkg-config instead.  Much simpler.

On Thu, Jul 30, 2009 at 10:05 AM, Jeff Bailey  wrote:

> I haven't done pkg-config stuff yet.  Maybe check in on #autotools in
> freenode.  I'm usually there, as are other people.
> Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754
>
>
> On Thu, Jul 30, 2009 at 12:12 PM, Kenton Varda  wrote:
>
>> Yeargh, I'm behind the times.  pkg-config?  I guess I should be
>> integrating with that rather than writing my own script?
>>
>>
>> On Thu, Jul 30, 2009 at 9:05 AM, Jeff Bailey wrote:
>>
>>> lgtm
>>>
>>> 42 Packages which depend on Protocol Buffers should call this script
>>> automatically   43 as part of their own configure script.
>>>
>>> Provide an example with PKG_CONFIG or something like that.
>>>
>>> Otherwise, looks good.  Thanks!
>>>
>>> Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754
>>>
>>>
>>> On Thu, Jul 30, 2009 at 12:00 PM, Kenton Varda wrote:
>>>
 (New patch set uploaded.)


 On Thu, Jul 30, 2009 at 8:59 AM, Kenton Varda wrote:

>
>
> On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey wrote:
>
>> *sigh*  It looks like the version at appspot.com isn't GA+ enabled,
>> so I sign in and it thinks I'm not signed in.
>> Anyhow, a few comments:
>>
>> Since it's generated by configure.ac, do you need it in bin_SCRIPTS?
>>  I think that might cause it to get looked at twice.
>>
>
> The purpose of putting it in bin_SCRIPTS is to make sure that it is
> installed, which configure is not going to do automatically.  The automake
> docs say that bin_SCRIPTS are by default not included in the dist, which 
> is
> what we want here (since configure generates it).
>
>
>> You should pretty much always do a set -e at the top of a shell script
>> to catch errors early on.
>>
>
> Oops, fixed.
>
>
>>
>>  *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" !=
>> ""; then
>>
>> Should those all be @pre...@?
>>
>
> Yes.  :/
>
>
>> Also, I think test -a might be a bashism in this case.
>>
>
> Changed to "&& test".
>
>
>> Same for this line:
>>
>>
>>  *79* if test $full_library = true -o $explicit_library = false; then
>>
>
> Done.
>
> Also, I added --ldflags as a separate option since LDFLAGS and LIBS are
> traditionally separate.  Not sure why gtk-config itself does not do this.
>
> Also also, I expanded the help text.
>
> Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since I
> doubt anyone will correctly parse it otherwise (since people will code
> against official releases which have no suffix).
>


>>>
>>
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Add a protobuf-config script like the old gtk-config

2009-07-30 Thread Jeff Bailey
I haven't done pkg-config stuff yet.  Maybe check in on #autotools in
freenode.  I'm usually there, as are other people.
Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754


On Thu, Jul 30, 2009 at 12:12 PM, Kenton Varda  wrote:

> Yeargh, I'm behind the times.  pkg-config?  I guess I should be integrating
> with that rather than writing my own script?
>
>
> On Thu, Jul 30, 2009 at 9:05 AM, Jeff Bailey wrote:
>
>> lgtm
>>
>> 42 Packages which depend on Protocol Buffers should call this script
>> automatically   43 as part of their own configure script.
>>
>> Provide an example with PKG_CONFIG or something like that.
>>
>> Otherwise, looks good.  Thanks!
>>
>> Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754
>>
>>
>> On Thu, Jul 30, 2009 at 12:00 PM, Kenton Varda  wrote:
>>
>>> (New patch set uploaded.)
>>>
>>>
>>> On Thu, Jul 30, 2009 at 8:59 AM, Kenton Varda  wrote:
>>>


 On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey wrote:

> *sigh*  It looks like the version at appspot.com isn't GA+ enabled, so
> I sign in and it thinks I'm not signed in.
> Anyhow, a few comments:
>
> Since it's generated by configure.ac, do you need it in bin_SCRIPTS?
>  I think that might cause it to get looked at twice.
>

 The purpose of putting it in bin_SCRIPTS is to make sure that it is
 installed, which configure is not going to do automatically.  The automake
 docs say that bin_SCRIPTS are by default not included in the dist, which is
 what we want here (since configure generates it).


> You should pretty much always do a set -e at the top of a shell script
> to catch errors early on.
>

 Oops, fixed.


>
>  *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" != "";
> then
>
> Should those all be @pre...@?
>

 Yes.  :/


> Also, I think test -a might be a bashism in this case.
>

 Changed to "&& test".


> Same for this line:
>
>
>  *79* if test $full_library = true -o $explicit_library = false; then
>

 Done.

 Also, I added --ldflags as a separate option since LDFLAGS and LIBS are
 traditionally separate.  Not sure why gtk-config itself does not do this.

 Also also, I expanded the help text.

 Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since I
 doubt anyone will correctly parse it otherwise (since people will code
 against official releases which have no suffix).

>>>
>>>
>>
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Add a protobuf-config script like the old gtk-config

2009-07-30 Thread Kenton Varda
Yeargh, I'm behind the times.  pkg-config?  I guess I should be integrating
with that rather than writing my own script?

On Thu, Jul 30, 2009 at 9:05 AM, Jeff Bailey  wrote:

> lgtm
>
> 42 Packages which depend on Protocol Buffers should call this script
> automatically  43 as part of their own configure script.
>
> Provide an example with PKG_CONFIG or something like that.
>
> Otherwise, looks good.  Thanks!
>
> Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754
>
>
> On Thu, Jul 30, 2009 at 12:00 PM, Kenton Varda  wrote:
>
>> (New patch set uploaded.)
>>
>>
>> On Thu, Jul 30, 2009 at 8:59 AM, Kenton Varda  wrote:
>>
>>>
>>>
>>> On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey wrote:
>>>
 *sigh*  It looks like the version at appspot.com isn't GA+ enabled, so
 I sign in and it thinks I'm not signed in.
 Anyhow, a few comments:

 Since it's generated by configure.ac, do you need it in bin_SCRIPTS?  I
 think that might cause it to get looked at twice.

>>>
>>> The purpose of putting it in bin_SCRIPTS is to make sure that it is
>>> installed, which configure is not going to do automatically.  The automake
>>> docs say that bin_SCRIPTS are by default not included in the dist, which is
>>> what we want here (since configure generates it).
>>>
>>>
 You should pretty much always do a set -e at the top of a shell script
 to catch errors early on.

>>>
>>> Oops, fixed.
>>>
>>>

  *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" != "";
 then

 Should those all be @pre...@?

>>>
>>> Yes.  :/
>>>
>>>
 Also, I think test -a might be a bashism in this case.

>>>
>>> Changed to "&& test".
>>>
>>>
 Same for this line:


  *79* if test $full_library = true -o $explicit_library = false; then

>>>
>>> Done.
>>>
>>> Also, I added --ldflags as a separate option since LDFLAGS and LIBS are
>>> traditionally separate.  Not sure why gtk-config itself does not do this.
>>>
>>> Also also, I expanded the help text.
>>>
>>> Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since I
>>> doubt anyone will correctly parse it otherwise (since people will code
>>> against official releases which have no suffix).
>>>
>>
>>
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Add a protobuf-config script like the old gtk-config

2009-07-30 Thread Jeff Bailey
lgtm

42 Packages which depend on Protocol Buffers should call this script
automatically  43 as part of their own configure script.

Provide an example with PKG_CONFIG or something like that.

Otherwise, looks good.  Thanks!

Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754


On Thu, Jul 30, 2009 at 12:00 PM, Kenton Varda  wrote:

> (New patch set uploaded.)
>
>
> On Thu, Jul 30, 2009 at 8:59 AM, Kenton Varda  wrote:
>
>>
>>
>> On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey wrote:
>>
>>> *sigh*  It looks like the version at appspot.com isn't GA+ enabled, so I
>>> sign in and it thinks I'm not signed in.
>>> Anyhow, a few comments:
>>>
>>> Since it's generated by configure.ac, do you need it in bin_SCRIPTS?  I
>>> think that might cause it to get looked at twice.
>>>
>>
>> The purpose of putting it in bin_SCRIPTS is to make sure that it is
>> installed, which configure is not going to do automatically.  The automake
>> docs say that bin_SCRIPTS are by default not included in the dist, which is
>> what we want here (since configure generates it).
>>
>>
>>> You should pretty much always do a set -e at the top of a shell script to
>>> catch errors early on.
>>>
>>
>> Oops, fixed.
>>
>>
>>>
>>>  *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" != "";
>>> then
>>>
>>> Should those all be @pre...@?
>>>
>>
>> Yes.  :/
>>
>>
>>> Also, I think test -a might be a bashism in this case.
>>>
>>
>> Changed to "&& test".
>>
>>
>>> Same for this line:
>>>
>>>
>>>  *79* if test $full_library = true -o $explicit_library = false; then
>>>
>>
>> Done.
>>
>> Also, I added --ldflags as a separate option since LDFLAGS and LIBS are
>> traditionally separate.  Not sure why gtk-config itself does not do this.
>>
>> Also also, I expanded the help text.
>>
>> Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since I
>> doubt anyone will correctly parse it otherwise (since people will code
>> against official releases which have no suffix).
>>
>
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Add a protobuf-config script like the old gtk-config

2009-07-30 Thread Kenton Varda
(New patch set uploaded.)

On Thu, Jul 30, 2009 at 8:59 AM, Kenton Varda  wrote:

>
>
> On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey wrote:
>
>> *sigh*  It looks like the version at appspot.com isn't GA+ enabled, so I
>> sign in and it thinks I'm not signed in.
>> Anyhow, a few comments:
>>
>> Since it's generated by configure.ac, do you need it in bin_SCRIPTS?  I
>> think that might cause it to get looked at twice.
>>
>
> The purpose of putting it in bin_SCRIPTS is to make sure that it is
> installed, which configure is not going to do automatically.  The automake
> docs say that bin_SCRIPTS are by default not included in the dist, which is
> what we want here (since configure generates it).
>
>
>> You should pretty much always do a set -e at the top of a shell script to
>> catch errors early on.
>>
>
> Oops, fixed.
>
>
>>
>>  *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" != "";
>> then
>>
>> Should those all be @pre...@?
>>
>
> Yes.  :/
>
>
>> Also, I think test -a might be a bashism in this case.
>>
>
> Changed to "&& test".
>
>
>> Same for this line:
>>
>>
>>  *79* if test $full_library = true -o $explicit_library = false; then
>>
>
> Done.
>
> Also, I added --ldflags as a separate option since LDFLAGS and LIBS are
> traditionally separate.  Not sure why gtk-config itself does not do this.
>
> Also also, I expanded the help text.
>
> Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since I
> doubt anyone will correctly parse it otherwise (since people will code
> against official releases which have no suffix).
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Add a protobuf-config script like the old gtk-config

2009-07-30 Thread Kenton Varda
On Wed, Jul 29, 2009 at 6:34 PM, Jeff Bailey  wrote:

> *sigh*  It looks like the version at appspot.com isn't GA+ enabled, so I
> sign in and it thinks I'm not signed in.
> Anyhow, a few comments:
>
> Since it's generated by configure.ac, do you need it in bin_SCRIPTS?  I
> think that might cause it to get looked at twice.
>

The purpose of putting it in bin_SCRIPTS is to make sure that it is
installed, which configure is not going to do automatically.  The automake
docs say that bin_SCRIPTS are by default not included in the dist, which is
what we want here (since configure generates it).


> You should pretty much always do a set -e at the top of a shell script to
> catch errors early on.
>

Oops, fixed.


>
>  *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" != "";
> then
>
> Should those all be @pre...@?
>

Yes.  :/


> Also, I think test -a might be a bashism in this case.
>

Changed to "&& test".


> Same for this line:
>
>
>  *79* if test $full_library = true -o $explicit_library = false; then
>

Done.

Also, I added --ldflags as a separate option since LDFLAGS and LIBS are
traditionally separate.  Not sure why gtk-config itself does not do this.

Also also, I expanded the help text.

Also^3, I made --version strip the suffix ("-pre", "rc1", etc.) since I
doubt anyone will correctly parse it otherwise (since people will code
against official releases which have no suffix).

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Add a protobuf-config script like the old gtk-config

2009-07-29 Thread Jeff Bailey
*sigh*  It looks like the version at appspot.com isn't GA+ enabled, so I
sign in and it thinks I'm not signed in.
Anyhow, a few comments:

Since it's generated by configure.ac, do you need it in bin_SCRIPTS?  I
think that might cause it to get looked at twice.

You should pretty much always do a set -e at the top of a shell script to
catch errors early on.


 *73* if test "@prefix@" != /usr -a "@prefix" != / -a "@prefix" != ""; then

Should those all be @pre...@?

Also, I think test -a might be a bashism in this case.  From a handy cheat
sheet on bashisms (https://wiki.ubuntu.com/DashAsBinSh):

While dash supports most uses of the -a and -o options, they have very
confusing semantics even in bash and are best avoided. Commands like the
following:


[ \( "$foo" = "$bar" -a -f /bin/baz \) -o ! -x /bin/quux ]

should be replaced with:


(([ "$foo" = "$bar" ] && [ -f /bin/baz ]) || [ ! -x /bin/quux ])

Same for this line:


 *79* if test $full_library = true -o $explicit_library = false; then



Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754


On Wed, Jul 29, 2009 at 9:23 PM,  wrote:

> Reviewers: jeffbailey,
>
> Description:
> This seems needed due to confusion about dependencies, pthreads, etc.
> I'm not entirely familiar with the conventions for these scripts,
> though.  Did I do it right?
>
> Please review this at http://codereview.appspot.com/98071
>
> Affected files:
>  M Makefile.am
>  M configure.ac
>  A protobuf-config.in
>
>
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to 
protobuf+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---