I have to agree with Brian here. Cluttering up the API spec files with
#ifdefs seems ugly and unnecessary, and tying this into autotools at a spec
level also seems awkward.
The idea was to mark certain fields as deprecated in one release and then
remove them in a follow-on release. We can make the actual removal subject
to SC vote if the concern is that things might get removed too soon, as
there's no harm in carrying them with the deprecated markings until such
time as there is agreement they really can be removed. Allowing individual
developers to "reach back" to resurrect them just means that they have to
be kept around forever as they're never really removed.
On Thu, Feb 23, 2017 at 11:13 AM, Brian Brooks <brian.bro...@linaro.org>
wrote:
> On Thu, Feb 23, 2017 at 8:14 AM, Petri Savolainen
> <petri.savolai...@linaro.org> wrote:
> > Added ODP_DEPRECATED macro which is visible through API and
> > is used to control if deprecated API definitions are enabled
> > or disabled.
> >
> > Deprecated definitions are first moved behind this macro and
> > then removed completely in a later API version. Deprecated APIs
> > are disabled by default.
> >
> > Added configuration option --enable-deprecated to control the
> > macro value.
> >
> > Signed-off-by: Petri Savolainen <petri.savolai...@linaro.org>
> > ---
> > configure.ac | 19 +++++++++--
> > include/odp/api/spec/.gitignore | 1 +
> > include/odp/api/spec/deprecated.h.in | 38
> ++++++++++++++++++++++
> > include/odp_api.h | 1 +
> > platform/Makefile.inc | 1 +
> > platform/linux-generic/Makefile.am | 1 +
> > .../linux-generic/include/odp/api/deprecated.h | 26 +++++++++++++++
> > 7 files changed, 85 insertions(+), 2 deletions(-)
> > create mode 100644 include/odp/api/spec/deprecated.h.in
> > create mode 100644 platform/linux-generic/include/odp/api/deprecated.h
> >
> > diff --git a/configure.ac b/configure.ac
> > index e4bd3a7..9927916 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -16,7 +16,8 @@ ODP_VERSION_API_MAJOR=odpapi_major_version
> > AC_SUBST(ODP_VERSION_API_MAJOR)
> > ODP_VERSION_API_MINOR=odpapi_minor_version
> > AC_SUBST(ODP_VERSION_API_MINOR)
> > -AC_CONFIG_FILES([include/odp/api/spec/version.h])
> > +AC_CONFIG_FILES([include/odp/api/spec/version.h
> > + include/odp/api/spec/deprecated.h])
> >
> > AM_INIT_AUTOMAKE([1.9 tar-pax subdir-objects])
> > AC_CONFIG_SRCDIR([helper/config.h.in])
> > @@ -284,7 +285,7 @@ ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG"
> > ODP_ABI_COMPAT=1
> > abi_compat=yes
> > AC_ARG_ENABLE([abi-compat],
> > - [ --disable-abi-compat disables ABI compatible mode, enables
> inline code in header files],
> > + [ --disable-abi-compat disables ABI compatible mode, enables
> inline code in header files],
> > [if test "x$enableval" = "xno"; then
> > ODP_ABI_COMPAT=0
> > abi_compat=no
> > @@ -294,6 +295,19 @@ AC_ARG_ENABLE([abi-compat],
> > AC_SUBST(ODP_ABI_COMPAT)
> >
> > ############################################################
> ##############
> > +# Enable/disable deprecated API definitions
> > +###########################################################
> ###############
> > +ODP_DEPRECATED=0
> > +deprecated=no
> > +AC_ARG_ENABLE([deprecated],
> > + [ --enable-deprecated enable deprecated API definitions],
> > + [if test "x$enableval" = "xyes"; then
> > + ODP_DEPRECATED=1
> > + deprecated=yes
> > + fi])
> > +AC_SUBST(ODP_DEPRECATED)
> > +
> > +###########################################################
> ###############
> > # Default warning setup
> > ############################################################
> ##############
> > ODP_CFLAGS="$ODP_CFLAGS -W -Wall -Werror -Wstrict-prototypes
> -Wmissing-prototypes"
> > @@ -379,6 +393,7 @@ AC_MSG_RESULT([
> > static libraries: ${enable_static}
> > shared libraries: ${enable_shared}
> > ABI compatible: ${abi_compat}
> > + Deprecated APIs: ${deprecated}
> > cunit: ${cunit_support}
> > test_vald: ${test_vald}
> > test_perf: ${test_perf}
> > diff --git a/include/odp/api/spec/.gitignore b/include/odp/api/spec/.
> gitignore
> > index 6702033..df9c87d 100644
> > --- a/include/odp/api/spec/.gitignore
> > +++ b/include/odp/api/spec/.gitignore
> > @@ -1 +1,2 @@
> > +deprecated.h
> > version.h
> > diff --git a/include/odp/api/spec/deprecated.h.in
> b/include/odp/api/spec/deprecated.h.in
> > new file mode 100644
> > index 0000000..a319064
> > --- /dev/null
> > +++ b/include/odp/api/spec/deprecated.h.in
> > @@ -0,0 +1,38 @@
> > +/* Copyright (c) 2017, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier: BSD-3-Clause
> > + */
> > +
> > +/**
> > + * @file
> > + *
> > + * Macro for deprecated API definitions
> > + */
> > +
> > +#ifndef ODP_API_DEPRECATED_H_
> > +#define ODP_API_DEPRECATED_H_
> > +#include <odp/visibility_begin.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * Deprecated API definitions
> > + *
> > + * Some API definitions may be deprecated by this or a previous API
> version.
> > + * This macro controls if those are enabled (and visible to the
> application)
> > + * or disabled.
> > + *
> > + * * 0: Deprecated API definitions are disabled
> > + * * 1: Deprecated API definitions are enabled
> > + */
> > +#define ODP_DEPRECATED @ODP_DEPRECATED@
>
> I am confused here because this turns a compiler dependency into a
> build system + compiler dependency.
>
> Lots of libraries choose to use Autotools as a build system, but it's
> unnatural
> to require API header files like odp.h to also use Autotools. Anyone can
> grab
> these API header files and: include them in their application to use ODP,
> to
> create stubs for an ODP implementation, and most importantly they should
> be usable with any build system (Makefile, Autotools, CMake, ninja...).
>
> The original problem stated here is that the current code for the
> deprecated
> macro requires the use of GCC. The fix, or the way to address
> portability, can
> simply be to conditionally compile different code for different compilers:
>
> #ifdef __GCC__
> #define DEPRECATED __attribute__((deprecated))
> #elif MSCV
> #define DEPRECATED __declspec(deprecated)
> #else
> // define it to nothing... a nop...
> #define DEPRECATED
> // or... if an implementation must be required for all compilers:
> #error Please add support for your compiler in compiler.h
> #endif
>
> It is succinct an easy to maintain compared to embedding these portability
> concerns deeper into areas such as the build system and API header files.
>
> The same approach to portability can be used for architecturally
> specific C code as well as any other system specific macros provided by
> the compiler.
>
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#include <odp/visibility_end.h>
> > +#endif
> > diff --git a/include/odp_api.h b/include/odp_api.h
> > index 73e5309..962415f 100644
> > --- a/include/odp_api.h
> > +++ b/include/odp_api.h
> > @@ -18,6 +18,7 @@
> > extern "C" {
> > #endif
> >
> > +#include <odp/api/deprecated.h>
> > #include <odp/api/version.h>
> > #include <odp/api/std_types.h>
> > #include <odp/api/compiler.h>
> > diff --git a/platform/Makefile.inc b/platform/Makefile.inc
> > index 874cf88..f282770 100644
> > --- a/platform/Makefile.inc
> > +++ b/platform/Makefile.inc
> > @@ -29,6 +29,7 @@ odpapispecinclude_HEADERS = \
> > $(top_srcdir)/include/odp/api/spec/cpumask.h \
> > $(top_srcdir)/include/odp/api/spec/crypto.h \
> > $(top_srcdir)/include/odp/api/spec/debug.h \
> > + $(top_srcdir)/include/odp/api/spec/deprecated.h \
> > $(top_srcdir)/include/odp/api/spec/errno.h \
> > $(top_srcdir)/include/odp/api/spec/event.h \
> > $(top_srcdir)/include/odp/api/spec/hash.h \
> > diff --git a/platform/linux-generic/Makefile.am
> b/platform/linux-generic/Makefile.am
> > index deab284..242c54a 100644
> > --- a/platform/linux-generic/Makefile.am
> > +++ b/platform/linux-generic/Makefile.am
> > @@ -34,6 +34,7 @@ odpapiinclude_HEADERS = \
> > $(srcdir)/include/odp/api/cpumask.h \
> > $(srcdir)/include/odp/api/crypto.h \
> > $(srcdir)/include/odp/api/debug.h \
> > + $(srcdir)/include/odp/api/deprecated.h \
> > $(srcdir)/include/odp/api/errno.h \
> > $(srcdir)/include/odp/api/event.h \
> > $(srcdir)/include/odp/api/hash.h \
> > diff --git a/platform/linux-generic/include/odp/api/deprecated.h
> b/platform/linux-generic/include/odp/api/deprecated.h
> > new file mode 100644
> > index 0000000..82797eb
> > --- /dev/null
> > +++ b/platform/linux-generic/include/odp/api/deprecated.h
> > @@ -0,0 +1,26 @@
> > +/* Copyright (c) 2017, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier: BSD-3-Clause
> > + */
> > +
> > +/**
> > + * @file
> > + *
> > + * Control deprecated API definitions
> > + */
> > +
> > +#ifndef ODP_PLAT_DEPRECATED_H_
> > +#define ODP_PLAT_DEPRECATED_H_
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include <odp/api/spec/deprecated.h>
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> > --
> > 2.8.1
> >
>
