On 1/13/22 7:04 PM, Ivan Zhakov wrote:
> [[ sorry for delayed response ]]
>
> On Fri, 7 Jan 2022 at 17:33, Yann Ylavic <ylavic....@gmail.com> wrote:
>>
>> Hi Ivan,
>>
>> On Fri, Jan 7, 2022 at 2:50 PM Ivan Zhakov <i...@visualsvn.com> wrote:
>>>
>>> This change does not compile on Windows in APR 1.7.x:
>>> [[[
>>> file_io\win32\readwrite.c(325): error C2065: 'file': undeclared identifier
>>> file_io\win32\readwrite.c(325): error C2223: left of '->filehand' must
>>> point to struct/union
>>
>> I was missing backport of r1895178, does r1896808 compile now?
>> (Sorry, no Windows at hand..).
> Yes, it builds now. Thanks!
>
>>
>>>
>>> I also have a high-level objection against backporting this change to
>>> APR 1.7.x: IMHO APR 1.7.x is a stable branch and I think that only
>>> regression fixes should be backported to the stable branch. r1896510
>>> is a significant change and as far as I understand it's not a
>>> regression fix. So I think it would be better to revert r1896510 and
>>> release it as part of APR 2.0 (or 1.8.x).
>>
>> I think that most if not all of the changes to 1.7.x since 1.7.0 are
>> fixes for bugs that were there before 1.7 already, not regressions
>> introduced by 1.7.0.
>
> Agreed on the bugfix/regressions part. I have misunderstood that
> r1896510 is a bugfix, perhaps, due to its size, and was thinking that
> it adds new functionality. But even with that in mind, I still think
> that the size of the change might be just too large for it to be an
> appropriate fit for a patch release.
>
> Speaking of the change itself, I think that there might be an
> alternative to making the apr_file_t also handle sockets on Windows.
> It might be better to specifically change the pollset implementation
> so that on Windows it would add a socket and use it for wakeup,
> instead of using the socket disguised as a file.
>
> If this alternative approach sounds fine, I could try to implement it.
But this could wait for a 1.7.2, correct? I am asking because there is some
desire to get 1.7.1 out of the door soon.
And yes I would be happy with 1.7.2 that only adds this over 1.7.1 and is
released soon after 1.7.2.
Regards
Rüdiger
