Bug 30572 - Unification of using null device in perl scripts
Summary: Unification of using null device in perl scripts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 31840
  Show dependency treegraph
 
Reported: 2009-10-20 08:47 PDT by Csaba Osztrogonác
Modified: 2009-11-24 10:51 PST (History)
4 users (show)

See Also:


Attachments
proposed patch (7.71 KB, patch)
2009-10-20 08:54 PDT, Csaba Osztrogonác
eric: review-
Details | Formatted Diff | Diff
proposed patch (7.62 KB, patch)
2009-10-26 10:41 PDT, Csaba Osztrogonác
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.