818771 - Upgrade SQLite in NSS CVS to 3.7.14.1

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

This bug is tracking the upstream solution for SQLite to build on Android using the NDK so that it works on Android 2.2 and higher, without hacks in our build system.

You can see one (backed-out) attempt here: http://hg.mozilla.org/mozilla-central/diff/b854ffeef0d1/db/sqlite3/src/sqlite3.c

That approach probably works fine, but we backed it out because we have a policy of not adding our own private patches to sqlite.

Our standalone NSS build system also runs into this issue. See bug 772144.

Once upstream SQLite supports Android properly, we can remove the hacks to our build system.

Comment 1

[D. Richard Hipp]

I think the issue has already been addressed here: http://www.sqlite.org/src/info/70b5b309568ac5

The fix appeared in SQLite version 3.7.8, released on 2011-09-19.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Thanks for clarifying that. FYI, we actually maintain two copies of SQLite for Mozilla: one in Gecko, and one in NSS. The Mozilla version indeed has been modified to include the fix for the fdatasync issue on Android. However, the version of SQLite in the NSS CVS repo is still 3.6.22, which is about two years older than the version in mozilla-central.

I am morphing this bug to be an NSS bug that tracks the landing of SQLite 3.7.14.1 into the NSS CVS repository. Doing so will put the NSS CVS repository in sync with mozilla-central's version of SQLite, and will eliminate the need for us to specify -Dfdatasync=fsync in our the config.mk we created for SQLite in NSS.

In general, it is good for Mozilla to keep the version of SQLite in NSS in sync with the version used in mozilla-central when possible, especially if/when we switch to the SQLite-based certificate DB format, so that we maximize the amount of testing of that version of SQLite.

Comment 3

[Marco Bonardo [:mak]]

(In reply to Brian Smith (:bsmith) from comment #0)
> You can see one (backed-out) attempt here:
> http://hg.mozilla.org/mozilla-central/diff/b854ffeef0d1/db/sqlite3/src/
> sqlite3.c
> 
> That approach probably works fine, but we backed it out because we have a
> policy of not adding our own private patches to sqlite.

Apart that, the fix was also wrong, since it was much easier to just modify the Makefile to provide an fdatasync define on android.

Should we file an NSS upgrade bug every time we upgrade SQLite in central? We can do that.

Comment 4

[Kai Engert (:KaiE:)]

I would like to clarify that (in my understanding) this bug isn't a blocker to bug 772144.

Modifying NSS' private copy of sqlite to include the Android specific define shouldn't be an issue for Mozilla.

When building NSS as part of the Mozilla applications, the older copy of sqlite inside the NSS tree isn't used.

(A standalone NSS build builds libsqlite3.so, but in a Mozilla build tree you'll only get libmozsqlite3.so)

I conclude there isn't a need to hurry with this upgrade, in case the NSS team is worried for any reason.

Comment 5

[Wan-Teh Chang]

I didn't know this bug existed. I wanted to do this and already
wrote a patch. I will see if I should add the Android-specific
change to my patch.

Comment 6

[Wan-Teh Chang]

Attachment: Export sqlite3_wal_checkpoint

sqlite3_wal_checkpoint is a new function used by the Mac OS X
CFNetwork framework, so it needs to be exported when
DYLD_LIBRARY_PATH is set to point to NSS's libsqlite3.dylib.
Otherwise we may get this error when running many commands,
for example,

wtc-macbookair:sqlite wtc$ cvs diff sqlite.def
dyld: Symbol not found: _sqlite3_wal_checkpoint
  Referenced from: /System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork
  Expected in: /Users/wtc/nss-tip.2/mozilla/dist/Darwin12.2.0_64_DBG.OBJ/lib/libsqlite3.dylib
 in /System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork
cvs [diff aborted]: end of file from server (consult above messages if any)

Comment 7

[Wan-Teh Chang]

Comment on attachment 689564 [details] [diff] [review]
Export sqlite3_wal_checkpoint

The patch to update to SQLite 3.7.14.1 (containing sqlite3.c and sqlite3.h)
is too large to attach to Bugzilla. Please use this patch to indicate your
approval of the update to SQLite 3.7.14.1 as well.

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 689564 [details] [diff] [review]
Export sqlite3_wal_checkpoint

Today in the NSS teleconference, everybody agreed to upgrade to 3.7.14.1, so r+ for that change too.

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Wan-Teh Chang from comment #7)
> Comment on attachment 689564 [details] [diff] [review]
> Export sqlite3_wal_checkpoint
> 
> The patch to update to SQLite 3.7.14.1 (containing sqlite3.c and sqlite3.h)
> is too large to attach to Bugzilla. Please use this patch to indicate your
> approval of the update to SQLite 3.7.14.1 as well.

security/nss/lib/sqlite/README says that there is a local patch applied to support System V. Let's remove that comment in the README or even remove the README altogether. If such a patch is still necessary, then we should just upstream it and import in a new sqlite with the official fix.

Comment 10

[Wan-Teh Chang]

Attachment: Export sqlite3_wal_checkpoint, update README

I removed the obsolete comment that bsmith noted from README.

Patch checked in on the NSS trunk (NSS 3.14.2).

Checking in README;
/cvsroot/mozilla/security/nss/lib/sqlite/README,v  <--  README
new revision: 1.2; previous revision: 1.1
done
Checking in sqlite.def;
/cvsroot/mozilla/security/nss/lib/sqlite/sqlite.def,v  <--  sqlite.def
new revision: 1.7; previous revision: 1.6
done
Checking in sqlite3.c;
/cvsroot/mozilla/security/nss/lib/sqlite/sqlite3.c,v  <--  sqlite3.c
new revision: 1.7; previous revision: 1.6
done
Checking in sqlite3.h;
/cvsroot/mozilla/security/nss/lib/sqlite/sqlite3.h,v  <--  sqlite3.h
new revision: 1.4; previous revision: 1.3
done

Comment 11

[Wan-Teh Chang]

Attachment: Add a local change to fix Solaris compilation error

I found that the local change for Solaris is still necessary.
I know how to avoid it (don't compile with -D_SVID_GETTOD),
but I don't have time to work on this tonight.

Patch checked in on the NSS trunk (NSS 3.14.2).

Checking in README;
/cvsroot/mozilla/security/nss/lib/sqlite/README,v  <--  README
new revision: 1.3; previous revision: 1.2
done
Checking in sqlite3.c;
/cvsroot/mozilla/security/nss/lib/sqlite/sqlite3.c,v  <--  sqlite3.c
new revision: 1.8; previous revision: 1.7
done

Comment 12

[Wan-Teh Chang]

Attachment: Proper fix for the Solaris gettimeofday compilation error

This patch reverts the local change to sqlite (attachment 690701 [details] [diff] [review])
and fixes the Solaris gettimeofday() compilation error correctly.

Notes on the patch:

1. _SVID_GETTOD is the macro that Solaris's <sys/time.h> header
checks to declare gettimeofday() with only one argument. The
first change in the patch is to stop compiling with -D_SVID_GETTOD.

In a separate bug, I will remove support for Solaris versions older
than Solaris 8 (SunOS 5.8). So this patch only modifies SunOS5.x.mk
for Solaris 9-11 (SunOS 5.9-5.11). (Solaris 8 was released at the
same time as Windows XP, so it is safe to drop support for Solaris
versions older than Solaris 8.)

2. unix_rand.c: first note that unix_rand.c includes the following
headers at the beginning:
  #include <unistd.h>
  #include <sys/time.h>
  #include <sys/wait.h>

These three headers make it unnecessary to manually declare
gethostname() and gettimeofday() and include <wait.h>.

It is not necessary to include <sys/times.h> because we don't
call the times() function for Solaris.

Finally, 'sony' is a dead System V Unix variant, so we can
simply delete the gettimeofday(&tv) call.

Comment 13

[Wan-Teh Chang]

Comment on attachment 691046 [details] [diff] [review]
Proper fix for the Solaris gettimeofday compilation error

Patch checked in on the NSS trunk (NSS 3.14.2).

Checking in coreconf/SunOS5.10.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.10.mk,v  <--  SunOS5.10.mk
new revision: 1.4; previous revision: 1.3
done
Checking in coreconf/SunOS5.10_i86pc.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.10_i86pc.mk,v  <--  SunOS5.10_i86pc.mk
new revision: 1.5; previous revision: 1.4
done
Checking in coreconf/SunOS5.11.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.11.mk,v  <--  SunOS5.11.mk
new revision: 1.3; previous revision: 1.2
done
Checking in coreconf/SunOS5.11_i86pc.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.11_i86pc.mk,v  <--  SunOS5.11_i86pc.mk
new revision: 1.3; previous revision: 1.2
done
Checking in coreconf/SunOS5.8.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.8.mk,v  <--  SunOS5.8.mk
new revision: 1.4; previous revision: 1.3
done
Checking in coreconf/SunOS5.8_i86pc.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.8_i86pc.mk,v  <--  SunOS5.8_i86pc.mk
new revision: 1.6; previous revision: 1.5
done
Checking in coreconf/SunOS5.9.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.9.mk,v  <--  SunOS5.9.mk
new revision: 1.5; previous revision: 1.4
done
Checking in coreconf/SunOS5.9_i86pc.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.9_i86pc.mk,v  <--  SunOS5.9_i86pc.mk
new revision: 1.6; previous revision: 1.5
done
Checking in nss/lib/freebl/unix_rand.c;
/cvsroot/mozilla/security/nss/lib/freebl/unix_rand.c,v  <--  unix_rand.c
new revision: 1.42; previous revision: 1.41
done
Checking in nss/lib/sqlite/README;
/cvsroot/mozilla/security/nss/lib/sqlite/README,v  <--  README
new revision: 1.4; previous revision: 1.3
done
Checking in nss/lib/sqlite/sqlite3.c;
/cvsroot/mozilla/security/nss/lib/sqlite/sqlite3.c,v  <--  sqlite3.c
new revision: 1.9; previous revision: 1.8
done