Bug 30572

Summary: Unification of using null device in perl scripts
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ddkilzer, eric, mrowe
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 31840    
Attachments:
Description Flags
proposed patch
eric: review-
proposed patch darin: review+

Description Csaba Osztrogonác 2009-10-20 08:47:23 PDT
To use a really platform independent perl scripts we mustn't use /dev/null as null device.
In perl there is File::Spec->devnull() to determine platorm dependent null device.

I prefer using File::Spec->devnull() everywhere. But there are few 
place, where the shell command is in `...`. (VCSUtils.pm and bisect-builds)  
In these files we can use this method:

my $devNull = File::Spec->devnull();
`... $devNull ...`
Comment 1 Csaba Osztrogonác 2009-10-20 08:54:40 PDT
Created attachment 41507 [details]
proposed patch
Comment 2 Eric Seidel (no email) 2009-10-21 10:37:38 PDT
Comment on attachment 41507 [details]
proposed patch

I believe our style is to put spaces around '.' operators.

Also, I'm surprised that sometimes you use local variables to hold $devNull and sometimes you make direct calls to File::Spec->devnull().

Is devnull() always a string path?
Comment 3 Csaba Osztrogonác 2009-10-22 14:22:57 PDT
(In reply to comment #2)
> I believe our style is to put spaces around '.' operators.
Yes, you're right, sorry. I will correct it.
 
> Also, I'm surprised that sometimes you use local variables to hold $devNull
> and sometimes you make direct calls to File::Spec->devnull().
I told I prefer using File::Spec->devnull() everywhere, but I have no idea how could I use it directly in `...`. Accordingly I use local variable same as in svn-create-patch originally. Have you got better idea to solve this problem?
 
> Is devnull() always a string path?
You can find devNull() here: http://perldoc.perl.org/File/Spec.html
"Returns a string representation of the null device."
It works correctly, we used it previosly in WebKit scripts.
Comment 4 Csaba Osztrogonác 2009-10-22 14:23:45 PDT
(In reply to comment #2)
> I believe our style is to put spaces around '.' operators.
Yes, you're right, sorry. I will correct it.
 
> Also, I'm surprised that sometimes you use local variables to hold $devNull
> and sometimes you make direct calls to File::Spec->devnull().
I told I prefer using File::Spec->devnull() everywhere, but I have no idea how could I use it directly in `...`. Accordingly I use local variable same as in svn-create-patch originally. Have you got better idea to solve this problem?
 
> Is devnull() always a string path?
You can find devNull() here: http://perldoc.perl.org/File/Spec.html
"Returns a string representation of the null device."
It works correctly, we used it previosly in WebKit scripts.
Comment 5 Csaba Osztrogonác 2009-10-26 10:40:32 PDT
(In reply to comment #2)
> (From update of attachment 41507 [details])
> I believe our style is to put spaces around '.' operators.
Updated.

> Also, I'm surprised that sometimes you use local variables to hold $devNull and
> sometimes you make direct calls to File::Spec->devnull().
I modified bisect-builds, we can use exec instead of backstick operator.

But I have no idea what should we do with backstick of svn-create-patch:
    print `svn cat ${sourceFile} | diff -u $devNull - | tail -n +3`;

We can't use system or exec insted of backsticks. 
I think we shouldn't rewrite this code at any price.
Comment 6 Csaba Osztrogonác 2009-10-26 10:41:42 PDT
Created attachment 41876 [details]
proposed patch
Comment 7 Eric Seidel (no email) 2009-10-26 10:45:57 PDT
CCing perl folks, as I am not one.
Comment 8 Csaba Osztrogonác 2009-10-26 11:58:17 PDT
Thx for review.

Sending        WebKitTools/ChangeLog
Sending        WebKitTools/Scripts/VCSUtils.pm
Sending        WebKitTools/Scripts/bisect-builds
Sending        WebKitTools/Scripts/resolve-ChangeLogs
Sending        WebKitTools/Scripts/run-iexploder-tests
Sending        WebKitTools/Scripts/run-jsc
Sending        WebKitTools/Scripts/run-mangleme-tests
Sending        WebKitTools/Scripts/run-webkit-tests
Sending        WebKitTools/Scripts/webkitdirs.pm
Transmitting file data .........
Committed revision 50080.
Comment 9 David Kilzer (:ddkilzer) 2009-11-24 10:51:09 PST
This broke the bisect-builds script.  See Bug 31840.