On 13.10.2016 13:28, Johan Corveleyn wrote:
> On Thu, Oct 13, 2016 at 1:22 PM, Ivan Zhakov <i...@apache.org> wrote:
>> On 12 October 2016 at 22:30, Johan Corveleyn <jcor...@gmail.com> wrote:
>>> On Wed, Oct 12, 2016 at 10:13 PM, <i...@apache.org> wrote:
>>>> Author: ivan
>>>> Date: Wed Oct 12 20:13:24 2016
>>>> New Revision: 1764536
>>>> URL: http://svn.apache.org/viewvc?rev=1764536&view=rev
>>>> * STATUS: Veto r1759116 group.
>>>> @@ -135,6 +119,24 @@ Candidate changes:
>>>> Veto-blocked changes:
>>>> + * r1759116, r1759117, r1759122, r1759123, r1759124
>>>> + Fix FSFS format 7 packing for revisions packs with lots of changes.
>>>> + Justification:
>>>> + Problem occurred in at least two user repositories. Without the fix,
>>>> + format 7 repositories with an exceptionally large number of changes
>>>> + a pack cannot be packed - which renders using f7 pointless for those
>>>> + users.
>>>> + Branch:
>>>> + ^/subversion/branches/1.9.x-fsfs-pack-fixes
>>>> + Notes:
>>>> + r1759116 adds a workaround for trunc() corrupting apr_file_t
>>>> internal state.
>>>> + r1759117-23 provide the actual fixes.
>>>> + r1759124 adds a regression test with the necessary internal API
>>>> + Votes:
>>>> + +1: stefan2
>>>> + -1: ivan (apr_file_trunc() bug should be reported and fixed in APR,
>>>> + not by unreliable workaround with unknown consequences)
>>> So that leaves you claiming that it's an "unreliable workaround with
>>> unknown consequences". Why? Please clarify. We are talking about a
>>> bugfix which fixes an important problem reported by users. Do you have
>>> any suggestions for improvement? Perhaps the fix should be technically
>>> discussed on the mailing list? Or should we just leave this important
>>> problem unfixed?
>> I think it's almost always better to solve the problem in its origin.
>> Any workaround is unreliable, so we should always try to fix the
>> problem at the root before trying the workarounds. Note that we are
>> talking about a relatively obvious bug in other Apache's project, not
>> about something irrecoverable.
>> As far as I know, this problem has never been reported to the APR's
>> dev list. And this is the first action that we should have done.
>>> Do you have any suggestions for improvement?
>> I have plans to send a patch to APR in order to solve this problem. So
>> we don't need to release this workaround.
> Okay, I understand this should be fixed in APR.
> But should we block a workaround in our code for a bug that causes
> problems in the wild for existing svn 1.9 installations? Those users
> deserve a fix in a 1.9.x patch release, IMHO. If you can arrange this
> by fixing it in APR, creating a patch release of APR, and then
> creating a 1.9.x svn release with that fix (in a reasonable
> timeframe), then fine. But otherwise I think a svn-internal-workaround
> is warranted.
There's no chance of doing this in a reasonable time-frame because we
require APR-1.3 and no fix we can possibly come up with will be released
in a 1.3 patch.
We have any number of workarounds for APR quirks in the code. I can't
think of a single valid reason to veto another one, especially since
we'd /still/ have to have the workaround in our code for people who use
older APR versions (likely versions prior to 1.5 or even 1.6, depending
on what the fix ends up looking like).
"Any workaround is unreliable" is just nonsense, IMHO.