On 29.12.2010 20:50, Peter Samuelson wrote:
> [Branko Cibej]
>> Found it, r843793 and I agree with the change. So, effectively,
>> svn_error__locate has been public since then, and r1053469 should be
>> reverted anyway because it breaks the ABI.
> It doesn't break the ABI. I kept the function itself.
Sorry, I missed that in the diff.
> Whether it is
> useful to populate ->file and ->line, shipping potentially a huge
> number of constant strings and integers in our binaries that will never
> actually be seen, is the question. Essentially, we are reimplementing
> 'gcc -g' in a less useful, but impossible to omit or strip out, way.
> "Impossible to omit or strip out" is, I guess, the selling point. But
> I think I'd rather just see '-g' added to CFLAGS by default.
-g isn't exactly the same thing, certainly not on compilers that cannot
generate optimized code whilst generating useful debug info. I see no
reason to put those macros inside an ifdef at this point.
>> The name may or may not be misleading, depending on interpretation
>> ... no-one should call svn_error__locate directly, the public API
>> are effectively the macro wrappers.
> Fine, svn_error__locate is not part of the public API, but it is very
> much part of the ABI. I think it's annoying that we have this __
> function that we're not allowed to remove, or change the calling
> signature. Certainly it's not something we can fix _now_, but we
> should definitely avoid doing more of this in the future.
>
> How I noticed: in the Debian builds, I ship a semi-autogenerated list
> of public symbols and the version of libsvn in which they first
> appeared. This is for automatic dependency generation when building
> other packages; most libraries with backward-compatible ABIs in Debian
> do this, and it works very well. In that list I grep out all the
> svn_*__ symbols, as nobody should be using those. Thus we discovered
> that certain symbols like svn_error__locate do appear in binaries,
> through no fault of the third-party developers
We're allowed to make exceptions. This particular exception has been in
the code since before 1.0 was released (and before our versioning and
naming and API revving rules were formalized). There's really no good
reason to go changing it now.
So we have an inconsistency in our A[PB]I. To which I respond, Emerson!
-- Brane
Received on 2010-12-29 21:21:04 CET