Repository navigation
fix: perch could not load its parser on Windows - #441
Conversation
|
Perch Scan: no issues in 200 methods at 1 issue fixed since the last push. |
![perchcode-bot[bot]](https://avatars.githubusercontent.com/in/5082622?s=60&v=4){kind=small}
There was a problem hiding this comment.
Perch found 6 issues in code this pull request changed. See the scan
| * Renames one DLL in a PE file's import table, in place, and says whether it found it. The new name must be no longer than the | ||
| * old, since the string is overwritten where it sits. | ||
| */ | ||
| export function renameImport(pe: Buffer, from: string, to: string): boolean { |
There was a problem hiding this comment.
Fixed: the scan of 7e0f121 no longer reports this.
P1 Defect: off by one (99%)
Perch believes renameImport has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/treesitter/pack.ts::renameImport. See the scan
There was a problem hiding this comment.
The scan of 7e0f121 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| import { tmpdir } from 'node:os'; | ||
| import { dirname, join } from 'node:path'; | ||
|
|
||
| type Pack = typeof import('@xberg-io/tree-sitter-language-pack'); |
There was a problem hiding this comment.
Fixed: the scan of 7e0f121 no longer reports this.
P1 Defect: off by one (89%)
Perch believes <top-level> has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/treesitter/pack.ts::<top-level>. See the scan
There was a problem hiding this comment.
The scan of 7e0f121 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| * to node.exe binds to the running Node.js, which is what every node-gyp addon does. The copy is loaded in place of the pack's | ||
| * index.js, which does nothing but load the addon and export it. A pack whose addon imports no node.dll loads as published. | ||
| */ | ||
| function load(): Pack { |
There was a problem hiding this comment.
Fixed: the scan of 7e0f121 no longer reports this.
P1 Defect: off by one (87%)
Perch believes load has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/treesitter/pack.ts::load. See the scan
There was a problem hiding this comment.
The scan of 7e0f121 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| * to node.exe binds to the running Node.js, which is what every node-gyp addon does. The copy is loaded in place of the pack's | ||
| * index.js, which does nothing but load the addon and export it. A pack whose addon imports no node.dll loads as published. | ||
| */ | ||
| function load(): Pack { |
There was a problem hiding this comment.
Fixed: the scan of a707bdd no longer reports this.
P1 Security: weak crypto (79%)
Perch believes load has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/treesitter/pack.ts::load. See the scan
There was a problem hiding this comment.
The scan of a707bdd no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| import { tmpdir } from 'node:os'; | ||
| import { dirname, join } from 'node:path'; | ||
|
|
||
| type Pack = typeof import('@xberg-io/tree-sitter-language-pack'); |
There was a problem hiding this comment.
Fixed: the scan of a707bdd no longer reports this.
P1 Security: weak crypto (71%)
Perch believes <top-level> has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/treesitter/pack.ts::<top-level>. See the scan
There was a problem hiding this comment.
The scan of a707bdd no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| const optional = header + 24; | ||
| const directories = optional + (pe.readUInt16LE(optional) === 0x20b ? 112 : 96); | ||
| const table = optional + pe.readUInt16LE(header + 20); | ||
| const offset = (rva: number): number | undefined => { |
There was a problem hiding this comment.
Fixed: the scan of 7e0f121 no longer reports this.
P2 Defect: off by one (95%)
Perch believes renameImport.offset has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/treesitter/pack.ts::renameImport.offset. See the scan
There was a problem hiding this comment.
The scan of 7e0f121 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
There was a problem hiding this comment.
Perch found an issue in code this pull request changed. See the scan
| @@ -244,9 +244,11 @@ export function openStore(out) { | |||
| }, | |||
| /** Ignore generated results while allowing closures and the default rule directory to be committed. */ | |||
| async exclude(root) { | |||
There was a problem hiding this comment.
Fixed: the scan of 7e0f121 no longer reports this.
P2 Defect: off by one (68%)
Perch believes openStore.store.exclude has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/store.js::openStore.store.exclude. See the scan
There was a problem hiding this comment.
The scan of 7e0f121 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
There was a problem hiding this comment.
Perch found an issue in code this pull request changed. See the scan
|
|
||
| /** A fresh repository on `main` with a committed tree, then checked out on a feature branch, since fix and refactor commit to the current branch. */ | ||
| /** Windows will not remove a directory something still has open, and a test's repository can stay open a moment after the test ends. */ | ||
| export const removeDir = dir => rm(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 200 }); |
There was a problem hiding this comment.
Fixed: the scan of 7e0f121 no longer reports this.
P2 Defect: leak (63%)
Perch believes removeDir has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check test/helpers.js::removeDir. See the scan
There was a problem hiding this comment.
The scan of 7e0f121 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
The Windows build of the parser addon in @xberg-io/tree-sitter-language-pack 1.22.1 cannot load, and nothing in CI ran on Windows to notice.
The pack's Windows addon imports two N-API functions from node.dll, which Node.js does not ship. On Windows perch now loads a copy of the addon with that import renamed to node.exe, which exports them.
exclude compared the results directory against the root with a '/' prefix, which a Windows path never has. The Windows CI job runs the parser tests; the scan tests do not yet pass there.
Test cleanup retries removing a temp repository, since Windows refuses while it is still open.
A file already at the temp path is checked byte for byte before it is loaded, and a section's address maps to the file only inside its raw data.
facc7bf to
6c2f826
Compare
There was a problem hiding this comment.
Perch found an issue in code this pull request changed. See the scan
|
|
||
| /** A fresh repository on `main` with a committed tree, then checked out on a feature branch, since fix and refactor commit to the current branch. */ | ||
| /** Windows will not remove a directory something still has open, and a test's repository can stay open a moment after the test ends. */ | ||
| export const removeDir = dir => rm(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 200 }); |
There was a problem hiding this comment.
Fixed: the scan of 0a11efe no longer reports this.
P2 Defect: other defect (63%)
Perch believes removeDir has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check test/helpers.js::removeDir. See the scan
There was a problem hiding this comment.
The scan of 0a11efe no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
Perch stopped before reading anything on Windows. The parser addon in
@xberg-io/tree-sitter-language-pack@1.22.1importsnapi_create_externalandnapi_type_tag_objectfromnode.dll, which Node.js does not ship, so Windows refused to load it with "The specified module could not be found" (xberg-io/tree-sitter-language-pack#208).node.exe, which exports both functions, and loads the copy. An addon that imports nonode.dllloads as published, so the patch goes inert once upstream ships the fix (fix(node): Windows addon imported node.dll and failed to load xberg-io/tree-sitter-language-pack#213).src/treesitter/pack.ts..perch/was never gitignored on Windows: perch checked that the results directory was inside the repository with a/prefix, which a Windows path does not have.perch --version, and runs the parser, scan and CLI tests on Windows under Node 22 and 24.Windows users update perch. Nothing else changes.
Closes #440