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

profiler: declare missing printErr #13590

Closed
wants to merge 1 commit into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Jun 9, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

profiler?

v8/tools/tickprocessor.js assumes presence of global printErr, which
is defined in d8.

`v8/tools/tickprocessor.js` assumes presence of global `printErr`, which
is defined in `d8`.
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Jun 9, 2017
@indutny
Copy link
Member Author

indutny commented Jun 9, 2017

cc @nodejs/collaborators

@indutny
Copy link
Member Author

indutny commented Jun 9, 2017

cc @nodejs/v8

@refack
Copy link
Contributor

refack commented Jun 9, 2017

@indutny any way to regression test this? (point me in the right direction I'll write the test)?

@refack
Copy link
Contributor

refack commented Jun 9, 2017

@indutny any way to regression test this? (point me in the right direction I'll write the test)?

No need test/tick-processor/test-tick-processor-unknown.js fails:

=== release test-tick-processor-unknown ===
Path: tick-processor/test-tick-processor-unknown
undefined:2649
  printErr(str);
  ^

ReferenceError: printErr is not defined
    at TickProcessor.printError (eval at <anonymous> (internal/v8_prof_processor.js:32:1), <anonymous>:2649:3)
    at TickProcessor.LogReader.processLog_ (eval at <anonymous> (internal/v8_prof_processor.js:32:1), <anonymous>:2409:12)
    at TickProcessor.LogReader.processLogLine (eval at <anonymous> (internal/v8_prof_processor.js:32:1), <anonymous>:2292:10)
    at TickProcessor.processLogFile (eval at <anonymous> (internal/v8_prof_processor.js:32:1), <anonymous>:2677:10)
    at eval (eval at <anonymous> (internal/v8_prof_processor.js:32:1), <anonymous>:3844:15)
    at eval (eval at <anonymous> (internal/v8_prof_processor.js:32:1), <anonymous>:3848:3)
    at internal/v8_prof_processor.js:32:1
    at NativeModule.compile (bootstrap_node.js:563:7)
    at Function.NativeModule.require (bootstrap_node.js:506:18)
    at startup (bootstrap_node.js:116:20)
undefined:2649
  printErr(str);
  ^

@indutny
Copy link
Member Author

indutny commented Jun 9, 2017

@refack yeah.

@refack
Copy link
Contributor

refack commented Jun 9, 2017

@addaleax
Copy link
Member

Landed in 27cc30a

@addaleax addaleax closed this Jun 12, 2017
addaleax pushed a commit that referenced this pull request Jun 12, 2017
`v8/tools/tickprocessor.js` assumes presence of global `printErr`, which
is defined in `d8`.

PR-URL: #13590
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
addaleax pushed a commit that referenced this pull request Jun 12, 2017
`v8/tools/tickprocessor.js` assumes presence of global `printErr`, which
is defined in `d8`.

PR-URL: #13590
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@addaleax addaleax mentioned this pull request Jun 12, 2017
@indutny indutny deleted the fix/profiler-error branch June 13, 2017 07:13
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
`v8/tools/tickprocessor.js` assumes presence of global `printErr`, which
is defined in `d8`.

PR-URL: #13590
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@MylesBorins
Copy link
Contributor

landed on v6.x. Let me know if it should be backed out

@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants