nodejs / llnode

An lldb plugin for Node.js and V8, which enables inspection of JavaScript states for insights into Node.js processes and their core dumps.
Other
1.15k stars 99 forks source link

Install Fails for LLDB-10 #350

Closed No9 closed 1 year ago

No9 commented 4 years ago

FYI - It looks like https://github.com/llvm-mirror/lldb.git doesn't have a release_100 branch that maps to lldb-10. So the error below gets thrown.

Does it need a patch to warn the user in a friendlier way? Not 100% about the release cadence on the llvm mirror

Cloning lldb release_100 into /usr/local/lib/node_modules/llnode/lldb-10.0
Cloning into '/usr/local/lib/node_modules/llnode/lldb-10.0'...
warning: Could not find remote branch release_100 to clone.
fatal: Remote branch release_100 not found in upstream origin
child_process.js:651
    throw err;
    ^

Error: Command failed: git clone --depth 1 --branch release_100 https://github.com/llvm-mirror/lldb.git /usr/local/lib/node_modules/llnode/lldb-10.0
    at checkExecSyncError (child_process.js:630:11)
    at Object.execFileSync (child_process.js:648:15)
    at Object.cloneHeaders (/usr/local/lib/node_modules/llnode/scripts/lldb.js:47:19)
    at configureInstallation (/usr/local/lib/node_modules/llnode/scripts/configure.js:89:25)
    at main (/usr/local/lib/node_modules/llnode/scripts/configure.js:12:34)
    at Object.<anonymous> (/usr/local/lib/node_modules/llnode/scripts/configure.js:20:1)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
    at Module.load (internal/modules/cjs/loader.js:1002:32)
    at Function.Module._load (internal/modules/cjs/loader.js:901:14) {
  status: 128,
  signal: null,
  output: [ null, null, null ],
  pid: 3484,
  stdout: null,
  stderr: null
}
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! llnode@3.0.0 preinstall: `node scripts/configure.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the llnode@3.0.0 preinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
mmarchini commented 4 years ago

I might have a patch to fix it (had a similar problem with lldb 7.1).

No9 commented 4 years ago

Thanks @mmarchini - I've down graded to 9 for now so it's not urgent if you have something already then ill let you land it. Happy to test if you would like a second pair of eyes.

mmarchini commented 4 years ago

Ok, it's not what I imagined, but I did some work in the past to address this problem as well. What happened is the LLVM project officially moved to GitHub last hear, and as such it seems the llvm-mirror org hasn't been updated anymore (which makes sense). The source of truth now is https://github.com/llvm/llvm-project. The problem is if someone clones it they'll get the entire llvm project (which is huge) instead of just lldb. The tags and branches format is also different, and I'm pretty sure I hit a edge case where the version on my machine didn't had a tag.

With that being said, it seems like headers are now available as release artifacts there, so I'll investigate using that instead of a git clone.

Edit: well, not all versions have headers as release artifacts, so we'll need a bit more fiddling... It'll be a bit more work than I imagined.

mmarchini commented 4 years ago

On a side note, if you also install lldb headers you can probably use LLDB 10 without problems.

mmarchini commented 4 years ago

Starting to wonder if we should just ship the headers by ourselves instead :thinking:

joyeecheung commented 4 years ago

I guess if we ship the headers, we need to figure out a way to make it work across different lldb versions..we could put the headers for supported versions in this repo and fetch them during installation, or just bundle them all in (though I am not sure how much bigger the resulting tarball would be)

mmarchini commented 4 years ago

Yes, those are all good options. What I've heard from an lldb maintainer is that they try to keep the ABI backwards compatible across major changes, although I'm not sure how well it works on practice. Should be easy enough to setup a GitHub Action to use the headers from, let's say, 3.9 and test on all majors up to 10. If this works we might even be able to provide pre built binaries.

codisms commented 3 years ago

What I found was happening is that the function getLlvmConfig() was expecting either the executable llvm-config-10.0 or llvm-config to be available in the PATH. Neither was in my path so I added a symlink to resolve the issue:

cd /usr/bin
sudo ln -s ../lib/llvm-10/bin/llvm-config
bravely commented 2 years ago

@codisms Right on the money, thanks!

No9 commented 1 year ago

Closing this as the work is happening in #389