Hi Bjorn,
Patch approved.
However, I agree with Andrei.
You need to improve your comments on the patch before pushing.
Bug #42216 mysqltest: Use of diff belonging to current OS, with wrong
option for Solaris
Windows systems require a special code to check the availability of the
diff tool. However,
the function that does this verification uses a "-v" option that is not
available in Solaris, thus
generating failures in such platform.
The fix only calls the function on windows and sets....
Andrei Elkin wrote:
> Bjorn, hello.
>
>
>> #At file:///home/bm136801/mysql/5.0-42216/
>>
>> 2732 Bjorn Munch 2009-01-27
>> Bug #42216 mysqltest: Use of diff belonging to current OS, with wrong
> option for Solaris
>> Call the check_diff() only if Windows, also fixed wrong flag
>> if diff failed
>>
>
> Could you please provide the problem description section/clause and
> the idea of your solution?
>
>
>> modified:
>> client/mysqltest.c
>>
>> === modified file 'client/mysqltest.c'
>> --- a/client/mysqltest.c 2009-01-09 18:30:55 +0000
>> +++ b/client/mysqltest.c 2009-01-27 14:13:39 +0000
>> @@ -1386,7 +1386,11 @@ void show_diff(DYNAMIC_STRING* ds,
>> needs special processing due to return values
>> on that OS
>> */
>>
>
>
>> +#ifdef __WIN__
>> have_diff = diff_check();
>> +#else
>> + have_diff = 1;
>> +#endif
>>
>
> I think you need to mention somewhere (e.g at top level comments) why
> diff_check() is necessary only on windows.
> How `diff' is supposed to be found on other platforms.
>
>
>>
>> if (have_diff)
>> {
>> @@ -1410,7 +1414,7 @@ void show_diff(DYNAMIC_STRING* ds,
>> "2>&1",
>> NULL) > 1) /* Most "diff" tools return >1 if error */
>> {
>> - have_diff= 1;
>> + have_diff= 0;
>> }
>> }
>> }
>>
>>
>
> cheers,
>
> Andrei
>
>
>
>
