Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Binary relocations #93

Merged
merged 3 commits into from
Dec 3, 2018
Merged

Binary relocations #93

merged 3 commits into from
Dec 3, 2018

Conversation

guybedford
Copy link
Contributor

Implements #57 in statically analyzing require('bindings') and require('node-pre-gyp').find loads and handling them as asset relocations.

Cases supported:

  • import binary from 'node-pre-gyp'; binary.find(__dirname + 'package.json') (plus import * variant)
  • require('bindings')('asdf')
  • var bindings = require('bindings'); bindings('asdf')

This should be merged with the other PRs, so will rebase depending on what order the merges happen.

Because of the issue with architecture matching, I originally wanted to enable this specifically behind a "binary" flag, but since the default case is then effectively for binaries just not to work, I've left this out for now.

@guybedford
Copy link
Contributor Author

Fixes #33.

@codecov-io
Copy link

codecov-io commented Dec 3, 2018

Codecov Report

Merging #93 into master will increase coverage by 2.24%.
The diff coverage is 82.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   67.54%   69.78%   +2.24%     
==========================================
  Files           3        3              
  Lines         265      331      +66     
==========================================
+ Hits          179      231      +52     
- Misses         86      100      +14
Impacted Files Coverage Δ
src/asset-relocator.js 92.22% <82.71%> (-6.99%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 290b4a0...ee69641. Read the comment docs.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants