Bug 14141 - Please add a version to the Gtk port
Summary: Please add a version to the Gtk port
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Linux
: P2 Enhancement
Assignee: Christian Dywan
URL:
Keywords: Gtk
Depends on: 16562 16584 16624 17066 17171 17176
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-14 09:02 PDT by Mike Hommey
Modified: 2008-06-06 01:07 PDT (History)
6 users (show)

See Also:


Attachments
Add version macros (8.67 KB, patch)
2008-06-02 11:48 PDT, Christian Dywan
no flags Details | Formatted Diff | Diff
Add version macros, with autotool magic (9.92 KB, patch)
2008-06-02 14:44 PDT, Christian Dywan
no flags Details | Formatted Diff | Diff
Correct libtool version (10.37 KB, patch)
2008-06-05 15:12 PDT, Christian Dywan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Hommey 2007-06-14 09:02:33 PDT
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.
Comment 1 Alp Toker 2008-02-03 23:32:09 PST
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.
Comment 2 Christian Dywan 2008-02-04 11:13:36 PST
(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.
Comment 3 Alp Toker 2008-02-08 04:02:34 PST
(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.
Comment 4 Christian Dywan 2008-06-02 11:48:35 PDT
Created attachment 21465 [details]
Add version macros

This patch implements version macros and functions similar to what glib and gtk have.
Comment 5 Alp Toker 2008-06-02 12:00:23 PDT
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 <christian@imendio.com>
>+ *
>+ * 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 <christian@imendio.com>
>+ *
>+ * 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)
> {
Comment 6 Christian Dywan 2008-06-02 14:44:03 PDT
Created attachment 21468 [details]
Add version macros, with autotool magic

Fixed according to comments including integration with autotools.
Comment 7 Alp Toker 2008-06-04 23:10:22 PDT
(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?
Comment 8 Sebastian Dröge (slomo) 2008-06-05 05:28:05 PDT
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 ;)
Comment 9 Christian Dywan 2008-06-05 15:12:14 PDT
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 ;)
Comment 10 Christian Dywan 2008-06-05 15:53:16 PDT
Committed in r34387. Left out libtool versioning as discussed in IRC.
Comment 11 Alp Toker 2008-06-05 20:57:11 PDT
Comment on attachment 21513 [details]
Correct libtool version

Landed already.
Comment 12 Sebastian Dröge (slomo) 2008-06-06 01:07:39 PDT
Ok, so is there a new bug about the libtool versioning?