Bug 14141 – Please add a version to the Gtk port

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Mike Hommey]

Please add a VERSION variable in WebCore/WebCore.pro so that the QT and Gtk port libraries have a "proper" SONAME.
Please then update this VERSION whenever the ABI is broken.

[Alp Toker]

I think we're pretty much ready for versioning.

We should introduce macros similar to what GTK+ provides for version checking.

We'll probably additionally want to allow ways to test the WebCore version at runtime.

Patches welcome.

[Christian Dywan]

(In reply to comment #1)
> I think we're pretty much ready for versioning.

Well, very close, but not quite ready yet unfortunately.

Adding the following bugs that touch fundamental API as dependencies:
  bug 17176, bug 17066, bug 16584, bug 16562, bug 16624, bug 17171

I don't know what the state of some of these bugs is, I hope this won't take too long.

[Alp Toker]

(In reply to comment #2)
> (In reply to comment #1)
> > I think we're pretty much ready for versioning.
> 
> Well, very close, but not quite ready yet unfortunately.
> 
> Adding the following bugs that touch fundamental API as dependencies:
>   bug 17176, bug 17066, bug 16584, bug 16562, bug 16624, bug 17171

Now that the .pc package name and header locations are fixed, it's time to introduce versioning macros ASAP. Perhaps that should be done in a different bug.

I think we should consider using versioning, at least as a "test run", very soon. It might make your planned API/ABI changes less painful to early adopters.

[Christian Dywan]

Created attachment 21465 [details]
Add version macros

This patch implements version macros and functions similar to what glib and gtk have.

[Alp Toker]

Comment on attachment 21465 [details]
Add version macros

>diff --git a/GNUmakefile.am b/GNUmakefile.am
>index 3118722..bd86ced 100644
>--- a/GNUmakefile.am
>+++ b/GNUmakefile.am
>@@ -294,6 +294,7 @@ webkitgtk_h_api += \
> 	WebKit/gtk/webkit/webkit.h \
> 	WebKit/gtk/webkit/webkitdefines.h \
> 	WebKit/gtk/webkit/webkitnetworkrequest.h \
>+	WebKit/gtk/webkit/webkitversion.h \
> 	WebKit/gtk/webkit/webkitwebbackforwardlist.h \
> 	WebKit/gtk/webkit/webkitwebframe.h \
> 	WebKit/gtk/webkit/webkitwebhistoryitem.h \
>@@ -317,6 +318,7 @@ webkitgtk_headers += \
> webkitgtk_sources += \
> 	WebKit/gtk/webkit/webkitnetworkrequest.cpp \
> 	WebKit/gtk/webkit/webkitprivate.cpp \
>+	WebKit/gtk/webkit/webkitversion.cpp \
> 	WebKit/gtk/webkit/webkitwebbackforwardlist.cpp \
> 	WebKit/gtk/webkit/webkitwebframe.cpp \
> 	WebKit/gtk/webkit/webkitwebhistoryitem.cpp \
>diff --git a/WebKit/gtk/webkit/webkit.h b/WebKit/gtk/webkit/webkit.h
>index babd50a..55d5b56 100644
>--- a/WebKit/gtk/webkit/webkit.h
>+++ b/WebKit/gtk/webkit/webkit.h
>@@ -20,6 +20,7 @@
> #ifndef __WEBKIT_H__
> #define __WEBKIT_H__
> 
>+#include <webkit/webkitversion.h>
> #include <webkit/webkitdefines.h>
> #include <webkit/webkitnetworkrequest.h>
> #include <webkit/webkitwebframe.h>
>diff --git a/WebKit/gtk/webkit/webkitversion.cpp b/WebKit/gtk/webkit/webkitversion.cpp
>new file mode 100644
>index 0000000..ea78993
>--- /dev/null
>+++ b/WebKit/gtk/webkit/webkitversion.cpp
>@@ -0,0 +1,57 @@
>+/*
>+ * Copyright (C) 2008 Christian Dywan <[e-mail]>
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public License
>+ * along with this library; see the file COPYING.LIB.  If not, write to
>+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>+ * Boston, MA 02110-1301, USA.
>+ */
>+
>+#include "webkitversion.h"
>+

I believe everything after the includes should be wrapped with extern "C". Check with me if in doubt.

>+/**
>+ * webkit_major_version:
>+ *
>+ * The major version number of the WebKit that is linked against.
>+ *
>+ * Return value: The major version
>+ */
>+guint webkit_major_version(void)
>+{
>+    return WEBKIT_MAJOR_VERSION;
>+}

Please remove the empty void from the implementations (though it's correct to have them in the header).

Should be only one newline below..

>+
>+
>+/**
>+ * webkit_minor_version:
>+ *
>+ * The minor version number of the WebKit that is linked against.
>+ *
>+ * Return value: The minor version
>+ */
>+guint webkit_minor_version(void)
>+{
>+    return WEBKIT_MINOR_VERSION;
>+}
>+
>+/**
>+ * webkit_micro_version:
>+ *
>+ * The micro version number of the WebKit that is linked against.
>+ *
>+ * Return value: The micro version
>+ */
>+guint webkit_micro_version(void)
>+{
>+    return WEBKIT_MICRO_VERSION;
>+}
>diff --git a/WebKit/gtk/webkit/webkitversion.h b/WebKit/gtk/webkit/webkitversion.h
>new file mode 100644
>index 0000000..29c96fa
>--- /dev/null
>+++ b/WebKit/gtk/webkit/webkitversion.h
>@@ -0,0 +1,52 @@
>+/*
>+ * Copyright (C) 2008 Christian Dywan <[e-mail]>
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Library General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Library General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Library General Public License
>+ * along with this library; see the file COPYING.LIB.  If not, write to
>+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>+ * Boston, MA 02110-1301, USA.
>+ */
>+
>+#ifndef WEBKIT_VERSION_H
>+#define WEBKIT_VERSION_H
>+
>+#include <glib.h>
>+#include <webkit/webkitdefines.h>
>+
>+G_BEGIN_DECLS
>+
>+#define WEBKIT_MAJOR_VERSION 1
>+#define WEBKIT_MINOR_VERSION 0
>+#define WEBKIT_MICRO_VERSION 1

^ I wonder if we should hook this up to autoconf from the get-go.

>+
>+#define WEBKIT_CHECK_VERSION(major, minor, micro) \
>+  (WEBKIT_MAJOR_VERSION > (major) || \
>+  (WEBKIT_MAJOR_VERSION == (major) && WEBKIT_MINOR_VERSION > (minor)) || \
>+  (WEBKIT_MAJOR_VERSION == (major) && WEBKIT_MINOR_VERSION == (minor) && \
>+  WEBKIT_MICRO_VERSION >= (micro)))
>+
>+WEBKIT_API guint
>+webkit_major_version (void);
>+
>+WEBKIT_API guint
>+webkit_minor_version (void);
>+
>+WEBKIT_API guint
>+webkit_micro_version (void);
>+
>+WEBKIT_API gboolean
>+webkit_check_version (guint major, guint minor, guint micro);
>+
>+G_END_DECLS
>+
>+#endif
>diff --git a/WebKit/gtk/webkit/webkitwebsettings.cpp b/WebKit/gtk/webkit/webkitwebsettings.cpp
>index 26d60b3..08e409d 100644
>--- a/WebKit/gtk/webkit/webkitwebsettings.cpp
>+++ b/WebKit/gtk/webkit/webkitwebsettings.cpp
>@@ -252,6 +252,13 @@ static void webkit_web_settings_class_init(WebKitWebSettingsClass* klass)
>                                     0,
>                                     flags));
> 
>+    /**
>+    * WebKitWebSettings:zoom-step:
>+    *
>+    * The value by which the zoom level is changed when zooming in or out.
>+    *
>+    * Since: 1.0.1
>+    */
>     g_object_class_install_property(gobject_class,
>                                     PROP_ZOOM_STEP,
>                                     g_param_spec_float(
>diff --git a/WebKit/gtk/webkit/webkitwebview.cpp b/WebKit/gtk/webkit/webkitwebview.cpp
>index d831c74..9de77f5 100644
>--- a/WebKit/gtk/webkit/webkitwebview.cpp
>+++ b/WebKit/gtk/webkit/webkitwebview.cpp
>@@ -1220,6 +1220,13 @@ static void webkit_web_view_class_init(WebKitWebViewClass* webViewClass)
>                                                          FALSE,
>                                                          WEBKIT_PARAM_READWRITE));
> 
>+    /**
>+    * WebKitWebView:zoom-level:
>+    *
>+    * The level of zoom of the content.
>+    *
>+    * Since: 1.0.1
>+    */
>     g_object_class_install_property(objectClass, PROP_ZOOM_LEVEL,
>                                     g_param_spec_float("zoom-level",
>                                                        "Zoom level",
>@@ -1229,6 +1236,13 @@ static void webkit_web_view_class_init(WebKitWebViewClass* webViewClass)
>                                                        1.0f,
>                                                        WEBKIT_PARAM_READWRITE));
> 
>+    /**
>+    * WebKitWebView:full-content-zoom:
>+    *
>+    * Whether the full content is scaled when zooming.
>+    *
>+    * Since: 1.0.1
>+    */
>     g_object_class_install_property(objectClass, PROP_FULL_CONTENT_ZOOM,
>                                     g_param_spec_boolean("full-content-zoom",
>                                                          "Full content zoom",
>@@ -2065,6 +2079,8 @@ void webkit_web_view_set_transparent(WebKitWebView* webView, gboolean flag)
>  * elements in the page.
>  *
>  * Return value: the zoom level of @web_view
>+ *
>+ * Since: 1.0.1
>  */
> gfloat webkit_web_view_get_zoom_level(WebKitWebView* webView)
> {
>@@ -2097,6 +2113,8 @@ static void webkit_web_view_apply_zoom_level(WebKitWebView* webView, gfloat zoom
>  * If the "full-content-zoom" property is set to %FALSE (the default)
>  * the zoom level changes the text size, or if %TRUE, scales all
>  * elements in the page.
>+ *
>+ * Since: 1.0.1
>  */
> void webkit_web_view_set_zoom_level(WebKitWebView* webView, gfloat zoomLevel)
> {
>@@ -2113,6 +2131,8 @@ void webkit_web_view_set_zoom_level(WebKitWebView* webView, gfloat zoomLevel)
>  * Increases the zoom level of @web_view. The current zoom
>  * level is incremented by the value of the "zoom-step"
>  * property of the #WebKitWebSettings associated with @web_view.
>+ *
>+ * Since: 1.0.1
>  */
> void webkit_web_view_zoom_in(WebKitWebView* webView)
> {
>@@ -2132,6 +2152,8 @@ void webkit_web_view_zoom_in(WebKitWebView* webView)
>  * Decreases the zoom level of @web_view. The current zoom
>  * level is decremented by the value of the "zoom-step"
>  * property of the #WebKitWebSettings associated with @web_view.
>+ *
>+ * Since: 1.0.1
>  */
> void webkit_web_view_zoom_out(WebKitWebView* webView)
> {
>@@ -2152,6 +2174,8 @@ void webkit_web_view_zoom_out(WebKitWebView* webView)
>  *
>  * Return value: %FALSE if only text should be scaled (the default),
>  * %TRUE if the full content of the view should be scaled.
>+ *
>+ * Since: 1.0.1
>  */
> gboolean webkit_web_view_get_full_content_zoom(WebKitWebView* webView)
> {
>@@ -2168,6 +2192,8 @@ gboolean webkit_web_view_get_full_content_zoom(WebKitWebView* webView)
>  * %TRUE if the full content of the view should be scaled.
>  *
>  * Sets whether the zoom level affects only text or all elements.
>+ *
>+ * Since: 1.0.1
>  */
> void webkit_web_view_set_full_content_zoom(WebKitWebView* webView, gboolean zoomFullContent)
> {

[Christian Dywan]

Created attachment 21468 [details]
Add version macros, with autotool magic

Fixed according to comments including integration with autotools.

[Alp Toker]

(In reply to comment #6)
> Created an attachment (id=21468) [edit]
> Add version macros, with autotool magic
> 
> Fixed according to comments including integration with autotools.
> 

+LIBWEBKITGTK_VERSION=webkit_major_version:webkit_minor_version:webkit_micro_version

^ Trying to remember what we decided (we were reading sjoerd's mail on soname versioning IIRC). Is this safe?

[Sebastian Dröge (slomo)]

No, it's plain wrong.

You should forget about any correlation between the "version number", i.e. webkit 1.0.1.2.3 and the "library version".

The correct thing to do is to start at 0:0:0 and then apply the following rules before every release:

dnl ################################################################
dnl # libtool versioning
dnl ################################################################
dnl #
dnl # +1 :  0 : +1   == new interface that does not break old one.
dnl # +1 :  0 :  0   == removed an interface. Breaks old apps.
dnl #  ? : +1 :  ?   == internal changes that doesn't break anything.
dnl #
dnl # CURRENT : REVISION : AGE
dnl #
LT_CURRENT=2
LT_REVISION=2
LT_AGE=1


LT_* are then passed to the -version-info parameter like
-version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE)


This will then care for correct numbers at the end of the filenames of the library ;)

[Christian Dywan]

Created attachment 21513 [details]
Correct libtool version

This patch has a proper libtool version and a little table as well as a libtool documentation link.

We don't gurarantee stability yet, so don't rely on this ;)

[Christian Dywan]

Committed in r34387. Left out libtool versioning as discussed in IRC.

[Alp Toker]

Comment on attachment 21513 [details]
Correct libtool version

Landed already.

[Sebastian Dröge (slomo)]

Ok, so is there a new bug about the libtool versioning?