After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 670542 - Add version information for deprecations
Add version information for deprecations
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-02-21 14:00 UTC by Emmanuele Bassi (:ebassi)
Modified: 2012-02-27 06:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gmacros: Add G_GNUC_UNAVAILABLE (965 bytes, patch)
2012-02-21 14:00 UTC, Emmanuele Bassi (:ebassi)
accepted-commit_now Details | Review
gmacros: Add flexible API version boundaries (6.67 KB, patch)
2012-02-21 14:00 UTC, Emmanuele Bassi (:ebassi)
reviewed Details | Review
value array: Annotate with versioned deprecation (2.75 KB, patch)
2012-02-21 14:01 UTC, Emmanuele Bassi (:ebassi)
accepted-commit_now Details | Review
Add versioned deprecation annotation (12.12 KB, patch)
2012-02-21 14:01 UTC, Emmanuele Bassi (:ebassi)
accepted-commit_now Details | Review
Add flexible API version boundaries/v2.0 (10.86 KB, patch)
2012-02-24 13:58 UTC, Emmanuele Bassi (:ebassi)
reviewed Details | Review
value array: Annotate with versioned deprecation (2.75 KB, patch)
2012-02-24 13:58 UTC, Emmanuele Bassi (:ebassi)
accepted-commit_now Details | Review
Add versioned deprecation annotation (12.12 KB, patch)
2012-02-24 13:58 UTC, Emmanuele Bassi (:ebassi)
accepted-commit_now Details | Review

Description Emmanuele Bassi (:ebassi) 2012-02-21 14:00:23 UTC
The deprecation strategy we use is "all or nothing" you either enable deprecation warnings for every single deprecated symbol, or you don't, and you get to keep both the pieces when something will break.

While this is acceptable for projects that closely follow the GNOME platform's development cycle, it may not be optimal for projects outside our purview — ISVs and OSVs, for instance, may update their products on a longer cycle, and resynchronize the the platform every 12/18 months, which means three cycles worth of deprecations have to be corrected all in one go.

A potential way out is to use versioned annotations for deprecated symbols, and use a way to define the lower and upper boundaries of the used API; anything that was deprecated after the lower boundary does not warn, and everything that was introduced after the upper boundary will give an undefined symbol warning. As prior art, this mechanism is what Apple uses the for their platform API, as you can see here:

  http://opensource.apple.com/source/CarbonHeaders/CarbonHeaders-18.1/AvailabilityMacros.h
  https://developer.apple.com/library/mac/#releasenotes/Cocoa/AppKit.html

The following patchset implements something similar to the AvailabilityMacros.h, without going into weird defines like GLIB_AVAILABLE_IN_2_32_AND_DEPRECATED_IN_2_36. It should be possible to play around with the pre-processor to chain up defines, or encode the version information as arguments, for instance, but as a first rough version it is working fairly well already. I also attached a couple of commits that port all the symbols deprecated after GLib 2.26 to the new versioning scheme.
Comment 1 Emmanuele Bassi (:ebassi) 2012-02-21 14:00:37 UTC
Created attachment 208114 [details] [review]
gmacros: Add G_GNUC_UNAVAILABLE

A macro to annotate a function as not available, when compiling with GCC.
Comment 2 Emmanuele Bassi (:ebassi) 2012-02-21 14:00:49 UTC
Created attachment 208115 [details] [review]
gmacros: Add flexible API version boundaries

There are cases when it should be possible to define at compile time
what range of functions and types should be used, in order to get,
or restrict, the compiler warnings for deprecated or newly added
types or functions.

For instance, if GLib introduces a deprecation warning on a type in
version 2.32, application code can decide to specify the minimum and
maximum boundary of the used API to be 2.30; when compiling against
a new version of GLib, this would produce the following results:

  - all deprecations introduced prior to 2.32 would emit compiler
    warnings when used by the application code;
  - all deprecations introduced in 2.32 would not emit compiler
    warnings when used by the application code;
  - all new symbols introduced in 2.32 would emit a compiler warning.

Using this scheme it should be possible to have fairly complex
situations, like the following one:

  assuming that an application is compiled with:
    GLIB_VERSION_MIN_REQUIRED = GLIB_VERSION_2_30
    GLIB_VERSION_MAX_ALLOWED  = GLIB_VERSION_2_32

  and a GLib header containing:

    void function_A (void) GLIB_DEPRECATED_IN_2_26;
    void function_B (void) GLIB_DEPRECATED_IN_2_28;
    void function_C (void) GLIB_DEPRECATED_IN_2_30;
    void function_D (void) GLIB_AVAILABLE_IN_2_32;
    void function_E (void) GLIB_AVAILABLE_IN_2_34;

  any application code using the above functions will get the following
  compiler warnings:

    function_A: deprecated symbol warning
    function_B: deprecated symbol warning
    function_C: no warning
    function_D: no warning
    function_E: undefined symbol warning

This means that it should be possible to gradually port code towards
non-deprecated API gradually, on a per-release basis.
Comment 3 Emmanuele Bassi (:ebassi) 2012-02-21 14:01:03 UTC
Created attachment 208116 [details] [review]
value array: Annotate with versioned deprecation
Comment 4 Emmanuele Bassi (:ebassi) 2012-02-21 14:01:10 UTC
Created attachment 208117 [details] [review]
Add versioned deprecation annotation

We start from GLib 2.26.
Comment 5 Dan Winship 2012-02-21 14:57:48 UTC
I like the idea of emitting warnings if you use API newer than what you claimed to need in configure.ac... I accidentally "broke" NetworkManager with some too-new glib API last week.
Comment 6 Matthias Clasen 2012-02-22 07:58:00 UTC
Review of attachment 208114 [details] [review]:

Should add a this macro to the docs too. Otherwise, looks fine
Comment 7 Matthias Clasen 2012-02-22 08:07:11 UTC
Review of attachment 208115 [details] [review]:

Should the version macros go into gversion.h, maybe ?

But I like it, in general.
Comment 8 Matthias Clasen 2012-02-22 08:07:13 UTC
Review of attachment 208115 [details] [review]:

Should the version macros go into gversion.h, maybe ?

But I like it, in general.
Comment 9 Matthias Clasen 2012-02-22 08:09:26 UTC
Review of attachment 208116 [details] [review]:

ok
Comment 10 Matthias Clasen 2012-02-22 08:11:17 UTC
Review of attachment 208117 [details] [review]:

ok
Comment 11 Matthias Clasen 2012-02-22 08:11:17 UTC
Review of attachment 208117 [details] [review]:

ok
Comment 12 Emmanuele Bassi (:ebassi) 2012-02-22 08:35:52 UTC
(In reply to comment #7)
> Review of attachment 208115 [details] [review]:
> 
> Should the version macros go into gversion.h, maybe ?

yeah, I was wondering that myself.

a new header would definitely look better than dumping all this stuff inside gmacros.h - which, admittedly, is already hard to follow in parts.

I'll rework the patch.
Comment 13 Emmanuele Bassi (:ebassi) 2012-02-22 08:40:10 UTC
it would also be nice to have a m4 macro to define the MIN_REQUIRED and MAX_ALLOWED range automatically when checking for GLib, something like:

  GLIB_VERSION_RANGE([MIN_REQUIRED], [MAX_ALLOWED])

which manipulates the CPPFLAGS variable.
Comment 14 Colin Walters 2012-02-22 14:59:05 UTC
In the big picture Windows has supported this sort of thing for a long time, and a lot of developers like it a lot.  It makes total sense. 

The downside is that it'd just be for GLib...to be *truly* useful we'd need to go both up the stack (to GTK+) and down the stack (to glibc).

It's really glibc's insistence on opting you in to the latest SSE version 673 strcmp() that shaves THREE WHOLE NANOSECONDS when you build against the newer headers that sabotages any work above.
Comment 15 Matthias Clasen 2012-02-23 02:01:20 UTC
Unfortunately,
it seems that __attribute__((unavailable)) is unavailable itself :-( 
My gcc 4.7 doesn't seem to know anything about it. Quick googling seems to indicate that it is something that may be clang-only and hasn't been added to gcc yet ?
Comment 16 Matthias Clasen 2012-02-23 03:12:36 UTC
Few more thoughts:

- we can of course do without unavailable, by using deprecated("too new") instead

- I'd like to see a comment in the header that explains what changes need to be
  made when a new stable version is released:
  - add a new GLIB_VERSION_2_x macro
  - update the default values for the min and max
  - anything else ?

- Should we generate a macro for GLIB_CURRENT_VERSION and use that instead of 
  the hardcoded GLIB_VERSION_2_32 in some places ?
Comment 17 Emmanuele Bassi (:ebassi) 2012-02-24 12:27:44 UTC
(In reply to comment #15)
> Unfortunately,
> it seems that __attribute__((unavailable)) is unavailable itself :-( 

ugh.

> My gcc 4.7 doesn't seem to know anything about it. Quick googling seems to
> indicate that it is something that may be clang-only and hasn't been added to
> gcc yet ?

reading around in the gcc list archives, it seems they favour using ((deprecated)) instead of ((unavailable)) which makes sense in a "ok, deprecated, unavailable" progression, but it is a bit of an abuse in the way I'd like to use it (i.e. a warning for a symbol that is not *yet* available, given the version constraints).

(In reply to comment #16)
> Few more thoughts:
> 
> - we can of course do without unavailable, by using deprecated("too new")
> instead
> 
> - I'd like to see a comment in the header that explains what changes need to be
>   made when a new stable version is released:
>   - add a new GLIB_VERSION_2_x macro
>   - update the default values for the min and max
>   - anything else ?
> 
> - Should we generate a macro for GLIB_CURRENT_VERSION and use that instead of 
>   the hardcoded GLIB_VERSION_2_32 in some places ?

using something like: __attribute__((deprecated("Symbol not available given the versions constraint"))) is fine by me.

I'll update the patchset with the changes outlined.
Comment 18 Emmanuele Bassi (:ebassi) 2012-02-24 13:58:08 UTC
Created attachment 208350 [details] [review]
Add flexible API version boundaries/v2.0

I removed G_GNUC_UNAVAILABLE, given that it's an Apple extension; I use the deprecated attribute with a custom message, instead.

I moved the version definition macros to a separate header; I could not use gversion.h because otherwise I'd have to include it inside gtypes.h, which would create a cycle, or include gversion.h into every glib/* header.

I added two defines, one for the current (stable) version, and the other for the previous (stable) one. These defines allow the MIN_ALLOWED and MAX_REQUIRED defines to always default to the correct value without human intervention.

The header and configure.ac have notes to remind whoever is doing the minor version bump to also add the new defines when needed.
Comment 19 Emmanuele Bassi (:ebassi) 2012-02-24 13:58:16 UTC
Created attachment 208351 [details] [review]
value array: Annotate with versioned deprecation
Comment 20 Emmanuele Bassi (:ebassi) 2012-02-24 13:58:23 UTC
Created attachment 208352 [details] [review]
Add versioned deprecation annotation

We start from GLib 2.26.
Comment 21 Matthias Clasen 2012-02-24 23:49:56 UTC
Review of attachment 208350 [details] [review]:

::: glib/gversionmacros.h
@@ +130,3 @@
+#  define GLIB_VERSION_MAX_ALLOWED      GLIB_VERSION_MIN_REQUIRED
+# else
+#  define GLIB_VERSION_MAX_ALLOWED      GLIB_VERSION_PREV_STABLE

Did you mean GLIB_VERSION_CUR_STABLE here ?
I would have expected cur_stable to be used in one of these definitions
Comment 22 Matthias Clasen 2012-02-24 23:51:04 UTC
Review of attachment 208351 [details] [review]:

ok
Comment 23 Matthias Clasen 2012-02-24 23:51:39 UTC
Review of attachment 208352 [details] [review]:

sure
Comment 24 Matthias Clasen 2012-02-27 06:57:16 UTC
Committed, with some docs tweaks. I'll commit the equivalent thing to gtk soon.