Update SQLite3 by hone · Pull Request #713 · heroku/heroku-buildpack-python

hone · 2018-06-21T02:03:43Z

With inspiration from @KevinBrolly, this patch uses the stack image SQLite3 package but also still providing the dev headers and binary that users may still be using today. We can just stop shipping SQLite3 from future binaries. In addition, we don't need to worry about what version and when to update SQLite3 and maintaining the packages ourselves.

I limited the patches to Python 2.7.15 and 3.6.6 with this change, so we wouldn't have to rebuild all the binaries. We'll need re-release the binaries in conjunction with this buildpack change. The binary building and buildpack share the same sqlite3 installation code except the binary building only installs the dev headers for python to build with the stack image.

edmorley

Thank you for working on this! This seemed like the best approach previously (#427 (comment)), I just had not had time to look into it.

edmorley · 2018-06-21T10:36:34Z

I think .heroku/python-sqlite3-version needs to be added to the list of files deleted here, since otherwise if the stack changes, the sqlite binary and headers end up being deleted, but yet the bin/steps/sqlite3 won't reinstall them, since .heroku/python-sqlite3-version will still exist.

Alternatively, perhaps the sqlite3 version file could be moved into .heroku/python/, since they are quite strongly tied together?

Good catch. I kept it out to keep it in line with .heroku/python-version.

edmorley · 2018-06-21T10:40:05Z

This steps file is sourced, so the set +e here will affect subsequent steps too. This may be fine, but mentioning it just in case.

edmorley · 2018-06-21T10:41:58Z

The Python directory is referenced manually a few times above - perhaps it would be worth moving HEROKU_PYTHON_DIR up higher and using for those instances too?

edmorley · 2018-06-21T10:44:18Z

I wonder if at some point in the future it would be worth adding some APT helper functions to buildpack-stdlib to reduce the duplication between the various buildpacks that have to play the non-system APT install dance?

edmorley · 2018-06-21T10:57:55Z

Is it worth mentioning here or in bin/steps/sqlite3 that:

libsqlite3-0 isn't being installed here since it already exists in the stack images

the reason the headers aren't installed in the stack image is Issues with the vendored sqlite/libsqlite #427 (comment)

sigmavirus24

It seems like the approach here is to build python with the same version but reset the dev headers and binary for other applications. Is that right?

sigmavirus24 · 2018-06-21T12:17:44Z

It seems we're duplicating this and we may be missing files, should we have an array of files we want to delete or a single clear-cache function to use?

schneems

From a compliance standpoint, this seems good.

My bash-fu might not be a strong as the other reviewers. I think the biggest issues to look at are cache related.

The travis tests are currently failing:

ASSERT:Expected </tmp/shunit.hUheiq/tmp/output.WHDe/stdout> to contain <Uninstalling requests>
-----> Need to update SQLite3, clearing cache
-----> Installing python-3.6.5
-----> Installing pip
-----> Installing requirements with pip
       Collecting psycopg2 (from -r /tmp/testvM0L0/requirements.txt (line 1))
         Downloading https://files.pythonhosted.org/packages/5e/d0/9e2b3ed43001ebed45caf56d5bb9d44ed3ebd68e12b87845bfa7bcd46250/psycopg2-2.7.5-cp36-cp36m-manylinux1_x86_64.whl (2.7MB)
       Installing collected packages: psycopg2
       Successfully installed psycopg2-2.7.5
testStackChange
Ran 18 tests.
FAILED (failures=1)
make: *** [test-heroku-16] Error 1

Are there existing tests that cover an app using sqlite3? Do you have a test app that I can manually try deploying against this branch?

schneems · 2018-06-21T14:20:31Z

Is it possible that one of the headers will have a space? Should we quote "$header" ?

schneems · 2018-06-21T14:21:01Z

Same question here. Should this be quoted?

KevinBrolly

I tried this buildpack out with the heroku-18 stack and the python getting started app and it does not appear to be working out of the box, the original gpg issue is still present:

$ gpg --version
gpg: symbol lookup error: gpg: undefined symbol: sqlite3_errstr

Also the sqlite3 binary still seems to be the old version:

~/.heroku/python/bin $ sqlite3 --version
3.7.9 2011-11-01 00:52:41 c7c6050ef060877ebe77b41d959e9df13f8c9b5e

KevinBrolly · 2018-06-21T15:12:44Z

I am not sure that this will ever be true, the cache is blank, so $INSTALLED_SQLITE3_VERSION is null, apt-cache policy libsqlite3-dev | grep Candidate | sed 's/ Candidate: //' will only return if apt-get update is run first, so I am not sure if this will work in the current context of the buildpack? If it is the case that apt-cache policy libsqlite3-dev | grep Candidate | sed 's/ Candidate: //' returns null then this condition will not be true unless the $BUILD_DIR/.heroku/python-sqlite3-version file exists, which it will not because it is only created at the end of this conditional block. Can anyone confirm this?

sigmavirus24 · 2018-07-16T12:24:49Z

Should this be removed like in the 3.6.5 build recipe?

yes, but not in 3.6.5. fixing.

sigmavirus24 · 2018-07-16T12:27:25Z

Interesting that this step is used both in the recipes and in the compile step. Is it because the stack image doesn't install sqlite3 by default so we need to ensure it's installed then but also need the right version installed when compiling?

We need to use it in both since the dev headers are missing on the stack image, so it won't be around for build or runtime. We need it at build time, so the python sqlite library is built against the version of sqlite3 on the stack image.

KevinBrolly

Did you only change the runtime files for 2.7.15 and 3.6.6 because they are the currently supported versions? If so you will also need to do 3.7.0 as it is also currently a supported version - https://devcenter.heroku.com/articles/python-support#supported-runtimes

KevinBrolly · 2018-07-16T12:45:31Z

You said Python 3.6.5+ here but the runtime build script for 3.6.5 has not been updated to install Sqlite3

Good catch! Thanks.

hone · 2018-07-16T19:59:08Z

@KevinBrolly you're right, I'll update the PR.

@KevinBrolly

With inspiration from @KevinBrolly, this patch uses the stack image SQLite3 package but also still providing the dev headers and binary that users may still be using today. The benefit is that we won't need to rebuild all the python binaries for this to take affect. We can just stop shipping SQLite3 from future binaries. In addition, we don't need to worry about what version and when to update SQLite3 and maintaining the packages ourselves. This also includes updates to Python 2.7.15 and Python 3.6.6 so they can rebuilt with the stack image dev headers instead of building our own vendored SQLite3.

hone force-pushed the sqlite3 branch 2 times, most recently from 94367f2 to 60b863c Compare June 21, 2018 04:59

edmorley reviewed Jun 21, 2018

View reviewed changes

sigmavirus24 reviewed Jun 21, 2018

View reviewed changes

schneems approved these changes Jun 21, 2018

View reviewed changes

KevinBrolly reviewed Jun 21, 2018

View reviewed changes

hone force-pushed the sqlite3 branch 6 times, most recently from 9023b38 to dd9a325 Compare June 22, 2018 21:53

hone force-pushed the sqlite3 branch 18 times, most recently from 2180c22 to 5fc9fa8 Compare July 12, 2018 23:43

hone force-pushed the sqlite3 branch 3 times, most recently from 5141e26 to 36e9145 Compare July 13, 2018 20:10

hone mentioned this pull request Jul 13, 2018

SQLite3 Upgrade #716

Closed

sigmavirus24 reviewed Jul 16, 2018

View reviewed changes

KevinBrolly reviewed Jul 16, 2018

View reviewed changes

hone force-pushed the sqlite3 branch from 36e9145 to 07b78f2 Compare July 16, 2018 21:03

sigmavirus24 approved these changes Jul 17, 2018

View reviewed changes

sigmavirus24 assigned hone Jul 17, 2018

add Python version operators

e82f1e4

hone force-pushed the sqlite3 branch 2 times, most recently from 84f7969 to 2e8409f Compare July 26, 2018 21:48

hone force-pushed the sqlite3 branch from 2e8409f to 221722f Compare July 26, 2018 21:49

hone merged commit 0fe4f91 into heroku:master Jul 26, 2018

hone deleted the sqlite3 branch July 26, 2018 22:21

CaseyFaist mentioned this pull request Aug 9, 2018

heroku-18 GPG errors #712

Closed

KevinBrolly mentioned this pull request Mar 15, 2019

Fix SQLite #812

Closed

luyizhao mentioned this pull request Dec 11, 2019

Cache unnecessarily being cleared due to lack of cached SQLite3 version file #909

Closed

Conversation

hone commented Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edmorley left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sigmavirus24 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schneems left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KevinBrolly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KevinBrolly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hone commented Jul 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hone commented Jun 21, 2018 •

edited

Loading

edmorley left a comment •

edited

Loading