Bug 61890 – make-hash-tools.pl: Perl 5.14 compatibility

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Evangelos Foutras]

Created attachment 95682 [details]
Patch for make-hash-tools.pl to use given/when instead of switch

The make-hash-tools.pl script, executed during the build process, uses the Switch module which has been removed from Perl 5.14.

In Arch Linux, we decided to patch make-hash-tools.pl to use the new given/when keywords available in Perl >= 5.10. I have attached our patch, which is quite trivial. However, you might want to retain compatibility with oder Perl versions, in which case a different patch will be needed (my Perl-fu is weak, and can't make a conditional dependency on either the Switch module or the new given/when syntax).

Other reports of this issue:

Gentoo: http://bugs.gentoo.org/show_bug.cgi?id=364489
FreeBSD: http://www.mail-archive.com/[e-mail]/msg33878.html

[Andras Becsi]

(In reply to comment #0)
> Created an attachment (id=95682) [details]
> Patch for make-hash-tools.pl to use given/when instead of switch
> 
> The make-hash-tools.pl script, executed during the build process, uses the Switch module which has been removed from Perl 5.14.
> 
> In Arch Linux, we decided to patch make-hash-tools.pl to use the new given/when keywords available in Perl >= 5.10. I have attached our patch, which is quite trivial. However, you might want to retain compatibility with oder Perl versions, in which case a different patch will be needed (my Perl-fu is weak, and can't make a conditional dependency on either the Switch module or the new given/when syntax).
> 
> Other reports of this issue:
> 
> Gentoo: http://bugs.gentoo.org/show_bug.cgi?id=364489
> FreeBSD: http://www.mail-archive.com/[e-mail]/msg33878.html

Because this is a generator script where performance is not an issue since, I think these could be simply changed to if/elif constructions, so we wouldn't need to worry about compatibility.

[Csaba Osztrogonác]

I agree with Andras, given/when can make <5.10 perl users unhappy, 
so if/elsif would be better here.

[Andras Becsi]

Created attachment 97788 [details]
proposed patch

In addition to the previous comments, during the changes on the generation process only two options remained in this script so switch/case is overkill anyhow.

[Andras Becsi]

Created attachment 97789 [details]
proposed patch with newline

[Csaba Osztrogonác]

Comment on attachment 97789 [details]
proposed patch with newline

LGTM, r=me

[WebKit Review Bot]

Comment on attachment 97789 [details]
proposed patch with newline

Rejecting attachment 97789 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1

Last 500 characters of output:
cripts/webkitpy/common/system/executive.py", line 382, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 438, in popen
    return subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

Full output: http://queues.webkit.org/results/8911496

[Csaba Osztrogonác]

Comment on attachment 97789 [details]
proposed patch with newline

One more attempt to cq+

[WebKit Review Bot]

Comment on attachment 97789 [details]
proposed patch with newline

Rejecting attachment 97789 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 1

Last 500 characters of output:
cripts/webkitpy/common/system/executive.py", line 382, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 438, in popen
    return subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

Full output: http://queues.webkit.org/results/8907579

[Csaba Osztrogonác]

Eric, have you got any idea why CQ rejects this patch?

[Eric Seidel (no email)]

No, but we saw one other such rejection the other day.  Maybe one of the bots got sick.

[Andras Becsi]

Comment on attachment 97789 [details]
proposed patch with newline

Let's have another try.

[WebKit Review Bot]

Comment on attachment 97789 [details]
proposed patch with newline

Rejecting attachment 97789 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1

Last 500 characters of output:
cripts/webkitpy/common/system/executive.py", line 382, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 438, in popen
    return subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

Full output: http://queues.webkit.org/results/8885010

[Andras Becsi]

Comment on attachment 97789 [details]
proposed patch with newline

Clearing flags on attachment: 97789

Committed r89265: <http://trac.webkit.org/changeset/89265>

[Andras Becsi]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

I think our handling of Ossy's name is broken again.