[xen-tools-dev] [PATCH v2 00/13] Next bunch of patches

[Stéphane Jourdois]

Le 23/07/2010 14:58, Axel Beckert a écrit :
> On Sun, Jul 18, 2010 at 10:04:10PM +0200, Stéphane Jourdois wrote:
>> Second version of my bunch of patch.
>
> Applied by rebasing.

Thanks ! git pull was not enough ?

> Some comments though:
>
> On Sun, Jul 18, 2010 at 10:04:17PM +0200, Stéphane Jourdois wrote:
>> Read and test only the shebang, not the whole file, +typos.
>
> Basically a good idea.
>
>> diff --git a/t/shell-syntax.t b/t/shell-syntax.t
>> index a53a228..836eb45 100755
>> --- a/t/shell-syntax.t
>> +++ b/t/shell-syntax.t
>> @@ -34,26 +34,28 @@ sub checkFile
> [...]
>>       # See if it is a shell script.
>>       my $isShell = 0;
>>
>> -    # Read the file.
>> +    # Read the shebang
>>       open( INPUT, "<", $file );
>> -    foreach my $line (<INPUT>  )
>> +    my $line =<INPUT>;
>> +    close( INPUT );
>> +
>> +    # Check if it is really a shell file
>> +    if ( $line =~ /^#! ?\/bin\/(ba)?sh/ )
>>       {
>> -        if ( ( $line =~ /\/bin\/sh/ ) ||
>> -             ( $line =~ /\/bin\/bash/ ) )
>> -        {
>> -            $isShell = 1;
>> -        }
>> +        $isShell = 1;
>>       }
>> -    close( INPUT );
>
> I changed that to only test for /bin/sh -- I don't want to see an
> /bin/bash shebang line without reason in the xen-tools code. :-)

That's also a good idea indeed.
Now we have to exterminate bashisms :-)

> And there was nearly none, except in some code to generate some
> policy.d scripts. A grep over all files found that, not this test.
>
> I'm thinking about how a test would have to look like to catch code
> which generates shell scripts.

Hard one. would have to catch every output to disk, run the script, and 
intercept bash shebangs. That would be doable I think.
A simple grep '/bin/bash' -r . as you did should be more than enough :)


> On Sun, Jul 18, 2010 at 10:04:16PM +0200, Stéphane Jourdois wrote:
>
>> diff --git a/t/no-tabs.t b/t/no-tabs.t
>> index bcc881d..2037fbc 100755
>> --- a/t/no-tabs.t
>> +++ b/t/no-tabs.t
>> @@ -38,6 +38,9 @@ sub checkFile
>>       # Nor about Makefiles
>>       return if ( $file =~ /\/Makefile$/ );
>>
>> +    # Nor about temporary files
>                     ^^^^^^^^^
>> +    return if ( $file =~ m{/\.[^/]+$} );
>> +
>>       # Nor about files which start with ./debian/
>>       return if ( $file =~ /^\.\/debian\// );
>>
>
> I'd not call dot files "temporary" files. Pushed a fix just now.

I wonder what was in my head :)


> On Sun, Jul 18, 2010 at 10:04:20PM +0200, Stéphane Jourdois wrote:
>> diff --git a/TODO b/TODO
>> index 10e7d0a..b20199d 100644
>> --- a/TODO
>> +++ b/TODO
>> @@ -18,13 +18,6 @@ Minor bugs to fix and features to add before a 4.2 release
>>      stored in the configuration hash.  The only issue is that trailing
>>      whitespace is missing from the "make_fs_foo" option.
>>
>> -* Test suite should pass
>> -
>> -   Currently failing:
>> -
>> -   t/pod-coverage.t (fails on Xen::Tools and Xen::Tools::Log despite
>> -   they do have POD. Strange.)
>> -
>>   * Setup locales in the hooks?
>>
>>      Currently no locales are set and this causes several domU errors
>
> Yay! Now all tests pass for me, too! Thanks a lot! :-)

You're very welcome.

On that subject, I added the following in my .git/hooks/pre-commit :
#!/bin/sh
exec make test

Each time I do a git commit, tests are run before accepting the commit.
If one day I want to commit without tests running, I can always use git 
commit --no-verify.

That is not something that you can add in the repository, each developer 
has to add this in his clone.


/me going to prepare the next bunch of patches :-)
Thanks very much for your pull and comments.

Stéphane Jourdois.

-- 
  ///  Stephane Jourdois     /"\  ASCII RIBBON CAMPAIGN \\\
(((    Consultant securite  \ /    AGAINST HTML MAIL    )))
  \\\   157 Bd Davout         X                         ///
   \\\  75020  Paris         / \    +33 6 8643 3085    ///