Message ID | 20190123003432.27216-1-blp@ovn.org |
---|---|
State | Not Applicable |
Headers | show |
Series | [ovs-dev] ovn: Add pkg-config support via libovn.pc. | expand |
Bleep bloop. Greetings Ben Pfaff, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author 竹內宏輝 Hiroki Takeuchi <hirokiht@gmail.com> needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Hiroki Takeuchi <hirokiht@gmail.com> Lines checked: 61, Warnings: 1, Errors: 1 Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
Ben Pfaff <blp@ovn.org> writes: > From: 竹內宏輝 Hiroki Takeuchi <hirokiht@gmail.com> > > Submitted-at: https://github.com/openvswitch/ovs/pull/270 > Signed-off-by: Hiroki Takeuchi <hirokiht@gmail.com> > --- > I'm sending this patch as a squashed version of what was submitted at > Github. I know that pkg-config is somewhat controversial, so I'd like to > hear what the denizens of ovs-dev have to say about including a libovn.pc. I like pkg-config, as a general rule. I wasn't aware that it was controversial, but I'm definitely not plugged into the latest trends in library development. > Thanks, > > Ben. > > configure.ac | 1 + > ovn/lib/automake.mk | 3 +++ > ovn/lib/libovn.pc.in | 11 +++++++++++ > 3 files changed, 15 insertions(+) > create mode 100644 ovn/lib/libovn.pc.in > > diff --git a/configure.ac b/configure.ac > index 505e3d041e93..473454265532 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -201,6 +201,7 @@ AC_CONFIG_FILES(lib/libopenvswitch.pc) > AC_CONFIG_FILES(lib/libsflow.pc) > AC_CONFIG_FILES(ofproto/libofproto.pc) > AC_CONFIG_FILES(ovsdb/libovsdb.pc) > +AC_CONFIG_FILES(ovn/lib/libovn.pc) > AC_CONFIG_FILES(include/openvswitch/version.h) > > dnl This makes sure that include/openflow gets created in the build directory. > diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk > index 6178fc2d5aa4..cb216f626293 100644 > --- a/ovn/lib/automake.mk > +++ b/ovn/lib/automake.mk > @@ -24,6 +24,9 @@ nodist_ovn_lib_libovn_la_SOURCES = \ > ovn/lib/ovn-sb-idl.c \ > ovn/lib/ovn-sb-idl.h > > +pkgconfig_DATA += \ > + ovn/lib/libovn.pc Is it intended that the OVN library will be used by anything other than OVN? If so, it's probably also good to enhance the ABI version information to work similarly to the way the openvswitch library is versioned. If not, maybe it's not a good idea to have a pkg-config entry for the library (since it would encourage non-ovn utilities to link to it without any kind of ABI contract). I don't have any idea whether it would be useful to provide access to the OVN library to other projects. > + > # ovn-sb IDL > OVSIDL_BUILT += \ > ovn/lib/ovn-sb-idl.c \ > diff --git a/ovn/lib/libovn.pc.in b/ovn/lib/libovn.pc.in > new file mode 100644 > index 000000000000..6d9b22be6568 > --- /dev/null > +++ b/ovn/lib/libovn.pc.in > @@ -0,0 +1,11 @@ > +prefix=@prefix@ > +exec_prefix=@exec_prefix@ > +libdir=@libdir@ > +includedir=@includedir@ > + > +Name: libovn > +Description: Open Virtual Network for Open vSwitch I think the Description here should include the word 'library' - but it's just a nit. > +Version: @VERSION@ > +Libs: -L${libdir} -lovn > +Libs.private: @LIBS@ > +Cflags: -I${includedir}/ovn I believe this file should also have a Requires: stanza for libopenvswitch (since any application linking to the ovn library will need to link to the openvswitch library as well). At the moment, it could be very restrictive: Requires: libopenvswitch = @VERSION@ But it will probably need to be relaxed in the future (to some minimum version). I admit I don't know enough about the ovn library to make an informed suggestion here.
On Wed, Jan 23, 2019 at 10:17:40AM -0500, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > > From: 竹內宏輝 Hiroki Takeuchi <hirokiht@gmail.com> > > > > Submitted-at: https://github.com/openvswitch/ovs/pull/270 > > Signed-off-by: Hiroki Takeuchi <hirokiht@gmail.com> > > --- > > I'm sending this patch as a squashed version of what was submitted at > > Github. I know that pkg-config is somewhat controversial, so I'd like to > > hear what the denizens of ovs-dev have to say about including a libovn.pc. > > I like pkg-config, as a general rule. I wasn't aware that it was > controversial, but I'm definitely not plugged into the latest trends in > library development. I think I misremembered this: it is libtool .la files that are deprecated, not pkg-config .pc files. Reference: https://www.netfort.gr.jp/~dancer/column/libpkg-guide/libpkg-guide.html#pkgconfig Never mind, then. I'll ask Hiroki to comment on the rest here.
diff --git a/configure.ac b/configure.ac index 505e3d041e93..473454265532 100644 --- a/configure.ac +++ b/configure.ac @@ -201,6 +201,7 @@ AC_CONFIG_FILES(lib/libopenvswitch.pc) AC_CONFIG_FILES(lib/libsflow.pc) AC_CONFIG_FILES(ofproto/libofproto.pc) AC_CONFIG_FILES(ovsdb/libovsdb.pc) +AC_CONFIG_FILES(ovn/lib/libovn.pc) AC_CONFIG_FILES(include/openvswitch/version.h) dnl This makes sure that include/openflow gets created in the build directory. diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk index 6178fc2d5aa4..cb216f626293 100644 --- a/ovn/lib/automake.mk +++ b/ovn/lib/automake.mk @@ -24,6 +24,9 @@ nodist_ovn_lib_libovn_la_SOURCES = \ ovn/lib/ovn-sb-idl.c \ ovn/lib/ovn-sb-idl.h +pkgconfig_DATA += \ + ovn/lib/libovn.pc + # ovn-sb IDL OVSIDL_BUILT += \ ovn/lib/ovn-sb-idl.c \ diff --git a/ovn/lib/libovn.pc.in b/ovn/lib/libovn.pc.in new file mode 100644 index 000000000000..6d9b22be6568 --- /dev/null +++ b/ovn/lib/libovn.pc.in @@ -0,0 +1,11 @@ +prefix=@prefix@ +exec_prefix=@exec_prefix@ +libdir=@libdir@ +includedir=@includedir@ + +Name: libovn +Description: Open Virtual Network for Open vSwitch +Version: @VERSION@ +Libs: -L${libdir} -lovn +Libs.private: @LIBS@ +Cflags: -I${includedir}/ovn