Bug 187687 – Add --target-path option to dump-class-layout

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Add --target option to dump-class-layout

[Yusuke Suzuki]

Created attachment 345064 [details]
Patch

[Yusuke Suzuki]

Created attachment 345065 [details]
Patch

[Daniel Bates]

Comment on attachment 345065 [details]
Patch

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

> Tools/Scripts/dump-class-layout:78
> +    if not args.target_path == None:

I do not know much about the JSCOnly port. Would it make sense (and solve the problem your are having) to expose a —root option like we do for run-webkit-tests to provide a path to an arbitrary directory for the built products? If so, I suggest we do that for consistency.

If we choose to go with the approach proposed in this patch then please change “== None”  to “is None” (per PEP8)  and I am assuming that argparse will emit and error if target-path is the empty string. Otherwise, we should handle that. Again I would prefer we add —root over —target-path.

[Yusuke Suzuki]

Comment on attachment 345065 [details]
Patch

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

>> Tools/Scripts/dump-class-layout:78
>> +    if not args.target_path == None:
> 
> I do not know much about the JSCOnly port. Would it make sense (and solve the problem your are having) to expose a —root option like we do for run-webkit-tests to provide a path to an arbitrary directory for the built products? If so, I suggest we do that for consistency.
> 
> If we choose to go with the approach proposed in this patch then please change “== None”  to “is None” (per PEP8)  and I am assuming that argparse will emit and error if target-path is the empty string. Otherwise, we should handle that. Again I would prefer we add —root over —target-path.

I don't think `--root` solves the problem. While macOS ports build JSC and WebCore as `framework`, JSCOnly, GTK, and WPE ports are not. They build JSC and WebCore as shared library `.so`.
Furthermore, the names of shared libraries are slightly different in each port. JSCOnly port's shared library is `libJavaScriptCore.so`, GTK ports one is `libjavascriptcoregtk-4.0.so`.
To support these things with `--root` option, we need to handle these differences in python code, which complicates dump-class-layout.

Instead of doing that, just passing a library with `--target-path` is simple. So I would like to choose this approach.

I'll fix `is None` thing and upload the patch.

[Yusuke Suzuki]

Created attachment 345071 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 345071 [details]
Patch

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

> Tools/ChangeLog:8
> +        We add a hatch to dump-class-layout for specifying target path directly.

A hatch? Maybe "argument"?

[Yusuke Suzuki]

Comment on attachment 345071 [details]
Patch

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

>> Tools/ChangeLog:8
>> +        We add a hatch to dump-class-layout for specifying target path directly.
> 
> A hatch? Maybe "argument"?

Ah, that means `an escape hatch`. Fixed.

[Yusuke Suzuki]

Committed r233856: <https://trac.webkit.org/changeset/233856>

[Radar WebKit Bug Importer]

<rdar://problem/42249696>