[webkit-reviews] review denied: [Bug 177430] Tools/Scripts/run-jsc --debugger no longer works in recent versions of Mac OS : [Attachment 321683] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 177430: Tools/Scripts/run-jsc --debugger no longer works in recent versions
of Mac OS
https://bugs.webkit.org/show_bug.cgi?id=177430

Attachment 321683: Patch

https://bugs.webkit.org/attachment.cgi?id=321683&action=review




--- Comment #4 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 321683
  --> https://bugs.webkit.org/attachment.cgi?id=321683
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321683&action=review

The direction is great. I’d like to see another iteration with code cleanup
before r+’ing.

> Tools/Scripts/run-jsc:50
> +my $dyld = jscProductDir();

This is not a good variable name. dyld is the dynamic linker, and this is just
its configuration option. 

Something like buildDirectory or productDirectory would be better.

> Tools/Scripts/run-jsc:51
> +$ENV{"DYLD_FRAMEWORK_PATH"} = $dyld;

This is only needed or available on Darwin, not everywhere. While technically
it’s ok to add unused environment variables, we try to do it cleaner than that.

> Tools/Scripts/run-jsc:56
> +    $jsc = $debuggerCmd . " -O \"env DYLD_FRAMEWORK_PATH=" . $dyld . "\" " .
File::Spec->catfile(jscProductDir(), "jsc -- ") . "@ARGV";

What will -O do for debuggers other than lldb?