Bug 149904 – [Win] Support 64-bit Build and Testing

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Brent Fulgham]

Several of our scripts don't do the right thing if you are attempting to build 64-bit Windows code. This patch corrects that problem.

[Brent Fulgham]

Created attachment 262655 [details]
Patch

[Daniel Bates]

Comment on attachment 262655 [details]
Patch

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

> Tools/ChangeLog:29
> +        (WinPort._build_path): Added properly binary target path to _build_path.

This sentence does not read well.

> Tools/ChangeLog:33
> +        ntsd debugger does not understand this argument.

Nit: ntsd => NTSD

(NTSD is an acronym for (Microsoft) NT Symbolic Debugger)

> Tools/Scripts/build-dumprendertree:64
> +chdir File::Spec->catdir('Tools', 'DumpRenderTree') or die;

We use double quoted string literals in Perl code unless the literal contains a double quote or we explicitly do not want string interpolation to be performed.

> Tools/Scripts/webkitdirs.pm:622
> +    my $binDir;

This is OK as-is. I don't see the need to abbreviate the word "directory" in the variable name "binDir". I know we use this abbreviation throughout the code, including in the name "productDir". Having said that, I don't feel we should continue such a naming convention.

> Tools/Scripts/webkitpy/port/factory.py:67
> +        optparse.make_option('--64-bit', action='store_const', const='x86_64', default=None, dest="architecture",
> +            help='use 64-bit binaries by default (x86_64 instead of x86)'),

This is OK as-is. I know you are just mimicking the existing code style. It's weird that we alternate between using single quoted and double quoted literals.

> Tools/Scripts/webkitpy/port/win.py:131
> +            binPath = 'bin32'

Nit: In Python code variable names should use the words separated by underscores naming convention. The name of this variable is also inconsistent with the name of the analogous variable used in the webkitdirs::executableProductDir(), binDir. I suggest that we name this variable bin_directory to be consistent. Regardless, we should pick a name for the variable that represents the name of the bin directory and use in both the Perl and the Python code up to naming convention style for consistency.

> Tools/Scripts/webkitpy/port/win.py:176
> +                self._filesystem.join(os.environ['SYSTEMROOT'], "system32", "ntsd.exe")]

This is OK as-is. This path is identical in both the 64-bit and 32-bit cases. Instead of duplicating this path for both branches we could append it to possible_paths outside of this if-else block.

[Brent Fulgham]

Comment on attachment 262655 [details]
Patch

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

>> Tools/ChangeLog:29
>> +        (WinPort._build_path): Added properly binary target path to _build_path.
> 
> This sentence does not read well.

You're right! I'll fix that.

>> Tools/ChangeLog:33
>> +        ntsd debugger does not understand this argument.
> 
> Nit: ntsd => NTSD
> 
> (NTSD is an acronym for (Microsoft) NT Symbolic Debugger)

Ok -- I'll correct that.

>> Tools/Scripts/build-dumprendertree:64
>> +chdir File::Spec->catdir('Tools', 'DumpRenderTree') or die;
> 
> We use double quoted string literals in Perl code unless the literal contains a double quote or we explicitly do not want string interpolation to be performed.

OK. Will fix.

>> Tools/Scripts/webkitdirs.pm:622
>> +    my $binDir;
> 
> This is OK as-is. I don't see the need to abbreviate the word "directory" in the variable name "binDir". I know we use this abbreviation throughout the code, including in the name "productDir". Having said that, I don't feel we should continue such a naming convention.

I'll change these to 'directory', since these are all local values, and won't cause much of a renaming-ripple-effect.

>> Tools/Scripts/webkitpy/port/win.py:131
>> +            binPath = 'bin32'
> 
> Nit: In Python code variable names should use the words separated by underscores naming convention. The name of this variable is also inconsistent with the name of the analogous variable used in the webkitdirs::executableProductDir(), binDir. I suggest that we name this variable bin_directory to be consistent. Regardless, we should pick a name for the variable that represents the name of the bin directory and use in both the Perl and the Python code up to naming convention style for consistency.

OK. I actually changed it to 'binary_directory', since I used 'binaryDirectory' in the Perl change.

[Brent Fulgham]

Committed r190698: <http://trac.webkit.org/changeset/190698>