Skip to content
This repository has been archived by the owner on Mar 4, 2022. It is now read-only.

Hack a quick fix to concurrent buffering issues #121

Closed
wants to merge 1 commit into from

Conversation

ryan-roemer
Copy link
Member

Add console.log() hack to flush buffers in builder concurrent --buffer. #120

/cc @exogen

@coveralls
Copy link

coveralls commented Aug 24, 2016

Coverage Status

Coverage increased (+0.01%) to 92.593% when pulling aa31208 on bug-concurrentBufferHack into 87edffe on master.

// but here we are...
//
// TODO: https://github.com/FormidableLabs/builder/issues/120
console.log(""); // eslint-disable-line no-console
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw/, process.stdout.write("") didn't work. But by all accounts, the above is almost identical to it: https://github.com/nodejs/node/blob/master/lib/console.js#L39-L44

Console.prototype.log = function(...args) {
  this._stdout.write(`${util.format.apply(null, args)}\n`);
};

So, we'll definitely categorize this as "very, very hacky" / "shouldn't help but it does"...

@exogen
Copy link
Contributor

exogen commented Aug 24, 2016

Now that we have a repro, I tried running it with FormidableLabs/builder#bug-concurrentBufferHack – unfortunately this doesn't appear to fix it!

@ryan-roemer
Copy link
Member Author

ryan-roemer commented Aug 24, 2016

@exogen -- UPDATE:

$ builder run test --buffer
// snipped

XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 *** THIS SHOULD BE LOGGED! *** 

[builder:proc:end:0] Command: node scripts/log.js
[builder:builder-core:end:39125] Task: run test ended normally

Worked for me on Node 0.10 (with LOGGED output) and not on 4 (no LOGGED output).

@exogen
Copy link
Contributor

exogen commented Aug 24, 2016

@ryan-roemer Nope:

$ builder run test --buffer
// snip
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
[builder:proc:end:0] Command: node scripts/log.js
[builder:builder-core:end:35473] Task: run test ended normally

(and I confirmed that node_modules/builder/lib/runner.js has the change here.)

Also, doesn't --buffer only apply to builder concurrent? Anyway, I also tried it with builder concurrent --buffer and builder run --unlimited-buffer, none of 'em show the expected output.

Maybe it's a Node version thing?

$ node -v
v4.4.6

@exogen
Copy link
Contributor

exogen commented Aug 24, 2016

FYI, I just checked Node 0.12 and I get the expected output even without this patch or any builder flags.

@ryan-roemer
Copy link
Member Author

Not needed and didn't solve the problem 😛

@ryan-roemer ryan-roemer deleted the bug-concurrentBufferHack branch August 30, 2016 18:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants