Re: Add a protobuf-config script like the old gtk-config
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
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
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
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
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
(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
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
*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 -~--~~~~--~~--~--~---
Add a protobuf-config script like the old gtk-config
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 -~--~~~~--~~--~--~---