[sql] Rewrite sqlite patching "system".

[sql] Rewrite sqlite patching "system".

Rewrite the patch system to leverage git.  The patches are generated by
git-format-patch in a sub-directory.  A developer can regenerate a
version-control history using git-am, then use regular git tools to work
with the changelists.  Hopefully this will make it easier to keep track
of patches going forward, and also make it easier to merge Chromium
changes with upstream changes when importing new SQLite versions.

This CL should not materially modify Chromium's SQLite.  sqlite3.c has
these changes:
 - An ancient change to func.c has been dropped for lack of current
   support.
 - Code for initializing the recover virtual table was added for
   testing.  Chromium does not set DEFAULT_ENABLE_RECOVER to enable
   this, instead sql/recovery.cc enables it as needed.
 - Some comment changes which hadn't propagated from recover.c.

Otherwise, minor cleanup changes.  SQLite's tests compile and run under Ububntu
14.04.01 and OSX 10.9 with Xcode 5.0, with most tests passing.  See
README.Chromium for more information.

BUG=340757
Committed: https://crrev.com/42cee4198cb450ceff63b2442f55dd606a17faa5
Cr-Commit-Position: refs/heads/master@{#313849}

[Scott Hess - ex-Googler, 2015-01-27 22:09:36]

shess@chromium.org changed reviewers:
+ michaeln@chromium.org

[Scott Hess - ex-Googler, 2015-01-27 22:09:37]

OK, I found the WebDatabase problem with 3.8.7, so pulling the trigger.  This
just sets things up for a sane import.  I believe the individual patch files
under patches/ appropriately represent history.  AFAICT this approach _could_
maintain attribution as patches are applied and re-generated, but I'm not sure
how valuable that would be.

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/Makef...
File third_party/sqlite/src/Makefile.linux-gcc (right):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/Makef...
third_party/sqlite/src/Makefile.linux-gcc:107: TCC = gcc -Os
On OSX gcc is a clang wrapper and -O6 no longer works.  IMHO not
necessary/appropriate anyhow.

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/src/f...
File third_party/sqlite/src/src/func.c (left):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/src/f...
third_party/sqlite/src/src/func.c:1117: sqlite3_free((void*)azChar);
This originally came from Gears, and AFAICT it's not necessary at all.

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/src/m...
File third_party/sqlite/src/src/main.c (right):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/src/m...
third_party/sqlite/src/src/main.c:2005: #endif
This is here for testing, production enables it as-needed in sql/recovery.cc.

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/src/s...
File third_party/sqlite/src/src/shell.c (right):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/src/src/s...
third_party/sqlite/src/src/shell.c:2667: #if !defined(__APPLE__)
OSX is different, so rather than try to make it work for the sqlite_shell target
in sqlite.gyp _and_ sqlite3 in SQLite's build system, I just punted.

[Scott Hess - ex-Googler, 2015-01-27 22:49:55]

On 2015/01/27 22:09:37, Scott Hess wrote:
> AFAICT this approach _could_
> maintain attribution as patches are applied and re-generated, but I'm not sure
> how valuable that would be.

With a little experimentation, it looks like it will easily support attribution.
 What I mean by this is that the author and date will be correct for the CLs,
represented in the From: and Date: line of the faux emails.  It would be
"merely" a matter of annotating the patches a bit, and making up something
suitable for patches which slipstream other patches.

[michaeln, 2015-01-29 02:40:09]

lgtm

> With a little experimentation, it looks like it will easily support
attribution.
>  What I mean by this is that the author and date will be correct for the CLs,
> represented in the From: and Date: line of the faux emails.  It would be
> "merely" a matter of annotating the patches a bit, and making up something
> suitable for patches which slipstream other patches.

How much more complicated would the process be? Looks like git format-patch +
git am retain the CL desc. Could you bake the orig author and date into that
field?

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/README.ch...
File third_party/sqlite/README.chromium (right):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/README.ch...
third_party/sqlite/README.chromium:19: hopefully upstreamed), some are fixed
we've backported from a later version of
fixed s/b fixes

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/patches/0...
File
third_party/sqlite/patches/0008-Virtual-table-supporting-recovery-of-corrupted-datab.patch
(right):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/patches/0...
third_party/sqlite/patches/0008-Virtual-table-supporting-recovery-of-corrupted-datab.patch:758:
+  unsigned nMaxChildren;
whew

[Scott Hess - ex-Googler, 2015-01-29 22:08:33]

On 2015/01/29 02:40:09, michaeln wrote:
> lgtm
> 
> > With a little experimentation, it looks like it will easily support
> attribution.
> >  What I mean by this is that the author and date will be correct for the
CLs,
> > represented in the From: and Date: line of the faux emails.  It would be
> > "merely" a matter of annotating the patches a bit, and making up something
> > suitable for patches which slipstream other patches.
> 
> How much more complicated would the process be? Looks like git format-patch +
> git am retain the CL desc. Could you bake the orig author and date into that
> field?

OK, I went through and replicated author/date info where it seemed clear, and
added a few more original-url references, then ran through the patch-generation
process to make things canonical.  It's likely that the timestamps are off due
to timezone issues, but that seems unlikely to matter for ancient patches.  The
hashes all changed, of course, which may be an annoying aspect of this approach.
 AFAICT, there's no way to suppress the From line, though I _think_ it won't
ever be useful in this case (actually, it might be actively misleading to git-am
since the point where the patch is generated is unlikely to be in the upstream
branch at all, though I don't fully understand the use, yet).

I merged together the patches to get fts2 compiling for testing since they
overlapped, which is not rename-detected.  I didn't change the patch diffs at
all, only the header area.

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/README.ch...
File third_party/sqlite/README.chromium (right):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/README.ch...
third_party/sqlite/README.chromium:19: hopefully upstreamed), some are fixed
we've backported from a later version of
On 2015/01/29 02:40:09, michaeln wrote:
> fixed s/b fixes

Done.

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/patches/0...
File
third_party/sqlite/patches/0008-Virtual-table-supporting-recovery-of-corrupted-datab.patch
(right):

https://codereview.chromium.org/885473002/diff/1/third_party/sqlite/patches/0...
third_party/sqlite/patches/0008-Virtual-table-supporting-recovery-of-corrupted-datab.patch:758:
+  unsigned nMaxChildren;
On 2015/01/29 02:40:09, michaeln wrote:
> whew

Yeah, this set of patches had gotten outdated and mixed in with the
amalgamation, I think.

[michaeln, 2015-01-29 23:12:36]

This new process and the readme looks great.

Do you think it will ever make more sense to produce a hermetic VFS for websql
instead of sticking with a monkey-patched VFS? Are there any other patches
besides the 0006-Modify-default-VFS-to-support-WebDatabase.patch that are needed
for that VFS?

I guess that's a question that needs to get asked each time its time to do a new
import.

[Scott Hess - ex-Googler, 2015-01-29 23:24:19]

On 2015/01/29 23:12:36, michaeln wrote:
> This new process and the readme looks great.
> 
> Do you think it will ever make more sense to produce a hermetic VFS for websql
> instead of sticking with a monkey-patched VFS? Are there any other patches
> besides the 0006-Modify-default-VFS-to-support-WebDatabase.patch that are
needed
> for that VFS?
> 
> I guess that's a question that needs to get asked each time its time to do a
new
> import.

I'm experimenting with whether a vfs based on base/ APIs will work.  Dumi was
originally concerned about subtleties that we might get wrong, but I'm not
really seeing it, yet.

[Scott Hess - ex-Googler, 2015-01-29 23:25:27]

On 2015/01/29 23:24:19, Scott Hess wrote:
> On 2015/01/29 23:12:36, michaeln wrote:
> > This new process and the readme looks great.
> > 
> > Do you think it will ever make more sense to produce a hermetic VFS for
websql
> > instead of sticking with a monkey-patched VFS? Are there any other patches
> > besides the 0006-Modify-default-VFS-to-support-WebDatabase.patch that are
> needed
> > for that VFS?
> > 
> > I guess that's a question that needs to get asked each time its time to do a
> new
> > import.
> 
> I'm experimenting with whether a vfs based on base/ APIs will work.  Dumi was
> originally concerned about subtleties that we might get wrong, but I'm not
> really seeing it, yet.

Hmm, and my goal is to get rid of the [backport] patches, the [fts2] patches,
and the vfs and webdb patches.

[Scott Hess - ex-Googler, 2015-01-29 23:28:33]

The CQ bit was checked by shess@chromium.org

[commit-bot: I haz the power, 2015-01-29 23:28:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/885473002/40001

[michaeln, 2015-01-29 23:36:22]

> > I'm experimenting with whether a vfs based on base/ APIs will work.  Dumi
was
> > originally concerned about subtleties that we might get wrong, but I'm not
> > really seeing it, yet.

I remember seeing things like retry certain ops after a brief sleep on windows
to for obscure reasons. I got the feeling that there was some secret sauce in
there about how sqlite interacted with the filesystem.

[Scott Hess - ex-Googler, 2015-01-29 23:45:31]

On 2015/01/29 23:36:22, michaeln wrote:
> > > I'm experimenting with whether a vfs based on base/ APIs will work.  Dumi
> was
> > > originally concerned about subtleties that we might get wrong, but I'm not
> > > really seeing it, yet.
> 
> I remember seeing things like retry certain ops after a brief sleep on windows
> to for obscure reasons. I got the feeling that there was some secret sauce in
> there about how sqlite interacted with the filesystem.

SQLite is compatible with many more platforms than we are.  Also, I think this
case would be greatly constrained versus what SQLite needs to deal with in the
wild.  For instance, we could make an exclusive lock in the browser process
(perhaps even using an actual SQLite vfs to manage it), and then proxy
finer-grained locking within that via IPC.

Beyond all that, though I suspect there's a big difference between building the
technical underpinnings for a growing feature versus refactoring a declining
feature.  We have no guarantees that the current system actually is going to
work in the wild after a SQLite import, because nobody is looking at those
subtle changes to see if any of them affects our monkey-patching.

I think it won't take too long to make a prototype to see the shape of things,
we can circle back at that time.  May even be possible to run things in some
sort of side-by-side validation mode, still thinking on that.

[commit-bot: I haz the power, 2015-01-30 01:14:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-30 01:15:53]

Patchset 3 (id:??) landed as
https://crrev.com/42cee4198cb450ceff63b2442f55dd606a17faa5
Cr-Commit-Position: refs/heads/master@{#313849}