Re:[OT] [RELEASE CANDIDATE] mod_perl-2.0.11 RC2

Guys,

Not being able myself to contribute really to this process, I did not comment on the 
previous emails here.
But as a grateful user of mod_perl for many years, I just want to express my immense 
gratitude to Steve and all other people who contribute to keep this project and product 
alive and well, and let them know that their efforts are not in vain.
The activity of my little software and services company in Germany, Spain and worldwide, 
depends vitally on Apache httpd, perl and mod_perl. And I expect that I am not alone in 
such a situation.

Thanks !
André


On 26.09.2019 10:02, Steve Hay wrote:
> On Thu, 26 Sep 2019 at 06:54, Damyan Ivanov <[email protected]> wrote:
>>
>> -=| Damyan Ivanov, 25.09.2019 23:06:56 +0300 |=-
>>> -=| Steve Hay, 25.09.2019 08:34:07 +0100 |=-
>>>> What happens if you revert the change in that file? I.e. Change
>>>> FINFO_NAME back to FINFO_NORM on line 168, and on line 18 (the
>>>> APR::Const -compile line)? Does that fix it for you?
>>>
>>> With these two lines reverted, all tests pass for me.
>>
>> Out of interest I also tried using "FINFO_NAME | FINFO_NORM" on line
>> 168 and the test failed with the same error. It appears that
>> FINFO_NAME triggers it.
>>
>> Tracing this in APR, the error seems to come from
>> file_io/unix/filestat.c¹ where the valid flags on line 73 don't
>> include APR_FINFO_NAME. This kind of makes sense, because the stat(2)
>> struct lacks information about the file name.
>>
>>   ¹ https://sources.debian.org/src/apr/1.6.5-1/file_io/unix/filestat.c/
>>
>> -- Damyan
>
> Good catch! That explains why using FINFO_NAME is failing for you but
> working for me (on Windows): apr/file_io/win32/filestat.c *does* have
> support for FINFO_NAME in it, filling in finfo->name from filename if
> it succeeded in getting filename (using calls other than stat()!).
>
> The point of the request_rec test in question is only to test a single
> field anyway (the rest are tested in TestAPR::finfo, as the comment
> says) so I will simply change it to test a different field. Evidently
> FINFO_NAME was a poor choice!
>
> Before I actually roll out an RC3, please can you confirm that
> switching to FINFO_SIZE as per this patch works for you (it does for
> me):
>
> Index: t/response/TestAPI/request_rec.pm
> ===================================================================
> --- t/response/TestAPI/request_rec.pm (revision 1866274)
> +++ t/response/TestAPI/request_rec.pm (working copy)
> @@ -15,7 +15,7 @@
>   use APR::Pool ();
>
>   use Apache2::Const -compile => qw(OK M_GET M_PUT);
> -use APR::Const    -compile => qw(FINFO_NAME);
> +use APR::Const    -compile => qw(FINFO_SIZE);
>
>   #this test module is only for testing fields in the request_rec
>   #listed in apache_structures.map
> @@ -165,12 +165,13 @@
>
>       # finfo
>       {
> -        my $finfo = APR::Finfo::stat(__FILE__,
> APR::Const::FINFO_NAME, $r->pool);
> +        my $size = (stat __FILE__)[7];
> +        my $finfo = APR::Finfo::stat(__FILE__,
> APR::Const::FINFO_SIZE, $r->pool);
>           $r->finfo($finfo);
>           # just one field test, all accessors are fully tested in
>           # TestAPR::finfo
> -        ok t_cmp($r->finfo->fname,
> -                 __FILE__,
> +        ok t_cmp($r->finfo->size,
> +                 $size,
>                    '$r->finfo');
>       }
>