GNOME Bugzilla – Bug 767587
Use compiler directives to export symbols
Last modified: 2016-08-21 17:13:33 UTC
Hi, Like the rest of the GTK+ stack, I thought it might be a good idea to move the symbol exporting mechanism to use compiler directives when they are available, so that we no longer need to maintain .def/.symbols files for the export of symbols, meaning we can clean up things a bit. I will attach patches here for this purpose. With blessings, thank you!
Created attachment 329675 [details] [review] Add a header for version macros Hi, This adds a public header that is used to define the version macros like what we do in GTK+ and GLib, to indicate what stable release series an API is available or deprecated, as well as a macro that can be overridden to control the symbols that are exported...
Created attachment 329676 [details] [review] Decorate the symbols in the headers with the versioning macros Hi, This decorates the public symbols in the various Pango libraries with the version macros, so that each of them now has an indication of which stable release they are introduced or deprecated. The private symbols in PangoFT2 and PangoWin32 that are needed by PangoCairo are also decorated with the more private _PANGO_EXTERN, so that they can be exported with the public symbols by overriding _PANGO_EXTERN during the build...
Created attachment 329677 [details] [review] pango-enum-types.c.template: Include config.h first Hi, All of the sources that we build for the library include config.h first, except pango-enum-types.c.template, so do that there as well, as this is needed so that the enumeration symbols get exported properly as well...
Created attachment 329678 [details] [review] Update the autotools build files to export symbols using compiler directives Hi, Like what is done in GLib and GTK+, add checks in autotools to see whether we can export symbols using compiler directives (i.e. visibility), and so remove the .def files from the source tree and the check script that was used to ensure the .def files are properly updated, which should clean up things in the build files a bit. Also, update the Visual Studio build files to use __declspec(dllexport) that does the same purpose, so that we will no longer need the .def files to export the symbols. This is all the patches I have for this issue. With blessings, thank you!
+ * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. There is no such thing. There only exists a Library GPL version 2, and a Lesser GPL version 2.1. Also, the new pango-version-macros.h is clearly copied (with s/GLIB/PANGO/g and some minor adjustments) from glib/glib/gversionmacros.h, so you cannot just slap your own copyright on it while omitting their copyright notice which is * Copyright (C) 1995-1997 Peter Mattis, Spencer Kimball and Josh MacDonald [...] /* * Modified by the GLib Team and others 1997-2000. See the AUTHORS * file for a list of people on the GLib Team. See the ChangeLog * files for a list of changes. These files are distributed with * GLib at ftp://ftp.gtk.org/pub/gtk/. */ (actually glib's copyright notices are in bad shape, I think this file is actually mostly (c) by ebassi/mclasen/desrt not the people mentioned above).
Created attachment 329930 [details] [review] Add a header for version macros (with updated copyright notice) Hi Christian, Thanks for letting me know, and sorry for the credits that were due and not given properly. Here's the new patch that addresses the issues. With blessings, thank you!
Review of attachment 329677 [details] [review]: Looks good.
Review of attachment 329678 [details] [review]: Looks good. Did you check the exported symbols are still the same with i.e nm -D?
Review of attachment 329930 [details] [review]: Fix the minor issue and feel free to push it. ::: pango/pango-version-macros.h @@ +3,3 @@ + * Copyright (C) 1999 Red Hat Software + * Copyright (C) 2012 Ryan Lortie, Matthias Clasen and Emmanuele Bassi + * Copyright (C) 2016 Chun-wei Fan Just yourself on the copyright.
Review of attachment 329676 [details] [review]: This is great, quite a lot of work here. If you checked that the exported symbols are the same go for it. i.e running nm -D before and after these patches.
Created attachment 330243 [details] [review] Decorate the symbols in the headers with the versioning macros (take ii) Hi Nacho, Thanks for the tip on nm -D, it turns out there was a public symbol I missed for Pango, and a number of private symbols that were actually exported before the patch, so I marked them as well. I tried to do the best with the Coretext items, but since I don't run a Mac build environment, I did it with my best shot there in terms of the symbols. I am going to go back in using a former patch for attachment 329930 [details] [review], for the copyrights items, that was added due to Christian's notes, as you mentioned. With blessings, thank you!
Created attachment 330246 [details] [review] Decorate the symbols in the headers with the versioning macros (take ii) Sorry, somehow I had the wrong patch up...
Review of attachment 330246 [details] [review]: Looks good to me
Review of attachment 329675 [details] [review]: Looks good.
Hi Nacho, Thanks, I have pushed the patches as follows: Attachment 329675 [details]: 57964ef 330246: ce097c0 329677: 57964ef 329678: 1147da1 (a typo was fixed for PANGO_DEPRECATED_FOR(f) for attachment 329675 [details] [review]) With blessings, thank you!
The committed version *still* has the copyright wrong. This code *is* copied (with minor modifications) from elsewhere, so you *must* get the copyright notice right. Comment 9 was wrong.
Hi Christian, Thanks for clearing things up here, and sorry about this as a result. Pushed the changes as cce034f to fix the copyright items. With blessings, thank you!