Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix build breaks discovered on Node 18.x #818

Merged
merged 2 commits into from
Dec 2, 2022

Conversation

BillyONeal
Copy link
Member

I'm not sure about the exact cause of this failure but I suspect it's a change from node version since that's the only thing we've changed:

  1) TarBzUnpacker
       UnpacksLegitimateSmallTarBz:
     Error: Premature close
      at new NodeError (node:internal/errors:393:5)
      at Stream.<anonymous> (node:internal/streams/pipeline:352:14)
      at Stream.emit (node:events:525:35)
      at Stream.stream.destroy (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:84:12)
      at _end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:67:14)
      at Stream.stream.end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:74:5)
      at ProgressTrackingStream.onend (node:internal/streams/readable:705:10)
      at Object.onceWrapper (node:events:627:28)
      at ProgressTrackingStream.emit (node:events:525:35)
      at endReadableNT (node:internal/streams/readable:1359:12)
      at processTicksAndRejections (node:internal/process/task_queues:82:21)

  2) TarBzUnpacker
       ImplementsUnpackOptions:
     Error: Premature close
      at new NodeError (node:internal/errors:393:5)
      at Stream.<anonymous> (node:internal/streams/pipeline:352:14)
      at Stream.emit (node:events:525:35)
      at Stream.stream.destroy (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:84:12)
      at _end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:67:14)
      at Stream.stream.end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:74:5)
      at ProgressTrackingStream.onend (node:internal/streams/readable:705:10)
      at Object.onceWrapper (node:events:627:28)
      at ProgressTrackingStream.emit (node:events:525:35)
      at endReadableNT (node:internal/streams/readable:1359:12)
      at processTicksAndRejections (node:internal/process/task_queues:82:21)

I'm pretty sure almost all of this compression code is filthy with how fds are treated; running the tests still gives:

(node:26580) Warning: Closing file descriptor 3 on garbage collection
(Use `node --trace-warnings ...` to show where the warning was created)
(node:26580) [DEP0137] DeprecationWarning: Closing a FileHandle object on garbage collection is deprecated. Please close FileHandle objects explicitly using FileHandle.prototype.close(). In the future, an error will be thrown if a file descriptor is closed during garbage collection.

In the medium term we will throw all of this out and feed it all to libarchive (either through linking vcpkg to libarchive ourselves or feeding it to cmake -E tar) but this targeted workaround gets my development working again.

… a change from node version since that's the only thing we've changed:

```
  1) TarBzUnpacker
       UnpacksLegitimateSmallTarBz:
     Error: Premature close
      at new NodeError (node:internal/errors:393:5)
      at Stream.<anonymous> (node:internal/streams/pipeline:352:14)
      at Stream.emit (node:events:525:35)
      at Stream.stream.destroy (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:84:12)
      at _end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:67:14)
      at Stream.stream.end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:74:5)
      at ProgressTrackingStream.onend (node:internal/streams/readable:705:10)
      at Object.onceWrapper (node:events:627:28)
      at ProgressTrackingStream.emit (node:events:525:35)
      at endReadableNT (node:internal/streams/readable:1359:12)
      at processTicksAndRejections (node:internal/process/task_queues:82:21)

  2) TarBzUnpacker
       ImplementsUnpackOptions:
     Error: Premature close
      at new NodeError (node:internal/errors:393:5)
      at Stream.<anonymous> (node:internal/streams/pipeline:352:14)
      at Stream.emit (node:events:525:35)
      at Stream.stream.destroy (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:84:12)
      at _end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:67:14)
      at Stream.stream.end (C:\Dev\vcpkg-tool\ce\common\temp\node_modules\.pnpm\through@2.3.8\node_modules\through\index.js:74:5)
      at ProgressTrackingStream.onend (node:internal/streams/readable:705:10)
      at Object.onceWrapper (node:events:627:28)
      at ProgressTrackingStream.emit (node:events:525:35)
      at endReadableNT (node:internal/streams/readable:1359:12)
      at processTicksAndRejections (node:internal/process/task_queues:82:21)
```

I'm pretty sure almost all of this compression code is filthy with how fds are treated; running the tests still gives:

```
(node:26580) Warning: Closing file descriptor 3 on garbage collection
(Use `node --trace-warnings ...` to show where the warning was created)
(node:26580) [DEP0137] DeprecationWarning: Closing a FileHandle object on garbage collection is deprecated. Please close FileHandle objects explicitly using FileHandle.prototype.close(). In the future, an error will be thrown if a file descriptor is closed during garbage collection.
```

In the medium term we will throw all of this out and feed it all to libarchive (either through linking vcpkg to libarchive ourselves or feeding it to `cmake -E tar`) but this targeted workaround gets my development working again.
@BillyONeal BillyONeal merged commit 3e84392 into microsoft:main Dec 2, 2022
@BillyONeal BillyONeal deleted the node-compat-fix branch December 2, 2022 19:22
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.

2 participants