Re: [boost] "peer reviewed" - Rights and responsibilities of maintainers

Subject: Re: [boost] "peer reviewed" - Rights and responsibilities of maintainers
From: Robert Ramey (ramey_at_[hidden])
Date: 2018-10-16 17:12:09

On 10/16/18 8:49 AM, Mike Dev via Boost wrote:
>> -----Original Message-----
>> From: Boost <boost-bounces_at_[hidden]> On Behalf Of Robert Ramey via Boost
>> Sent: Tuesday, October 16, 2018 5:22 PM
>>
>> On 10/16/18 8:11 AM, Peter Dimov via Boost wrote:
>>> Robert Ramey wrote:
>>>> On 10/16/18 7:23 AM, Peter Dimov via Boost wrote:
>>>>
>>>>> https://github.com/boostorg/serialization/pull/111/files
>>>>>
>>>>> which does indeed crash
>>>>>
>>>>> https://travis-ci.org/boostorg/serialization/jobs/441654483#L2546
>>>>>
>>>>> That's easy enough to follow and does show a legitimate problem, in
>>>> my > opinion, although Robert refuses to acknowledge it for some reason.
>>>>
>>>> The test does not show anything at all. It's ill conceived.
>>>
>>> What is ill-conceived about it? It's a minimal example of two shared
>>> libraries each using Serialization. As Alexander says, it's a simplified
>>> version of a crash they had in one of their projects.
>>
>> First of all, the test changes every time I look at it.  It's hard to
>> follow.
> 
> If you keep saying that the test is bad / irrelevant, it is no wonder
> the author changes the code to improve it.

If the test is bad/irrelevant, what else should I say.  I have no 
problem if the author want's to improve his test.  It's just not happening.

> 
>>
>> On the last version I looked at - since no there is not
>> BOOST_CLASS_EXPORT(class name) invoked, there are no indexed entries in
>> the extended type info tables so get_key() should return null under any
>> circumstances.
> 
> If I understand this correctly, the purpose of the test is to show that the
> application crashes. In that case, the return value is completely irrelevant.

I haven't seen the application so I don't know why it crashes.  I'm 
guessing that it might be related to the current (and recently improved 
state) singleton.  I DO know test_exported_dll also crashes on one 
compiler configuration. I would like to fix this but so far no one has 
been able to provide a convincing change nor a test which points out 
where the problem might be.
> 
> If you think that crash is not the fault of Boost.Serialization (e.g. because
> it is used wrongly), then it would be helpful to explain what you believe to
> be the actual reason for the crash.

Which crash: in his app, in test_exported_dll, or in his test?

> 
> What I don't know: Is there a travis build that shows that the test from Flamefire
> (https://github.com/boostorg/serialization/pull/111/files ) still fails
> with the changes introduced by Robert (I'm not sure when they were introduced)?

Right - that are the exact same results which result from Alexander's 
original patch.  That is, I merged in a simpler, less intrusive, less 
fragile version of Alexander's patch.  And it "fixes" everything that 
Alexander's patch fixed.  Clearly there are still some problems left - 
which are likely to be even more difficult to nail down.  Note that to 
get this far has required 9 months of effort - part of which entailed 
including a breaking patch in to the release.

Under ordinary circumstances, I would just fix Alexander's test and be 
done with it just to save time.  But I've come to understand that it 
won't actually save time so I haven't done so.

Robert Ramey

> 
> Mike
> 
> 
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>