Fix SQLite#812
Closed
KevinBrolly wants to merge 2 commits intoheroku:masterfrom
KevinBrolly:fix-sqlite
Closed
Conversation
…SQLite3 steps from runtime build scripts
Member
Ah great idea - doesn't require the headers to be in the stack image build variant (avoiding the concerns in #427 (comment)), without having to resort to a fragile DIY apt approach :-) |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context:
Ed Morely first brought up issues with how SQLite3 was being vendored into the buildpack in the following issue - #427
The buildpack was using a very old (v3.7.9, released on 2011-11-01) version of SQLite3 which had some vulnerabilities - https://www.cvedetails.com/vendor/9237/Sqlite.html
The vendored sqlite archive included three components,
libsqlite3-0,libsqlite3-dev, and the sqlite3 CLI binary.Python itself only has a run-time dependency on
libsqlite3-0and a build-time dependency onlibsqlite3-dev, the sqlite3 CLI binary is not needed for any python-specific functionality.@hone implemented a solution (#713) which installs the SQLite headers when building runtimes, and installs the headers and binary as a step in the build process while using
libsqlite3-0that is provided by the stack. This solution works but is quite complicated, and recently cause an issue withcedar-14builds - c361c2fWe could achieve a similar outcome by installing
libsqlite3-devin the docker container used for runtime builds, making it available as required while python is being built, then we can rely on the stack providedlibsqlite3-0during runtime, removing a step during runtime builds and app builds, as well as a whole lot of code.The only downside of this approach is the loss of the sqlite CLI binary, but I cannot think why this would be a necessary inclusion, and could be installed if required by individual applications using something like Heroku Buildpack apt - https://github.com/heroku/heroku-buildpack-apt
I made this a draft PR currently as a few more steps would be required to make it work. There is currently no Dockerfile for
cedar-14and it appears runtime builds are currently being done in-dyno, which would make this solution not work forcedar-14, however runtime builds could be moved to Docker forcedar-14or we could wait until thecedar-14deprecation next month.Additionally, the python runtime is currently being cached as part of the build step, so we would need to invalidate the cache to allow users to get the new runtimes once built.