Skip to content

Fix sqlite#858

Closed
CaseyFaist wants to merge 10 commits intomasterfrom
fix-sqlite
Closed

Fix sqlite#858
CaseyFaist wants to merge 10 commits intomasterfrom
fix-sqlite

Conversation

@CaseyFaist
Copy link
Copy Markdown
Contributor

@CaseyFaist CaseyFaist commented Sep 27, 2019

Rebased from #812 for further work + upstreaming - see other pr for full context on approach

@CaseyFaist CaseyFaist requested a review from a team as a code owner September 27, 2019 21:59
Comment thread builds/cedar-14.Dockerfile Outdated
Comment thread builds/heroku-16.Dockerfile Outdated
@dzuelke
Copy link
Copy Markdown
Contributor

dzuelke commented Sep 27, 2019

So right now this won't allow pip installs of packages that need the headers.

To achieve that, we need to, roughly, do this:

  1. apt-get install libsqlite3-dev in the Python formula (see https://github.com/heroku/heroku-buildpack-php/blob/343fa959d4c195beb7febe1996340624baf713b9/support/build/php#L59-L71 for a generic way of doing this)
  2. ensure realpath is also apt-get installed on cedar-14 in the Python formula (see previous point's link for the pattern)
  3. mkdir -p ${OUT_PREFIX}/include ${OUT_PREFIX}/lib/x86_64-linux-gnu
  4. cp /usr/include/sqlite3*.h ${OUT_PREFIX}/include
  5. ln -fs $(realpath /usr/lib/x86_64-linux-gnu/libsqlite3.so) ${OUT_PREFIX}/lib/x86_64-linux-gnu/libsqlite3.so

This will ensure that the libsqlite3 headers, as well as a symlink for the generic .so library (which gets resolved when compiled against) are included in the built Python tarball

Then, in bin/compile, just before pip install, we need to point the compiler infrastructure at those additional locations:

  1. export CFLAGS="-I/app/.heroku/python/include ${CFLAGS:-}"
  2. export LDFLAGS="-L/app/.heroku/lib/x86_64-linux-gnu ${LDFLAGS:-}"

This should then allow building e.g. a pysqlite dependency in requirements.txt.

@CaseyFaist CaseyFaist mentioned this pull request Sep 30, 2019
@CaseyFaist
Copy link
Copy Markdown
Contributor Author

CaseyFaist commented Sep 30, 2019

Capturing for future reference:

The reason for the shift in approach is that the apt-buildpack won't load headers properly given the package is still on the platform and ONLY the sqlite headers are not, and thus would not allow users of pysqlite or other utilities to use sqlite on runtime. We're opting to preserve this functionality instead, with added complexity as the tradeoff 🎉

@CaseyFaist CaseyFaist changed the title [WIP] Fix sqlite Fix sqlite Sep 30, 2019
@CaseyFaist
Copy link
Copy Markdown
Contributor Author

Also important: the c and ld flags will not be updated in this pr, but a future one when this functionality is flipped on. That's a potentially breaking change, so slicing this up there unblocks the rest of this work without causing breakage.

@CaseyFaist CaseyFaist closed this Mar 18, 2020
@edmorley edmorley deleted the fix-sqlite branch July 30, 2020 10:03
cybniv added a commit to cybniv/heroku-buildpack-python that referenced this pull request Sep 15, 2020
dm.xmlsec.binding, which comes as a dep for python-saml,
does not compile on Heroku, using this buildpack, because
apparently /usr/include is not available at that time.

After some research, it seems that is known behavior and the reason
why workarounds for some packages (e.g.sqlite [1]) exist.

Hence it may be easiest to simply do something very similiar for
libxmlsec1, hopefully without breaking anything else.

[1] heroku#858
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants