search

TryGhost / node-sqlite3

SQLite3 bindings for Node.js
BSD 3-Clause "New" or "Revised" License
6.22k stars 815 forks source link

node-sqlite3 does not clean up after itself after installation #1182

Open mgrandi opened 5 years ago

mgrandi commented 5 years ago

It seems that after building using the source code, it just leaves the intermediate build files everywhere including multiple copies of the compiled sqlite3 plugins, and this is causing issues with downstream consumers of this library where electron-builder is including all of the files and it bloats application size by like 20 MB

it seems the only file you actually need to keep is the stuff inside sqlite3/lib/

[2019-06-28 14:59:46] markgrandi@Gypaetus:~/Temp/node_test$ npm install --build-from-source sqlite3

> sqlite3@4.0.9 install /Users/markgrandi/Temp/node_test/node_modules/sqlite3
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using request for node-pre-gyp https download
  ACTION deps_sqlite3_gyp_action_before_build_target_unpack_sqlite_dep Release/obj/gen/sqlite-autoconf-3280000/sqlite3.c
  TOUCH Release/obj.target/deps/action_before_build.stamp
  CC(target) Release/obj.target/sqlite3/gen/sqlite-autoconf-3280000/sqlite3.o

<build warnings and such>

23 warnings generated.
  SOLINK_MODULE(target) Release/node_sqlite3.node
  COPY /Users/markgrandi/Temp/node_test/node_modules/sqlite3/lib/binding/node-v72-darwin-x64/node_sqlite3.node
  TOUCH Release/obj.target/action_after_build.stamp
npm WARN saveError ENOENT: no such file or directory, open '/Users/markgrandi/Temp/node_test/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN enoent ENOENT: no such file or directory, open '/Users/markgrandi/Temp/node_test/package.json'
npm WARN node_test No description
npm WARN node_test No repository field.
npm WARN node_test No README data
npm WARN node_test No license field.

+ sqlite3@4.0.9
added 113 packages from 102 contributors and audited 160 packages in 63.214s
found 0 vulnerabilities

[2019-06-28 15:01:29] markgrandi@Gypaetus:~/Temp/node_test$ cd node_modules/sqlite3/
[2019-06-28 15:11:03] markgrandi@Gypaetus:~/Temp/node_test/node_modules/sqlite3$ tree -h
.
├── [6.7K]  CHANGELOG.md
├── [2.7K]  CONTRIBUTING.md
├── [2.2K]  Dockerfile
├── [1.4K]  LICENSE
├── [9.7K]  README.md
├── [1.3K]  binding.gyp
├── [ 320]  build
│   ├── [ 14K]  Makefile
│   ├── [ 224]  Release
│   │   ├── [1.9M]  node_sqlite3.node
│   │   ├── [  96]  obj
│   │   │   └── [  96]  gen
│   │   │       └── [ 864]  sqlite-autoconf-3280000
│   │   │           ├── [ 15K]  INSTALL
│   │   │           ├── [ 717]  Makefile.am
│   │   │           ├── [ 547]  Makefile.fallback
│   │   │           ├── [ 37K]  Makefile.in
│   │   │           ├── [ 28K]  Makefile.msc
│   │   │           ├── [3.5K]  README.txt
│   │   │           ├── [7.1K]  Replace.cs
│   │   │           ├── [364K]  aclocal.m4
│   │   │           ├── [7.2K]  compile
│   │   │           ├── [ 43K]  config.guess
│   │   │           ├── [ 35K]  config.sub
│   │   │           ├── [466K]  configure
│   │   │           ├── [7.8K]  configure.ac
│   │   │           ├── [ 23K]  depcomp
│   │   │           ├── [ 15K]  install-sh
│   │   │           ├── [317K]  ltmain.sh
│   │   │           ├── [6.7K]  missing
│   │   │           ├── [516K]  shell.c
│   │   │           ├── [8.7K]  sqlite3.1
│   │   │           ├── [7.5M]  sqlite3.c
│   │   │           ├── [546K]  sqlite3.h
│   │   │           ├── [ 267]  sqlite3.pc.in
│   │   │           ├── [1.9K]  sqlite3.rc
│   │   │           ├── [ 33K]  sqlite3ext.h
│   │   │           └── [ 416]  tea
│   │   │               ├── [ 16K]  Makefile.in
│   │   │               ├── [1.3K]  README
│   │   │               ├── [ 147]  aclocal.m4
│   │   │               ├── [275K]  configure
│   │   │               ├── [8.1K]  configure.ac
│   │   │               ├── [  96]  doc
│   │   │               │   └── [ 494]  sqlite3.n
│   │   │               ├── [  96]  generic
│   │   │               │   └── [120K]  tclsqlite3.c
│   │   │               ├── [ 257]  license.terms
│   │   │               ├── [ 167]  pkgIndex.tcl.in
│   │   │               ├── [ 128]  tclconfig
│   │   │               │   ├── [ 14K]  install-sh
│   │   │               │   └── [131K]  tcl.m4
│   │   │               └── [ 160]  win
│   │   │                   ├── [ 14K]  makefile.vc
│   │   │                   ├── [ 17K]  nmakehlp.c
│   │   │                   └── [ 18K]  rules.vc
│   │   ├── [ 192]  obj.target
│   │   │   ├── [   0]  action_after_build.stamp
│   │   │   ├── [  96]  deps
│   │   │   │   └── [   0]  action_before_build.stamp
│   │   │   ├── [  96]  node_sqlite3
│   │   │   │   └── [ 192]  src
│   │   │   │       ├── [ 46K]  backup.o
│   │   │   │       ├── [ 64K]  database.o
│   │   │   │       ├── [ 13K]  node_sqlite3.o
│   │   │   │       └── [ 84K]  statement.o
│   │   │   └── [  96]  sqlite3
│   │   │       └── [  96]  gen
│   │   │           └── [  96]  sqlite-autoconf-3280000
│   │   │               └── [2.1M]  sqlite3.o
│   │   └── [2.1M]  sqlite3.a
│   ├── [1.3K]  action_after_build.target.mk
│   ├── [ 139]  binding.Makefile
│   ├── [5.7K]  config.gypi
│   ├── [ 160]  deps
│   │   ├── [2.6K]  action_before_build.target.mk
│   │   ├── [ 146]  sqlite3.Makefile
│   │   └── [5.3K]  sqlite3.target.mk
│   ├── [ 23K]  gyp-mac-tool
│   └── [5.8K]  node_sqlite3.target.mk
├── [  64]  cloudformation
├── [ 192]  deps
│   ├── [1.2K]  common-sqlite.gypi
│   ├── [ 191]  extract.py
│   ├── [2.7M]  sqlite-autoconf-3280000.tar.gz
│   └── [2.7K]  sqlite3.gyp
├── [ 192]  lib
│   ├── [  96]  binding
│   │   └── [  96]  node-v72-darwin-x64
│   │       └── [1.9M]  node_sqlite3.node
│   ├── [  39]  index.js
│   ├── [6.3K]  sqlite3.js
│   └── [1.4K]  trace.js
├── [3.2K]  package.json
├── [  43]  sqlite3.js
├── [ 416]  src
│   ├── [1.9K]  async.h
│   ├── [ 13K]  backup.cc
│   ├── [6.6K]  backup.h
│   ├── [ 19K]  database.cc
│   ├── [4.8K]  database.h
│   ├── [ 249]  gcc-preinclude.h
│   ├── [ 11K]  macros.h
│   ├── [4.7K]  node_sqlite3.cc
│   ├── [ 27K]  statement.cc
│   ├── [6.3K]  statement.h
│   └── [1.1K]  threading.h
└── [  96]  tools
    └── [  96]  docker
        └── [  96]  architecture
            └── [ 128]  linux-arm64
                ├── [2.5K]  Dockerfile
                └── [1.9K]  run.sh

28 directories, 85 files

[2019-06-28 15:13:20] markgrandi@Gypaetus:~/Temp/node_test/node_modules/sqlite3$ du -hs .
 22M    .

here is the output of an electron project that uses this library as a dependency (that has 4 copies of sqlite3.node)

[2019-06-28 15:21:17] markgrandi@Gypaetus:~/Temp/node_test/node_modules/sqlite3$ tree -h /Applications/Joplin.app/Contents/Resources/app/node_modules/sqlite3
/Applications/Joplin.app/Contents/Resources/app/node_modules/sqlite3
├── [2.7K]  CONTRIBUTING.md
├── [1.4K]  LICENSE
├── [  96]  bin
│   └── [  96]  darwin-x64-69
│       └── [1.9M]  sqlite3.node
├── [ 128]  build
│   ├── [ 192]  Release
│   │   ├── [1.9M]  node_sqlite3.node
│   │   ├── [  96]  obj
│   │   │   └── [  96]  gen
│   │   │       └── [ 864]  sqlite-autoconf-3260000
│   │   │           ├── [ 15K]  INSTALL
│   │   │           ├── [ 717]  Makefile.am
│   │   │           ├── [ 547]  Makefile.fallback
│   │   │           ├── [ 37K]  Makefile.in
│   │   │           ├── [ 28K]  Makefile.msc
│   │   │           ├── [3.5K]  README.txt
│   │   │           ├── [7.1K]  Replace.cs
│   │   │           ├── [364K]  aclocal.m4
│   │   │           ├── [7.2K]  compile
│   │   │           ├── [ 42K]  config.guess
│   │   │           ├── [ 35K]  config.sub
│   │   │           ├── [465K]  configure
│   │   │           ├── [7.8K]  configure.ac
│   │   │           ├── [ 23K]  depcomp
│   │   │           ├── [ 14K]  install-sh
│   │   │           ├── [316K]  ltmain.sh
│   │   │           ├── [6.7K]  missing
│   │   │           ├── [493K]  shell.c
│   │   │           ├── [8.7K]  sqlite3.1
│   │   │           ├── [7.4M]  sqlite3.c
│   │   │           ├── [543K]  sqlite3.h
│   │   │           ├── [ 267]  sqlite3.pc.in
│   │   │           ├── [1.9K]  sqlite3.rc
│   │   │           ├── [ 33K]  sqlite3ext.h
│   │   │           └── [ 416]  tea
│   │   │               ├── [ 16K]  Makefile.in
│   │   │               ├── [1.3K]  README
│   │   │               ├── [ 147]  aclocal.m4
│   │   │               ├── [274K]  configure
│   │   │               ├── [8.1K]  configure.ac
│   │   │               ├── [  96]  doc
│   │   │               │   └── [ 494]  sqlite3.n
│   │   │               ├── [  96]  generic
│   │   │               │   └── [115K]  tclsqlite3.c
│   │   │               ├── [ 257]  license.terms
│   │   │               ├── [ 167]  pkgIndex.tcl.in
│   │   │               ├── [ 128]  tclconfig
│   │   │               │   ├── [ 14K]  install-sh
│   │   │               │   └── [131K]  tcl.m4
│   │   │               └── [ 160]  win
│   │   │                   ├── [ 14K]  makefile.vc
│   │   │                   ├── [ 17K]  nmakehlp.c
│   │   │                   └── [ 18K]  rules.vc
│   │   └── [2.1M]  sqlite3.a
│   └── [ 160]  deps
│       ├── [2.7K]  action_before_build.target.mk
│       ├── [ 146]  sqlite3.Makefile
│       └── [5.1K]  sqlite3.target.mk
├── [ 192]  deps
│   ├── [1.2K]  common-sqlite.gypi
│   ├── [ 191]  extract.py
│   ├── [2.7M]  sqlite-autoconf-3260000.tar.gz
│   └── [2.6K]  sqlite3.gyp
├── [ 192]  lib
│   ├── [ 128]  binding
│   │   ├── [  96]  electron-v4.1-darwin-x64
│   │   │   └── [1.9M]  node_sqlite3.node
│   │   └── [  96]  node-v57-darwin-x64
│   │       └── [1.9M]  node_sqlite3.node
│   ├── [  39]  index.js
│   ├── [5.4K]  sqlite3.js
│   └── [1.4K]  trace.js
├── [ 914]  package.json
├── [  43]  sqlite3.js
└── [ 256]  src
    ├── [1.9K]  async.h
    ├── [4.8K]  database.h
    ├── [ 249]  gcc-preinclude.h
    ├── [8.7K]  macros.h
    ├── [6.3K]  statement.h
    └── [1.1K]  threading.h

19 directories, 63 files

[2019-06-28 15:21:26] markgrandi@Gypaetus:~/Temp/node_test/node_modules/sqlite3$ du -hs /Applications/Joplin.app/Contents/Resources/app/node_modules/sqlite3
 23M    /Applications/Joplin.app/Contents/Resources/app/node_modules/sqlite3
kewde commented 5 years ago

Hmm I'll think about this. Doing the 'cleanup' does not seem easy for some files. For example, in electron, node-v57-darwin-x64/node_sqlite3.node is never needed. But there is no way for node-sqlite3 to know what you're building for.

npm install --build-from-source sqlite3

Will build the node version, but electron-builder will detect native dependencies and rebuild them when packaging. (cfr. npmRebuild https://www.electron.build/configuration/configuration )

electron-builder configurations can be modified to only include particular files (as a workaround, just include the electron-/node_sqlite3.node and .js files).

fastman commented 5 years ago

I have the exact same issue but when building an app in the Yocto - sqlite3 adds around ~11 MB of unnecessary files (mainly that sqlite-autoconf-3280000 dir)

sqlite3/
├── [       1377]  binding.gyp
├── [       4096]  build
│   ├── [       1509]  action_after_build.target.mk
│   ├── [        139]  binding.Makefile
│   ├── [       5815]  config.gypi
│   ├── [       4096]  deps
│   │   ├── [       1887]  action_before_build.target.mk
│   │   ├── [        146]  sqlite3.Makefile
│   │   └── [       4506]  sqlite3.target.mk
│   ├── [      14173]  Makefile
│   ├── [       4668]  node_sqlite3.target.mk
│   └── [       4096]  Release
│       ├── [    1329364]  node_sqlite3.node
│       └── [       4096]  obj
│           └── [       4096]  gen
│               └── [       4096]  sqlite-autoconf-3280000
│                   ├── [     372955]  aclocal.m4
│                   ├── [       7333]  compile
│                   ├── [      44283]  config.guess
│                   ├── [      36136]  config.sub
│                   ├── [     477389]  configure
│                   ├── [       7974]  configure.ac
│                   ├── [      23567]  depcomp
│                   ├── [      15744]  INSTALL
│                   ├── [      15155]  install-sh
│                   ├── [     324412]  ltmain.sh
│                   ├── [        717]  Makefile.am
│                   ├── [        547]  Makefile.fallback
│                   ├── [      37445]  Makefile.in
│                   ├── [      28537]  Makefile.msc
│                   ├── [       6872]  missing
│                   ├── [       3558]  README.txt
│                   ├── [       7272]  Replace.cs
│                   ├── [     527983]  shell.c
│                   ├── [       8928]  sqlite3.1
│                   ├── [    7864301]  sqlite3.c
│                   ├── [      33981]  sqlite3ext.h
│                   ├── [     559572]  sqlite3.h
│                   ├── [        267]  sqlite3.pc.in
│                   ├── [       1992]  sqlite3.rc
│                   └── [       4096]  tea
│                       ├── [        147]  aclocal.m4
│                       ├── [     281277]  configure
│                       ├── [       8308]  configure.ac
│                       ├── [       4096]  doc
│                       │   └── [        494]  sqlite3.n
│                       ├── [       4096]  generic
│                       │   └── [     123249]  tclsqlite3.c
│                       ├── [        257]  license.terms
│                       ├── [      15902]  Makefile.in
│                       ├── [        167]  pkgIndex.tcl.in
│                       ├── [       1338]  README
│                       ├── [       4096]  tclconfig
│                       │   ├── [      13868]  install-sh
│                       │   └── [     134055]  tcl.m4
│                       └── [       4096]  win
│                           ├── [      13830]  makefile.vc
│                           ├── [      17368]  nmakehlp.c
│                           └── [      18743]  rules.vc
├── [       6862]  CHANGELOG.md
├── [       4096]  cloudformation
├── [       2814]  CONTRIBUTING.md
├── [       4096]  deps
│   ├── [       1258]  common-sqlite.gypi
│   ├── [        191]  extract.py
│   ├── [       2761]  sqlite3.gyp
│   └── [    2810415]  sqlite-autoconf-3280000.tar.gz
├── [       2286]  Dockerfile
├── [       4096]  lib
│   ├── [       4096]  binding
│   │   └── [       4096]  node-v64-linux-ia32
│   │       └── [    1329364]  node_sqlite3.node
│   ├── [         39]  index.js
│   ├── [       6437]  sqlite3.js
│   └── [       1437]  trace.js
├── [       1460]  LICENSE
├── [       4096]  node_modules
│   ├── [       4096]  node-pre-gyp
│   │   ├── [        794]  appveyor.yml
│   │   ├── [       4096]  bin
│   │   │   ├── [       3265]  node-pre-gyp
│   │   │   └── [         40]  node-pre-gyp.cmd
│   │   ├── [      12993]  CHANGELOG.md
│   │   ├── [        288]  contributing.md
│   │   ├── [       4096]  lib
│   │   │   ├── [       1909]  build.js
│   │   │   ├── [       1237]  clean.js
│   │   │   ├── [       2165]  configure.js
│   │   │   ├── [       1505]  info.js
│   │   │   ├── [       9133]  install.js
│   │   │   ├── [       4703]  node-pre-gyp.js
│   │   │   ├── [       2235]  package.js
│   │   │   ├── [       1076]  pre-binding.js
│   │   │   ├── [       3796]  publish.js
│   │   │   ├── [        567]  rebuild.js
│   │   │   ├── [        620]  reinstall.js
│   │   │   ├── [       1085]  reveal.js
│   │   │   ├── [       3212]  testbinary.js
│   │   │   ├── [       1812]  testpackage.js
│   │   │   ├── [       1723]  unpublish.js
│   │   │   └── [       4096]  util
│   │   │       ├── [      24507]  abi_crosswalk.json
│   │   │       ├── [       2998]  compile.js
│   │   │       ├── [       3642]  handle_gyp_opts.js
│   │   │       ├── [       7841]  napi.js
│   │   │       ├── [       4096]  nw-pre-gyp
│   │   │       │   ├── [        542]  index.html
│   │   │       │   └── [        160]  package.json
│   │   │       ├── [        769]  s3_setup.js
│   │   │       └── [      15445]  versioning.js
│   │   ├── [       1501]  LICENSE
│   │   ├── [       2338]  package.json
│   │   └── [      33030]  README.md
│   ├── [       4096]  rimraf
│   │   ├── [       1196]  bin.js
│   │   ├── [        765]  LICENSE
│   │   ├── [       1716]  package.json
│   │   ├── [       3600]  README.md
│   │   └── [       8994]  rimraf.js
│   └── [       4096]  semver
│       ├── [       4096]  bin
│       │   └── [       4418]  semver
│       ├── [        672]  CHANGELOG.md
│       ├── [        765]  LICENSE
│       ├── [       1565]  package.json
│       ├── [        619]  range.bnf
│       ├── [      15719]  README.md
│       └── [      38803]  semver.js
├── [       3552]  package.json
├── [       9908]  README.md
├── [         43]  sqlite3.js
├── [       4096]  src
│   ├── [       1976]  async.h
│   ├── [      12837]  backup.cc
│   ├── [       6771]  backup.h
│   ├── [      19764]  database.cc
│   ├── [       4964]  database.h
│   ├── [        249]  gcc-preinclude.h
│   ├── [      10842]  macros.h
│   ├── [       4811]  node_sqlite3.cc
│   ├── [      27143]  statement.cc
│   ├── [       6453]  statement.h
│   └── [       1153]  threading.h
└── [       4096]  tools
    └── [       4096]  docker
        └── [       4096]  architecture
            └── [       4096]  linux-arm64
                ├── [       2531]  Dockerfile
                └── [       1982]  run.sh

30 directories, 119 files
kewde commented 5 years ago

The only required files after a successful build are:

package.json
sqlite3.js
lib/**/*
node_modules/**/*

Regarding electron-builder specifically, I don't know if it recursively goes through the sqlite3 dependency and adheres to the exclusion files defined in our package.json as well. I suppose it's worth a try.

kewde commented 5 years ago

Please test out #1186, it's on branch electron-builds-light (https://github.com/mapbox/node-sqlite3/tree/electron-builds-light)

fastman commented 5 years ago

@kewde Should it work for normal installs too, or is it an electron specific thing?

kewde commented 5 years ago

It is an electron specific thing (the fix).

The problem however is generic, you need the source code to compile it. Deleting it after it's done, means you'd have to reinstall the package whenever you change version of node (requires recompilation).

fastman commented 5 years ago

@kewde thanks. What's the problem with deleting the source files? As you said, if you switch node version you have to recompile, so basically you have to reinstall the package. One thing is when you are installing the module for local development, other thing is when installing in produiction env. Would it be possible to distinguish those two envs? Something like when using production env and installing modules defined in package.json - devDependencies are not installed. (NODE_ENV=production npm install or npm install --production) What do you think?

mgrandi commented 5 years ago

if you switch node versions and that has an ABI change then you will need to recompile anyway, whether or not the source code is still present after the final build step. I'm not a nodejs programmer but i feel like the node_module folders should be tied to one specific version, much like the python virtualenv folders are tied to a specific python version upon creation

fastman commented 5 years ago

@mgrandi that's what I'm saying, once the modules is installed in production, any changes to the node version would require the sqlite3 module be "reinstalled" (recompiled). What I'm trying to say is that it should be safe to remove source files once the module has been installed using produciton env.

kewde commented 5 years ago

A change in node versions requires a recompilation, but not technically a complete "reinstall". A reinstall however will trigger compilation (if requested by the flags).

Will a npm install --build-from-source sqlite3 download the sources files again after they've been deleted? I would expect it to do a full reinstall, but I'd need to confirm that for sure. If so, we could trim down the folder to match only the required files for production environments.

I'm up for changing this behavior but not at the cost of suddenly 5 more issues saying that workflow suddenly broke because of the new patch.

mgrandi commented 5 years ago

i would expect that this project is only the sqlite3 C -> nodeJS code, and the build step would be to download the sqlite3 sources, build it , and then delete the sources afterwards

fastman commented 5 years ago

@mgrandi yup, that would be great!

tessus commented 5 years ago

@kewde I tested this, but it doesn't seem to work.

Please note that I ALWAYS have to run ./node_modules/.bin/electron-rebuild after installing sqlite3 via npm, otherwise I get Uncaught Error: Cannot find module.

After I run the electron-rebuild, the build directory is created in node_modules/sqlite3. I thought the fix should clean/remove the build dir after, but this doesn't happen. npm run dist which creates the Joplin app also does not remove the build dir.

daniellockyer commented 2 years ago

I welcome PRs for this 🙂