[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: libotr transition started by mistake :-/



Hi,

[It's probably too late already, but if you prefer I can move this
discussion to a dedicated bug filed against release.d.o.]

Here's my analysis of the potential ABI/API breakage in libotr
4.0->4.1, and attached the debdiff (implementing plan A) that I could
upload to sid (and then request to be aged or unblocked so that it
migrates to testing eventually).

Functions added to context.h
============================

otrl_context_find_recent_instance
---------------------------------

ConnContext * otrl_context_find_recent_instance(ConnContext * context,
               otrl_instag_t recent_instag);

This one is not modified at all in context.c

otrl_context_find_recent_secure_instance
----------------------------------------

ConnContext * otrl_context_find_recent_secure_instance(ConnContext * context);

This one is slightly changed in context.c. Ignoring the typo fix in
a comment, the change is:

  -           if (context->msgstate != OTRL_MSGSTATE_PLAINTEXT) return 1;
  +           if (c_iter->msgstate != OTRL_MSGSTATE_PLAINTEXT) return 1;

I doubt this breaks the API or ABI.

Function added to both mem.c and mem.h
======================================

int otrl_mem_differ(const unsigned char *buf1, const unsigned char *buf2,
    size_t len);

My (limited) understanding is that adding a function does not break
the API nor ABI.

readelf
=======

I've diff'ed the output of the following command run on the one hand
on current Jessie (libotr 4.0.0-3) and current sid (libotr 4.1.0-1):

  readelf -Ws /usr/lib/libotr.so.5 | awk '{print $8}' | sort

The result is that two symbols were added in 4.1:

  gcry_mpi_snew@GCRYPT_1.6
  otrl_mem_differ

The full output lines from readelf -Ws, regarding the added or modified
functions, is:

  * 4.0:

    117: 0000000000007a50   123 FUNC    GLOBAL DEFAULT   11 otrl_context_find_recent_instance
    185: 0000000000007b20   292 FUNC    GLOBAL DEFAULT   11 otrl_context_find_recent_secure_instance

  * 4.1:

    118: 0000000000007af0   123 FUNC    GLOBAL DEFAULT   11 otrl_context_find_recent_instance
    181: 000000000000bd50   100 FUNC    GLOBAL DEFAULT   11 otrl_mem_differ
    187: 0000000000007bc0   292 FUNC    GLOBAL DEFAULT   11 otrl_context_find_recent_secure_instance

I lack the background to tell if the differences seen in the 1st (Num)
and 2nd (Value) columns matter. Do they indicate ABI breakage?

symbols
=======

I've extracted the libotr5 binary packages from Jessie and sid, and
then run:

  dpkg-gensymbols -v4.0.0 -plibotr5 -P/tmp/intrigeri/4.0-extracted -Osymbols
  dpkg-gensymbols -v4.1.0 -plibotr5 -P/tmp/intrigeri/4.1-extracted -Osymbols

The second command tells me:

  --- symbols (libotr5_4.1.0_amd64)
  +++ dpkg-gensymbolsjwu_Lf	2014-11-01 17:42:21.409469548 +0100
  @@ -51,6 +51,7 @@
    otrl_instag_read_FILEp@Base 4.0.0
    otrl_instag_write@Base 4.0.0
    otrl_instag_write_FILEp@Base 4.0.0
  + otrl_mem_differ@Base 4.1.0
    otrl_mem_init@Base 4.0.0
    otrl_message_abort_smp@Base 4.0.0
    otrl_message_disconnect@Base 4.0.0

... which seems to confirm the fact that there was no
API/ABI breakage.

Applications built against libotr 4.0 and running with 4.1
==========================================================

  * pidgin-otr 4.0.1-1 rebuilt in a Jessie chroot: works fine on
    current sid

=> seems to confirm that applications built against the old version
still work fine with the new one.

The attached patch
==================

I've gone with the smallest possible change (removing one single
line), and left the warning be printed on stderr, in the hope it may
help debugging issues in case something is wrong with this analysis.
If the RT prefers, I can drop the warning too.

I've successfully tested the resulting libotr5 binary package with:

  * pidgin-otr 4.0.1-1 built in a Jessie chroot (against libotr 4.0)
  * pidgin-otr 4.0.1-1 from sid (that was built against libotr 4.1)

... but I could not directly test the actual intended effect of the
patch (it would require building an _older_ version of libotr with
this patch, building a package against the new version, and running it
with the patched old version). This leads me to think that perhaps
this patch may not be as useful as I used to think.

Conclusion
==========

Unless the differences I've found in readelf output indicate ABI
breakage, it seems equally safe to either:

  * upload with the attached debdiff to sid, wait for it to build on
    all architectures, and then let it migrate to testing;
  * just let libotr migrate to testing as is (my current understanding
    is that it should fix two RC bugs, without introducing any new
    breakage).

So, whatever the release team feels more comfortable with :)

[Regarding the plans for Jessie+1, I've filed #767652 so we (pkg-otr
team) don't forget about it.]

Cheers,
-- 
intrigeri

diff -Nru libotr-4.1.0/debian/changelog libotr-4.1.0/debian/changelog
--- libotr-4.1.0/debian/changelog	2014-10-21 22:26:51.000000000 +0200
+++ libotr-4.1.0/debian/changelog	2014-11-01 18:40:49.000000000 +0100
@@ -1,3 +1,16 @@
+libotr (4.1.0-2) unstable; urgency=medium
+
+  * New patch:
+    0001-Do-not-error-out-when-an-application-is-run-against-.patch
+    Do not error out when an application is run against an older libotr
+    than the one it was built against. We'll be using shlibs and/or
+    symbols files to deal with that in Jessie+1 (see #767652). For Jessie,
+    given libotr 4.1 is API- and ABI-compatible with libotr 4.0, let's
+    prevent this runtime check from breaking packages that were built
+    against 4.0 (Closes: #767230).
+
+ -- intrigeri <intrigeri@debian.org>  Sat, 01 Nov 2014 18:36:49 +0100
+
 libotr (4.1.0-1) unstable; urgency=medium
 
   * New upstream release.
diff -Nru libotr-4.1.0/debian/patches/0001-Do-not-error-out-when-an-application-is-run-against-.patch libotr-4.1.0/debian/patches/0001-Do-not-error-out-when-an-application-is-run-against-.patch
--- libotr-4.1.0/debian/patches/0001-Do-not-error-out-when-an-application-is-run-against-.patch	1970-01-01 01:00:00.000000000 +0100
+++ libotr-4.1.0/debian/patches/0001-Do-not-error-out-when-an-application-is-run-against-.patch	2014-11-01 18:40:49.000000000 +0100
@@ -0,0 +1,28 @@
+From: intrigeri <intrigeri@boum.org>
+Date: Sat, 1 Nov 2014 17:20:52 +0000
+Bug-Debian: https://bugs.debian.org/767230
+Forwarded: not-needed
+Subject: Do not error out when an application is run against an older libotr
+ than the one it was built against (Closes: #767230).
+
+We'll be using shlibs and/or symbols files to deal with that in Jessie+1
+(see #767652). For Jessie, given libotr 4.1 is API- and ABI-compatible with
+libotr 4.0, let's prevent this runtime check from breaking packages that were
+built against 4.0.
+
+---
+ src/proto.c | 1 -
+ 1 file changed, 1 deletion(-)
+
+diff --git a/src/proto.c b/src/proto.c
+index f560a82..7fc0fc5 100644
+--- a/src/proto.c
++++ b/src/proto.c
+@@ -61,7 +61,6 @@ gcry_error_t otrl_init(unsigned int ver_major, unsigned int ver_minor,
+ 		"with actual version %u.%u.%u.  Aborting.\n",
+ 		ver_major, ver_minor, ver_sub,
+ 		OTRL_VERSION_MAJOR, OTRL_VERSION_MINOR, OTRL_VERSION_SUB);
+-	return gcry_error(GPG_ERR_INV_VALUE);
+     }
+ 
+     /* Set the API version.  If we get called multiple times for some
diff -Nru libotr-4.1.0/debian/patches/series libotr-4.1.0/debian/patches/series
--- libotr-4.1.0/debian/patches/series	1970-01-01 01:00:00.000000000 +0100
+++ libotr-4.1.0/debian/patches/series	2014-11-01 18:40:49.000000000 +0100
@@ -0,0 +1 @@
+0001-Do-not-error-out-when-an-application-is-run-against-.patch

Reply to: