-
-
Couldn't load subscription status.
- Fork 7.3k
Ensuring private Buffer require in lib/* files #8603
Conversation
|
cc @trevnorris @odeke-em : could you please take a look at CONTRIBUTING.md to see how your commit message should be improved to be accepted? ;) |
2cecd27 to
7f87f4d
Compare
|
Thanks @indutny , I have taken a look at it and made an amend to the commit message. Please help me check if it is appropriate. |
|
Commit message first line should not be longer than 50 characters. Other than that LGTM. |
7f87f4d to
fca422f
Compare
|
Thanks for the patience. |
|
Thanks. Now, I just barely ran the tests. Things are falling over quite badly. Issue is there are places where |
|
Hey @trevnorris, I guess I bit on more than I could chew: could I kindly ask if someone with more knowledge of the internals [ like a core developer ] look at this since we'd probably want this done well? Instead of me holding you and @indutny back. |
|
@odeke-em No worries at all. Honestly I can't think of a good solution ATM. I'll leave this open and if you want to keep experimenting with it feel free. |
|
Looks like a simple case of circular dependencies. Try this: diff --git a/lib/assert.js b/lib/assert.js
index 0ab3f49..5bd30ea 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -24,7 +24,7 @@
// UTILITY
var util = require('util');
-var Buffer = require('buffer').Buffer;
+var b = require('buffer');
var pSlice = Array.prototype.slice;
// 1. The assert module provides functions that throw
@@ -145,7 +145,7 @@ function _deepEqual(actual, expected) {
if (actual === expected) {
return true;
- } else if (Buffer.isBuffer(actual) && Buffer.isBuffer(expected)) {
+ } else if (b.Buffer.isBuffer(actual) && b.Buffer.isBuffer(expected)) {
if (actual.length != expected.length) return false;
for (var i = 0; i < actual.length; i++) { |
Fixes usage of global object 'Buffer' in lib/* files by ensuring
that each file does an explicit require('buffer').Buffer.
Previously, when running a repl, due to usage of global 'Buffer',
any redefinition of Buffer would cause a crash eg var Buffer = {}.
fca422f to
6326bc7
Compare
|
@trevnorris For sure, will do. |
|
@vkurchatkin Thanks! Your solution cuts the deal, tests related to those modules on mine pass. Only thing tripped out on mine are errors related to EMFILE [too many files open], but I don't think that's directly related to the issue at hand. |
|
@trevnorris ping! |
Fixes usage of global object 'Buffer' in lib/* files by ensuring that
each file does an explicit require('buffer').Buffer. Previously, when
running a repl, due to usage of global 'Buffer', any redefinition of
Buffer would cause a crash eg var Buffer = {}.
Fixes: #8588
PR-URL: #8603
Reviewed-by: Trevor Norris <[email protected]>
|
Thanks. Merged into 523929c. |
|
Thanks for merging this in. |
Fixes usage of global object 'Buffer' in lib/* files by ensuring that
each file does an explicit require('buffer').Buffer. Previously, when
running a repl, due to usage of global 'Buffer', any redefinition of
Buffer would cause a crash eg var Buffer = {}.
Fixes: nodejs#8588
PR-URL: nodejs#8603
Reviewed-by: Trevor Norris <[email protected]>
Fixes usage of global object 'Buffer' in lib/* files by ensuring that
each file does an explicit require('buffer').Buffer. Previously, when
running a repl, due to usage of global 'Buffer', any redefinition of
Buffer would cause a crash eg var Buffer = {}.
Fixes: nodejs/node-v0.x-archive#8588
PR-URL: nodejs/node-v0.x-archive#8603
Reviewed-by: Trevor Norris <[email protected]>
Conflicts:
lib/_stream_readable.js
lib/_stream_writable.js
lib/assert.js
lib/dgram.js
lib/fs.js
lib/http.js
lib/net.js
lib/readline.js
lib/tls.js
lib/zlib.js
Port of nodejs/node-v0.x-archive#8603 The race condition present in the original PR didn't occur, so no workaround was needed. PR-URL: #1794 Reviewed-By: Trevor Norris <[email protected]>
This addresses issue #8588 in which Buffer was being used globally in lib/* files. Any redefinition of Buffer when using a repl would crash. Credits to @indutny and @vkurchatkin for providing the remedy.