[ovs-dev] ovn: Add pkg-config support via libovn.pc.

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.

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

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.