Skip to content

Devel headers #18807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ noinst_LIBRARIES =
nodist_noinst_DATA =
lib_LTLIBRARIES =
module_LTLIBRARIES =
pkginclude_HEADERS =
pkginclude_HEADERS = config.h
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First NAK of two: a config.h from autoconf should never be installed or depended on by development headers. This is discussed in https://sourceware.org/autobook/autobook/autobook_66.html#Installing-Header-Files

In the case of FRR, it is primarily lib/frratomic.h that causes this dependency. The reasons for this are that the use of atomic operations in Quagga began before ISO C11 was widely available. This is no longer a problem and the correct fix is to simply remove the compatibility handling in that file, which then means config.h does not need to be installed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reference — it's a valid point that exposing config.h in installed headers can lead to conflicts.

That said, the issue in FRR isn't limited to frratomic.h. Several headers use #ifdef HAVE_... checks, which makes some form of config header necessary. Removing the atomics compatibility code alone doesn't fully eliminate this dependency.

Renaming config.h (e.g., to frr_config.h) is a useful step to avoid name collisions and makes it safer to include in installed headers, though it still exposes some internal build configuration and ties the headers to specific platform checks.

nobase_pkginclude_HEADERS =
nobase_nodist_pkginclude_HEADERS =
nodist_pkginclude_HEADERS =
dist_yangmodels_DATA =
man_MANS =
Expand Down
7 changes: 7 additions & 0 deletions debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,10 @@ Description: FRRouting suite - GRPC interface
This provides the GRPC interface to the daemons.
Build-Profiles: <pkg.frr.grpc>

Package: frr-dev
Architecture: any
Section: libdevel
Depends: ${misc:Depends}, frr (= ${binary:Version})
Description: Development files for FRRouting
This package contains the development files (headers and static libraries)
necessary to build software that links against FRR internals.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second (independent) NAK of two: you're saying in your own description that you wrote "links against FRR internals."

This is counter to the purpose of a -dev package. The point is to provide a stable entry point to other projects to use things. FRR does not have stable APIs at this point, and does not have testing for stability. This would essentially be a "trap package", and it would complicate future work on introducing stable APIs.

Further, I would apply the "test requirement" here: without tests, this will simply break. There are test tools for this, abicc and abidiff: https://lvc.github.io/abi-compliance-checker/ https://sourceware.org/libabigail/manual/abidiff.html

Considering that there is a better solution available (comment below), this simply should not be done.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is a follow-up to the initial PoC proposing a new dplane plugin, which was submitted to get community input on integration. In the weekly call, we agreed the plugin must remain out-of-tree.

However, building it is currently not possible due to the broken frr-devel package on Red Hat and its absence on Debian. This PR fixes that.

There’s no claim of ABI stability — just the need to support out-of-tree development in a practical, working state.

3 changes: 3 additions & 0 deletions debian/frr-dev.install
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
usr/include/frr/*.h
usr/include/frr/*/*.h
usr/lib/*/frr/*.so
2 changes: 1 addition & 1 deletion debian/frr-snmp.install
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
usr/lib/*/frr/libfrrsnmp.*
usr/lib/*/frr/libfrrsnmp.so.*
usr/lib/*/frr/modules/*_snmp.so
8 changes: 4 additions & 4 deletions debian/frr.install
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ tools/etc/logrotate.d/frr etc/logrotate.d/
tools/frr-reload usr/lib/frr/
usr/bin/mtracebis
usr/bin/vtysh
usr/lib/*/frr/libfrr.*
usr/lib/*/frr/libfrrcares.*
usr/lib/*/frr/libfrrospfapiclient.*
usr/lib/*/frr/libmgmt_be_nb.*
usr/lib/*/frr/libfrr.so.*
usr/lib/*/frr/libfrrcares.so.*
usr/lib/*/frr/libfrrospfapiclient.so.*
usr/lib/*/frr/libmgmt_be_nb.so.*
usr/lib/*/frr/modules/bgpd_bmp.so
usr/lib/*/frr/modules/dplane_fpm_nl.so
usr/lib/*/frr/modules/zebra_cumulus_mlag.so
Expand Down
5 changes: 2 additions & 3 deletions debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,8 @@ override_dh_auto_install:
cp -r tools/etc/* debian/tmp/etc/
-rm debian/tmp/etc/frr/daemons.conf

# drop dev-only files
find debian/tmp -name '*.la' -o -name '*.a' -o -name 'lib*.so' | xargs rm -f
rm -rf debian/tmp/usr/include
# drop static libraries and ssd
find debian/tmp -name '*.la' -o -name '*.a' | xargs rm -f
-rm debian/tmp/usr/lib/frr/ssd

override_dh_auto_build:
Expand Down
6 changes: 3 additions & 3 deletions docker/centos-8/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ RUN dnf install --enablerepo=powertools -y rpm-build git autoconf pcre-devel \
groff pkgconfig json-c-devel pam-devel bison flex python3-pytest \
c-ares-devel python3-devel python3-sphinx libcap-devel platform-python-devel \
protobuf-c-devel \
https://ci1.netdef.org/artifact/LIBYANG-LIBYANG2/shared/build-00181/RedHat-8-x86_64-Packages/libyang-2.1.80-1.el8.x86_64.rpm \
https://ci1.netdef.org/artifact/LIBYANG-LIBYANG2/shared/build-00181/RedHat-8-x86_64-Packages/libyang-devel-2.1.80-1.el8.x86_64.rpm \
https://ci1.netdef.org/artifact/LIBYANG-LIBYANG2/shared/build-00184/RedHat-8-x86_64-Packages/libyang-2.1.128.83.gfc4dbd92-1.el8.x86_64.rpm \
https://ci1.netdef.org/artifact/LIBYANG-LIBYANG2/shared/build-00184/RedHat-8-x86_64-Packages/libyang-devel-2.1.128.83.gfc4dbd92-1.el8.x86_64.rpm \
https://ci1.netdef.org/artifact/RPKI-RTRLIB/shared/build-00146/CentOS-7-x86_64-Packages/librtr-0.8.0-1.el7.x86_64.rpm \
https://ci1.netdef.org/artifact/RPKI-RTRLIB/shared/build-00146/CentOS-7-x86_64-Packages/librtr-devel-0.8.0-1.el7.x86_64.rpm

Expand Down Expand Up @@ -43,7 +43,7 @@ RUN sed -i -e "s|mirrorlist=|#mirrorlist=|g" /etc/yum.repos.d/CentOS-* \
&& sed -i -e "s|#baseurl=http://mirror.centos.org|baseurl=http://vault.centos.org|g" /etc/yum.repos.d/CentOS-*

RUN mkdir -p /pkgs/rpm \
&& yum install -y https://ci1.netdef.org/artifact/LIBYANG-LIBYANG2/shared/build-00181/RedHat-8-x86_64-Packages/libyang-2.1.80-1.el8.x86_64.rpm \
&& yum install -y https://ci1.netdef.org/artifact/LIBYANG-LIBYANG2/shared/build-00184/RedHat-8-x86_64-Packages/libyang-2.1.128.83.gfc4dbd92-1.el8.x86_64.rpm \
https://ci1.netdef.org/artifact/RPKI-RTRLIB/shared/build-00146/CentOS-7-x86_64-Packages/librtr-0.8.0-1.el7.x86_64.rpm

COPY --from=centos-8-builder /rpmbuild/RPMS/ /pkgs/rpm/
Expand Down
8 changes: 4 additions & 4 deletions lib/subdir.am
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ clippy_scan += \
lib/zlog_5424_cli.c \
# end

pkginclude_HEADERS += \
nobase_pkginclude_HEADERS += \
lib/admin_group.h \
lib/affinitymap.h \
lib/agg_table.h \
Expand Down Expand Up @@ -346,7 +346,7 @@ pkginclude_HEADERS += \
# end


nodist_pkginclude_HEADERS += \
nobase_nodist_pkginclude_HEADERS += \
lib/route_types.h \
lib/version.h \
# end
Expand Down Expand Up @@ -390,7 +390,7 @@ lib_libfrrsnmp_la_SOURCES = \
#
if CARES
lib_LTLIBRARIES += lib/libfrrcares.la
pkginclude_HEADERS += lib/resolver.h
nobase_pkginclude_HEADERS += lib/resolver.h
endif

lib_libfrrcares_la_CFLAGS = $(AM_CFLAGS) $(CARES_CFLAGS)
Expand All @@ -405,7 +405,7 @@ lib_libfrrcares_la_SOURCES = \
#
if ZEROMQ
lib_LTLIBRARIES += lib/libfrrzmq.la
pkginclude_HEADERS += lib/frr_zmq.h
nobase_pkginclude_HEADERS += lib/frr_zmq.h
endif

lib_libfrrzmq_la_CFLAGS = $(AM_CFLAGS) $(ZEROMQ_CFLAGS)
Expand Down
19 changes: 15 additions & 4 deletions redhat/frr.spec.in
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,15 @@ Contributed python 2.7 tools which may be of use with frr.


%package devel
Summary: Header and object files for frr development
Summary: Development files for building applications or plugins using FRR
Group: System Environment/Daemons
Requires: %{name} = %{version}-%{release}

%description devel
The frr-devel package contains the header and object files necessary for
developing OSPF-API and frr applications.
The frr-devel package contains header files and configuration needed for
developing out-of-tree applications or plugins that interface with FRR, such
as dplane modules, monitoring tools, and routing integrations. This includes
headers from FRR's core libraries, zebra, and optional daemons.


%if %{with_rpki}
Expand Down Expand Up @@ -854,7 +856,16 @@ sed -i 's/ -M rpki//' %{_sysconfdir}/frr/daemons
%files devel
%{_libdir}/lib*.so
%dir %{_includedir}/%{name}
%{_includedir}/%{name}/*.h
# Core config header
%{_includedir}/%{name}/config.h
# Core library headers
%dir %{_includedir}/%{name}/lib
%{_includedir}/%{name}/lib/*.h
%{_includedir}/%{name}/lib/*/*.h
# Zebra headers
%dir %{_includedir}/%{name}/zebra
%{_includedir}/%{name}/zebra/*.h
# Optional daemon headers (still under frr/)
%dir %{_includedir}/%{name}/ospfd
%{_includedir}/%{name}/ospfd/*.h
%if %{with_bfdd}
Expand Down
5 changes: 4 additions & 1 deletion zebra/subdir.am
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ clippy_scan += \
zebra/zebra_cli.c \
# end

noinst_HEADERS += \
nobase_pkginclude_HEADERS += \
zebra/connected.h \
zebra/debug.h \
zebra/if_netlink.h \
Expand Down Expand Up @@ -200,6 +200,9 @@ noinst_HEADERS += \
zebra/zebra_l2_bridge_if.h \
zebra/zebra_vxlan_if.h \
zebra/zserv.h \
# end

noinst_HEADERS += \
zebra/dpdk/zebra_dplane_dpdk.h \
zebra/dpdk/zebra_dplane_dpdk_private.h \
# end
Expand Down
Loading