llvm dev - Jul 2008 - [LLVMdev] [PATCH] patch to compile llvm-gcc using nightly tester script(NewNightlyTester.pl)

On Mon, Jul 7, 2008 at 11:10 AM, Rajika Kumarasiri <rajikacc at gmail.com>
wrote:
> hello everybody,
>
> I have added few improvements to my patch. Please review the new patch
> directly.
> Thanks!
>
Hi Rajika,

A few stylistic comments. I'll let others comment on the algorithm for now:

     if ($VERBOSE) {
       print "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ; "
.
-            "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
+            "$SVNCMD/test-suite/trunk llvm-test ; cd ../../;" .
+	    "$SVNCMD/llvm-gcc-4.2/trunk dst-directory ) > $COLog
2>&1\n";
     }
     system "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ; " .
-          "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
+          "$SVNCMD/test-suite/trunk llvm-test ; cd ../../;" .
+	  "$SVNCMD/llvm-gcc-4.2/trunk dst-directory ) > $COLog
2>&1\n";

This could be done with something like this:

     my $cmd = "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ;
" .
              "$SVNCMD/llvm-gcc-4.2/trunk dst-directory; " .
              "cd llvm/projects; " .
              "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";

     print $cmd if ($VERBOSE);
     system $cmd;

This could be done in other spots that do similar things as well.
Also, why not name the LLVM-GCC source directory something other than
"dst-directory"? Maybe "llvm-gcc.src" or similar? You add a
few
extraneous newlines in the code. Please remove those.

-    my $CVSCMD = "$NICE cvs $CVSOPT -d $CVSRootDir co -P $CVSCOOPT";
+    my $CVSCMD = "$NICE cvs $CVSOPT -d $CVSRootDir co -P $CVSCOOPT";
# TODO, do we still maintain the CVS tree ?

80-column violation here and in other places. Put the comment before
here. (And, no, we don't maintain the CVS tree anymore.)

-bw

hi Bill,
Thanks for the comments. I'll update the patch according to that.

-Rajika

On Mon, Jul 7, 2008 at 11:56 PM, Bill Wendling <isanbard at gmail.com>
wrote:
> On Mon, Jul 7, 2008 at 11:10 AM, Rajika Kumarasiri <rajikacc at
gmail.com>
> wrote:
> > hello everybody,
> >
> > I have added few improvements to my patch. Please review the new patch
> > directly.
> > Thanks!
> >
> Hi Rajika,
>
> A few stylistic comments. I'll let others comment on the algorithm for
now:
>
>     if ($VERBOSE) {
>       print "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ;
" .
> -            "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
> +            "$SVNCMD/test-suite/trunk llvm-test ; cd ../../;" .
> +           "$SVNCMD/llvm-gcc-4.2/trunk dst-directory ) > $COLog
2>&1\n";
>     }
>     system "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ;
" .
> -          "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
> +          "$SVNCMD/test-suite/trunk llvm-test ; cd ../../;" .
> +         "$SVNCMD/llvm-gcc-4.2/trunk dst-directory ) > $COLog
2>&1\n";
>
> This could be done with something like this:
>
>     my $cmd = "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ;
" .
>              "$SVNCMD/llvm-gcc-4.2/trunk dst-directory; " .
>              "cd llvm/projects; " .
>              "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
>
>     print $cmd if ($VERBOSE);
>     system $cmd;
>
> This could be done in other spots that do similar things as well.
> Also, why not name the LLVM-GCC source directory something other than
> "dst-directory"? Maybe "llvm-gcc.src" or similar? You
add a few
> extraneous newlines in the code. Please remove those.
>
> -    my $CVSCMD = "$NICE cvs $CVSOPT -d $CVSRootDir co -P
$CVSCOOPT";
> +    my $CVSCMD = "$NICE cvs $CVSOPT -d $CVSRootDir co -P
$CVSCOOPT";
> # TODO, do we still maintain the CVS tree ?
>
> 80-column violation here and in other places. Put the comment before
> here. (And, no, we don't maintain the CVS tree anymore.)
>
> -bw
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>


-- 
comp.lang.c - http://groups.google.com/group/comp.lang.c/topics
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20080708/ca8282ca/attachment.html>

Thanks :-)

One last thing. It seems like the patch has a lot of whitespace
changes. Could you create the diff with the -b option?

-bw

On Mon, Jul 7, 2008 at 11:49 AM, Rajika Kumarasiri <rajikacc at gmail.com>
wrote:
> hi Bill,
> Thanks for the comments. I'll update the patch according to that.
>
> -Rajika
>
> On Mon, Jul 7, 2008 at 11:56 PM, Bill Wendling <isanbard at
gmail.com> wrote:
>>
>> On Mon, Jul 7, 2008 at 11:10 AM, Rajika Kumarasiri <rajikacc at
gmail.com>
>> wrote:
>> > hello everybody,
>> >
>> > I have added few improvements to my patch. Please review the new
patch
>> > directly.
>> > Thanks!
>> >
>> Hi Rajika,
>>
>> A few stylistic comments. I'll let others comment on the algorithm
for
>> now:
>>
>>     if ($VERBOSE) {
>>       print "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ;
" .
>> -            "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
>> +            "$SVNCMD/test-suite/trunk llvm-test ; cd
../../;" .
>> +           "$SVNCMD/llvm-gcc-4.2/trunk dst-directory ) >
$COLog 2>&1\n";
>>     }
>>     system "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects ;
" .
>> -          "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
>> +          "$SVNCMD/test-suite/trunk llvm-test ; cd ../../;"
.
>> +         "$SVNCMD/llvm-gcc-4.2/trunk dst-directory ) > $COLog
2>&1\n";
>>
>> This could be done with something like this:
>>
>>     my $cmd = "( time -p $SVNCMD/llvm/trunk llvm; cd llvm/projects
; " .
>>              "$SVNCMD/llvm-gcc-4.2/trunk dst-directory; " .
>>              "cd llvm/projects; " .
>>              "$SVNCMD/test-suite/trunk llvm-test ) > $COLog
2>&1\n";
>>
>>     print $cmd if ($VERBOSE);
>>     system $cmd;
>>
>> This could be done in other spots that do similar things as well.
>> Also, why not name the LLVM-GCC source directory something other than
>> "dst-directory"? Maybe "llvm-gcc.src" or similar?
You add a few
>> extraneous newlines in the code. Please remove those.
>>
>> -    my $CVSCMD = "$NICE cvs $CVSOPT -d $CVSRootDir co -P
$CVSCOOPT";
>> +    my $CVSCMD = "$NICE cvs $CVSOPT -d $CVSRootDir co -P
$CVSCOOPT";
>> # TODO, do we still maintain the CVS tree ?
>>
>> 80-column violation here and in other places. Put the comment before
>> here. (And, no, we don't maintain the CVS tree anymore.)
>>
>> -bw
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>
> --
> comp.lang.c - http://groups.google.com/group/comp.lang.c/topics
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>