Commit ed7422bd authored by Victor Costan's avatar Victor Costan Committed by Commit Bot

sqlite: Updated TODO owners and version numbers.

Additionally minor cleanup of upgrade steps in README.chromium.

This is a follow-up addressing some feedback received in the reviews for
the attached bug.

Bug: 807093
Change-Id: Idd5e4fb196aeac6c101c1773ac6a0c364bb73ec3
Reviewed-on: https://chromium-review.googlesource.com/907868Reviewed-by: default avatarChris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535900}
parent 1f9ee1ad
...@@ -29,10 +29,10 @@ config("chromium_sqlite3_compile_options") { ...@@ -29,10 +29,10 @@ config("chromium_sqlite3_compile_options") {
"SQLITE_SECURE_DELETE", "SQLITE_SECURE_DELETE",
# Custom flag to tweak pcache pools. # Custom flag to tweak pcache pools.
# TODO(shess): This shouldn't use faux-SQLite naming. # TODO(pwnall): This shouldn't use faux-SQLite naming.
"SQLITE_SEPARATE_CACHE_POOLS", "SQLITE_SEPARATE_CACHE_POOLS",
# TODO(shess): SQLite adds mutexes to protect structures which cross # TODO(pwnall): SQLite adds mutexes to protect structures which cross
# threads. In theory Chromium should be able to turn this to "2" which # threads. In theory Chromium should be able to turn this to "2" which
# should give a slight speed boost. "2" is safe as long as a single # should give a slight speed boost. "2" is safe as long as a single
# connection is not used by more than one thread at a time. # connection is not used by more than one thread at a time.
...@@ -46,13 +46,13 @@ config("chromium_sqlite3_compile_options") { ...@@ -46,13 +46,13 @@ config("chromium_sqlite3_compile_options") {
# Allow 256MB mmap footprint per connection. Should not be too open-ended # Allow 256MB mmap footprint per connection. Should not be too open-ended
# as that could cause memory fragmentation. 50MB encompasses the 99th # as that could cause memory fragmentation. 50MB encompasses the 99th
# percentile of Chrome databases in the wild. # percentile of Chrome databases in the wild.
# TODO(shess): A 64-bit-specific value could be 1G or more. # TODO(pwnall): A 64-bit-specific value could be 1G or more.
# TODO(shess): Figure out if exceeding this is costly. # TODO(pwnall): Figure out if exceeding this is costly.
"SQLITE_MAX_MMAP_SIZE=268435456", "SQLITE_MAX_MMAP_SIZE=268435456",
# Use a read-only memory map when mmap'ed I/O is enabled to prevent memory # Use a read-only memory map when mmap'ed I/O is enabled to prevent memory
# stompers from directly corrupting the database. # stompers from directly corrupting the database.
# TODO(shess): Upstream the ability to use this define. # TODO(pwnall): Upstream the ability to use this define.
"SQLITE_MMAP_READ_ONLY=1", "SQLITE_MMAP_READ_ONLY=1",
# By default SQLite pre-allocates 100 pages of pcache data, which will not # By default SQLite pre-allocates 100 pages of pcache data, which will not
...@@ -60,12 +60,12 @@ config("chromium_sqlite3_compile_options") { ...@@ -60,12 +60,12 @@ config("chromium_sqlite3_compile_options") {
# memory-usage goals. # memory-usage goals.
"SQLITE_DEFAULT_PCACHE_INITSZ=0", "SQLITE_DEFAULT_PCACHE_INITSZ=0",
# NOTE(shess): Some defines can affect the amalgamation. Those should be # Some defines can affect the amalgamation. Those should be added to
# added to google_generate_amalgamation.sh, and the amalgamation # google_generate_amalgamation.sh, and the amalgamation re-generated.
# re-generated. Usually this involves disabling features which include # Usually this involves disabling features which include keywords or
# keywords or syntax, for instance SQLITE_OMIT_VIRTUALTABLE omits the # syntax, for instance SQLITE_OMIT_VIRTUALTABLE omits the virtual table
# virtual table syntax entirely. Missing an item usually results in # syntax entirely. Missing an item usually results in syntax working but
# syntax working but execution failing. Review: # execution failing. Review:
# src/src/parse.py # src/src/parse.py
# src/tool/mkkeywordhash.c # src/tool/mkkeywordhash.c
...@@ -320,7 +320,8 @@ fuzzer_test("sqlite3_prepare_v2_fuzzer") { ...@@ -320,7 +320,8 @@ fuzzer_test("sqlite3_prepare_v2_fuzzer") {
} }
fuzzer_test("sqlite3_ossfuzz_fuzzer") { fuzzer_test("sqlite3_ossfuzz_fuzzer") {
# TODO(mmoroz, shess): remove fuzz/ossfuzz.c after next sqlite3 update. # TODO(mmoroz, pwnall): remove fuzz/ossfuzz.c in favor of test/osfuzz.c in
# sqlite's source code
sources = [ sources = [
"fuzz/ossfuzz.c", "fuzz/ossfuzz.c",
] ]
......
Name: sqlite Name: sqlite
URL: https://sqlite.org/ URL: https://sqlite.org/
Version: 3.21.0 Version: 3.22.0
Included In Release: Yes Included In Release: Yes
Security Critical: Yes Security Critical: Yes
License: Public domain License: Public domain
...@@ -110,9 +110,9 @@ git commit -m './scripts/generate_amalgamation.sh' amalgamation/ ...@@ -110,9 +110,9 @@ git commit -m './scripts/generate_amalgamation.sh' amalgamation/
# will have hash changes. A sensible check-in comment would be something like # will have hash changes. A sensible check-in comment would be something like
# the patch's checkin comment, plus "regenerate amalgamation and generate patch # the patch's checkin comment, plus "regenerate amalgamation and generate patch
# file." # file."
# TODO(shess): Should hash changes be checked in, or backed out? # TODO(pwnall): Should hash changes be checked in, or backed out?
# Find a sucker. Send review. # Find unsuspecting victim and send review.
--- ---
...@@ -132,31 +132,31 @@ others. The basic idea below is to generate git branches to work with: ...@@ -132,31 +132,31 @@ others. The basic idea below is to generate git branches to work with:
We will upload sqlite-new-upstream as a massive (800k LOC+) CL that cannot We will upload sqlite-new-upstream as a massive (800k LOC+) CL that cannot
possibly be reviewed, but is generated in an automated fashion. We will then possibly be reviewed, but is generated in an automated fashion. We will then
squash sqlite-new to sqlite-new-upstream and obtain one CL that only contains squash sqlite-new to sqlite-new-upstream and obtain one CL that only contains
diffs. The second CL is still large, but it's a fraction of the fits (automated) diffs. The second CL is still large, but it's a fraction of the first
CL. (automated) CL.
# The steps below are easier if done in the SQLite directory. # The steps below are easier if done in the SQLite directory.
cd third_party/sqlite cd third_party/sqlite
export OLD=3170000 export OLD=3220000
export NEW=3190300 export NEW=3230000
export GNU_SED=sed # OSX: "brew install gnu-sed", then use "gsed" here. export GNU_SED=sed # OSX: "brew install gnu-sed", then use "gsed" here.
#### Download and unpack the new SQLite release. #### Download and unpack the new SQLite release.
git new-branch sqlite-new-upstream git new-branch sqlite-new-upstream
# URL from "Alternative Source Code Formats" at https://sqlite.org/download.html # URL from "Alternative Source Code Formats" at https://sqlite.org/download.html
curl https://sqlite.org/2017/sqlite-src-3190300.zip > upstream.zip curl https://sqlite.org/2018/sqlite-src-3230000.zip > upstream.zip
mkdir sqlite-src-$NEW mkdir sqlite-src-$NEW
unzip ./upstream.zip -d sqlite-src-$NEW unzip ./upstream.zip -d sqlite-src-$NEW
rm ./upstream.zip rm ./upstream.zip
mv sqlite-src-$NEW/SQLite-*/* sqlite-src-$NEW/ mv sqlite-src-$NEW/sqlite-*/* sqlite-src-$NEW/
rmdir mv sqlite-src-$NEW/SQLite-*/ rmdir sqlite-src-$NEW/sqlite-*/
xdg-open sqlite-src-$NEW # Make sure everything looks right. xdg-open sqlite-src-$NEW # Make sure everything looks right.
#### Add the new release code in a separate CL, for code review sanity. #### Add the new release code in a separate CL, for code review sanity.
git add sqlite-src-$NEW # Committing the code as downloaded, on purpose. git add sqlite-src-$NEW # Committing the code as downloaded, on purpose.
git clean -i -d -x sqlite-src-$NEW # Make sure no file is git-ignored. git clean -i -d -x sqlite-src-$NEW # Make sure no file is git-ignored.
git commit -m "sqlite: Add code for release 3.19" git commit -m "sqlite: Add code for release 3.23.0"
git cl upload # Have the new code in a separate (impossible to review) CL. git cl upload # Have the new code in a separate (impossible to review) CL.
#### Create a branch for the old SQLite release's upstream version. #### Create a branch for the old SQLite release's upstream version.
...@@ -234,7 +234,7 @@ git cl upload --squash ...@@ -234,7 +234,7 @@ git cl upload --squash
# <https://www.sqlite.org/changes.html> which might be deemed important. Don't # <https://www.sqlite.org/changes.html> which might be deemed important. Don't
# enumerate all of the patch messages, those are assumed, but do reference any # enumerate all of the patch messages, those are assumed, but do reference any
# material changes made. # material changes made.
# TODO(shess) Describe an appropriate comment style. Seems like it should at # TODO(pwnall) Describe an appropriate comment style. Seems like it should at
# least include the SQLite version number. Meanwhile, look in the logs for past # least include the SQLite version number. Meanwhile, look in the logs for past
# commits to model things on. # commits to model things on.
...@@ -251,6 +251,8 @@ distinct CLs. ...@@ -251,6 +251,8 @@ distinct CLs.
4) Running SQLite's test suite within the Chromium checkout. 4) Running SQLite's test suite within the Chromium checkout.
TODO(pwnall): This hasn't been tried out for at least a year.
Prerequisites: The test suite requires tcl-dev and libicu-dev. Install those on Prerequisites: The test suite requires tcl-dev and libicu-dev. Install those on
Ubuntu like: Ubuntu like:
sudo apt-get install tcl8.6-dev libicu-dev sudo apt-get install tcl8.6-dev libicu-dev
...@@ -288,7 +290,7 @@ for the same reason as for OSX. ...@@ -288,7 +290,7 @@ for the same reason as for OSX.
-- --
NOTE(shess): On Ubuntu it is possible to run the tests in a tmpfs something NOTE(pwnall): On Ubuntu it is possible to run the tests in a tmpfs something
like: like:
TMPFS=/dev/shm/sqlite_build TMPFS=/dev/shm/sqlite_build
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment