You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ponomarenko Andrey <an...@yandex.ru> on 2016/06/25 05:45:25 UTC

ABI changes analysis

Hello,

I'm working on a new project for backward compatibility analysis of the Linux ABIs. The report for Subversion base libraries has been recently added to the project: http://abi-laboratory.pro/tracker/timeline/subversion/

The report is generated with the help of the abi-compliance-checker, abi-dumper and abi-tracker tools: https://github.com/lvc/abi-tracker

Hope this will help users, maintainers and developers of libraries to maintain backward compatibility.

Thank you.

Re: ABI changes analysis

Posted by Greg Stein <gs...@gmail.com>.
This is freakin' HOT. Very nice work!

I've reviewed the reports, and it looks like we've maintained all our ABI
guarantees. The changes are what I would expect.

Thank you for this!

Cheers,
-g


On Sat, Jun 25, 2016 at 12:45 AM, Ponomarenko Andrey <
andrewponomarenko@yandex.ru> wrote:

> Hello,
>
> I'm working on a new project for backward compatibility analysis of the
> Linux ABIs. The report for Subversion base libraries has been recently
> added to the project:
> http://abi-laboratory.pro/tracker/timeline/subversion/
>
> The report is generated with the help of the abi-compliance-checker,
> abi-dumper and abi-tracker tools: https://github.com/lvc/abi-tracker
>
> Hope this will help users, maintainers and developers of libraries to
> maintain backward compatibility.
>
> Thank you.
>

Re: ABI changes analysis

Posted by Greg Stein <gs...@gmail.com>.
This is freakin' HOT. Very nice work!

I've reviewed the reports, and it looks like we've maintained all our ABI
guarantees. The changes are what I would expect.

Thank you for this!

Cheers,
-g


On Sat, Jun 25, 2016 at 12:45 AM, Ponomarenko Andrey <
andrewponomarenko@yandex.ru> wrote:

> Hello,
>
> I'm working on a new project for backward compatibility analysis of the
> Linux ABIs. The report for Subversion base libraries has been recently
> added to the project:
> http://abi-laboratory.pro/tracker/timeline/subversion/
>
> The report is generated with the help of the abi-compliance-checker,
> abi-dumper and abi-tracker tools: https://github.com/lvc/abi-tracker
>
> Hope this will help users, maintainers and developers of libraries to
> maintain backward compatibility.
>
> Thank you.
>

Re: ABI changes analysis

Posted by Stefan <lu...@posteo.de>.
On 6/27/2016 03:43, Daniel Shahaf wrote:
> Stefan wrote on Mon, Jun 27, 2016 at 02:49:51 +0200:
>>> On Sun, Jun 26, 2016 at 09:26:27PM +0200, Stefan wrote:
>>>> I'm just wondering why the backward compatibility for 1.9.0 (and 1.8.0)
>>>> doesn't state 100% here [1].
>>>>
>>>> Checking out the details on 1.9.0 [2] and there the details on
>>>> libsvn_subr [3] suggests 3 functions were removed:
>>>> - svn__apr_hash_index_key ( apr_hash_index_key( apr_hash_index_t const* hi )
>>>> - svn__apr_hash_index_klen ( apr_hash_index_key( apr_hash_index_t const*
>>>> hi )
>>>> - svn__apr_hash_index_val ( apr_hash_index_key( apr_hash_index_t const* hi )
> …
>> And now I also remember and realize that these removed symbols were
>> actually private ones never intended to be exported (aka: double _ in
>> the name). So 1.8/1.9 corrected this and ABI compatibility for these
>> were intentionally broken.
> I think these names were intentionally exported via
> subversion/include/private/*.h by libsvn_subr for libsvn_*'s use, and
> were removed when we bumped the minimum APR version to one that has
> these functions natively (apr_hash_this_key() and friends).
>
> I.e., the minimum APR supported by 1.8 doesn't have
> apr_hash_index_key(), the minimum APR supported by 1.9 does have that
> function, so 1.9 code uses the apr_* function directly without an
> svn__* wrapper.
Ok, then my memory completely mislead me. Thanks for the correction.

Regards,
Stefan




Re: ABI changes analysis

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan wrote on Mon, Jun 27, 2016 at 02:49:51 +0200:
> > On Sun, Jun 26, 2016 at 09:26:27PM +0200, Stefan wrote:
> >> I'm just wondering why the backward compatibility for 1.9.0 (and 1.8.0)
> >> doesn't state 100% here [1].
> >>
> >> Checking out the details on 1.9.0 [2] and there the details on
> >> libsvn_subr [3] suggests 3 functions were removed:
> >> - svn__apr_hash_index_key ( apr_hash_index_key( apr_hash_index_t const* hi )
> >> - svn__apr_hash_index_klen ( apr_hash_index_key( apr_hash_index_t const*
> >> hi )
> >> - svn__apr_hash_index_val ( apr_hash_index_key( apr_hash_index_t const* hi )
\u2026
> And now I also remember and realize that these removed symbols were
> actually private ones never intended to be exported (aka: double _ in
> the name). So 1.8/1.9 corrected this and ABI compatibility for these
> were intentionally broken.

I think these names were intentionally exported via
subversion/include/private/*.h by libsvn_subr for libsvn_*'s use, and
were removed when we bumped the minimum APR version to one that has
these functions natively (apr_hash_this_key() and friends).

I.e., the minimum APR supported by 1.8 doesn't have
apr_hash_index_key(), the minimum APR supported by 1.9 does have that
function, so 1.9 code uses the apr_* function directly without an
svn__* wrapper.

Cheers,

Daniel

Re: ABI changes analysis

Posted by Stefan Hett <st...@egosoft.com>.
On 6/27/2016 1:57 PM, Greg Stein wrote:
>
>
> On Mon, Jun 27, 2016 at 6:54 AM, Greg Stein <gstein@gmail.com 
> <ma...@gmail.com>> wrote:
>
>
>     On Sun, Jun 26, 2016 at 7:49 PM, Stefan <luke1410@posteo.de
>     <ma...@posteo.de>> wrote:
>     >...
>
>         And now I also remember and realize that these removed symbols
>         were
>         actually private ones never intended to be exported (aka:
>         double _ in
>         the name).
>
>
> To be clear: they WERE intended to be exported, so other svn libraries 
> could use them. We have hundreds of such functions.
>
Yeah I must have misremembered something here. I also didn't want to 
suggest that this would violate the ABI compatibility (as you state they 
were private symbols and therefore not covered by the ABI guarantee). I 
just misremembered the reasoning why they were added and then removed 
(guess I just remember some discussion related to the clean up of 
symbols prior to the 1.7 or 1.8 release which is completely independent 
from these symbols).

-- 
Regards,
Stefan Hett


Re: ABI changes analysis

Posted by Greg Stein <gs...@gmail.com>.
On Mon, Jun 27, 2016 at 6:54 AM, Greg Stein <gs...@gmail.com> wrote:

>
> On Sun, Jun 26, 2016 at 7:49 PM, Stefan <lu...@posteo.de> wrote:
> >...
>
>> And now I also remember and realize that these removed symbols were
>> actually private ones never intended to be exported (aka: double _ in
>> the name).
>
>
To be clear: they WERE intended to be exported, so other svn libraries
could use them. We have hundreds of such functions.

Cheers,
-g

Re: ABI changes analysis

Posted by Greg Stein <gs...@gmail.com>.
On Sun, Jun 26, 2016 at 7:49 PM, Stefan <lu...@posteo.de> wrote:
>...

> And now I also remember and realize that these removed symbols were
> actually private ones never intended to be exported (aka: double _ in
> the name). So 1.8/1.9 corrected this and ABI compatibility for these
> were intentionally broken.
>

Hunh? No compatibility was broken. Those were private symbols, so removing
them was perfectly acceptable. They are/were *not* part of the (defined)
ABI.

Did you see something else? Some other breakage? Or are you just referring
to the difference between actual and defined/public ABI?

Cheers,
-g

Re: ABI changes analysis

Posted by Stefan <lu...@posteo.de>.
On 6/27/2016 01:43, James McCoy wrote:
> On Sun, Jun 26, 2016 at 09:26:27PM +0200, Stefan wrote:
>> I'm just wondering why the backward compatibility for 1.9.0 (and 1.8.0)
>> doesn't state 100% here [1].
>>
>> Checking out the details on 1.9.0 [2] and there the details on
>> libsvn_subr [3] suggests 3 functions were removed:
>> - svn__apr_hash_index_key ( apr_hash_index_key( apr_hash_index_t const* hi )
>> - svn__apr_hash_index_klen ( apr_hash_index_key( apr_hash_index_t const*
>> hi )
>> - svn__apr_hash_index_val ( apr_hash_index_key( apr_hash_index_t const* hi )
> $ grep -q -r svn__apr_hash branches/1.9.x || echo none
> none
>
>> Looking at the list of added functions, I see exactly these though [4].
>> Am I missing something?
> [3] and [4] are the same page, just pointing to different anchors.  The
> svn__apr_hash_index_* functions only show up in the "Removed" section.
Oh right. That got me confused. Didn't realized it was the same page.
Thanks for pointing that out.

And now I also remember and realize that these removed symbols were
actually private ones never intended to be exported (aka: double _ in
the name). So 1.8/1.9 corrected this and ABI compatibility for these
were intentionally broken.

Regards,
Stefan


Re: ABI changes analysis

Posted by James McCoy <ja...@jamessan.com>.
On Sun, Jun 26, 2016 at 09:26:27PM +0200, Stefan wrote:
> I'm just wondering why the backward compatibility for 1.9.0 (and 1.8.0)
> doesn't state 100% here [1].
> 
> Checking out the details on 1.9.0 [2] and there the details on
> libsvn_subr [3] suggests 3 functions were removed:
> - svn__apr_hash_index_key ( apr_hash_index_key( apr_hash_index_t const* hi )
> - svn__apr_hash_index_klen ( apr_hash_index_key( apr_hash_index_t const*
> hi )
> - svn__apr_hash_index_val ( apr_hash_index_key( apr_hash_index_t const* hi )

$ grep -q -r svn__apr_hash branches/1.9.x || echo none
none

> Looking at the list of added functions, I see exactly these though [4].
> Am I missing something?

[3] and [4] are the same page, just pointing to different anchors.  The
svn__apr_hash_index_* functions only show up in the "Removed" section.

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB

Re: ABI changes analysis

Posted by Stefan <lu...@posteo.de>.
On 6/25/2016 07:45, Ponomarenko Andrey wrote:
> Hello,
>
> I'm working on a new project for backward compatibility analysis of the Linux ABIs. The report for Subversion base libraries has been recently added to the project: http://abi-laboratory.pro/tracker/timeline/subversion/
>
> The report is generated with the help of the abi-compliance-checker, abi-dumper and abi-tracker tools: https://github.com/lvc/abi-tracker
>
> Hope this will help users, maintainers and developers of libraries to maintain backward compatibility.
>
> Thank you.

Really nice work there.

I'm just wondering why the backward compatibility for 1.9.0 (and 1.8.0)
doesn't state 100% here [1].

Checking out the details on 1.9.0 [2] and there the details on
libsvn_subr [3] suggests 3 functions were removed:
- svn__apr_hash_index_key ( apr_hash_index_key( apr_hash_index_t const* hi )
- svn__apr_hash_index_klen ( apr_hash_index_key( apr_hash_index_t const*
hi )
- svn__apr_hash_index_val ( apr_hash_index_key( apr_hash_index_t const* hi )

Looking at the list of added functions, I see exactly these though [4].
Am I missing something?

[1] http://abi-laboratory.pro/tracker/timeline/subversion/
[2]
http://abi-laboratory.pro/tracker/objects_report/subversion/1.8.16/1.9.0/report.html
[3]
http://abi-laboratory.pro/tracker/compat_report/subversion/1.8.16/1.9.0/d6980/abi_compat_report.html#Removed
[4]
http://abi-laboratory.pro/tracker/compat_report/subversion/1.8.16/1.9.0/d6980/abi_compat_report.html#Added

Regards,
Stefan



Re: ABI changes analysis

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Mon, Jun 27, 2016 at 19:04:15 -0500:
> On Sat, Jun 25, 2016 at 11:28 AM, Daniel Shahaf <d....@daniel.shahaf.name>
> wrote:
> 
> > Greg Stein wrote on Sat, Jun 25, 2016 at 07:29:11 -0500:
> > > I've reviewed the reports, and it looks like we've maintained all our ABI
> > > guarantees. The changes are what I would expect.
> > >
> > > Thank you for this!
> >
> > +1 on both counts.
> >
> > Perhaps we could run this on candidate tarballs as part of the release
> > process?
> >
> 
> At least at major releases, if not every candidate. I haven't explored the
> toolset to see how packaged/automatable it is.
> 
> Conceivably, we could have a buildbot run this nightly-ish (weekly?), and
> have it flag issues relative to the prior release.

Filed as SVN-4640.

Re: ABI changes analysis

Posted by Greg Stein <gs...@gmail.com>.
On Tue, Jul 5, 2016 at 6:36 AM, Ponomarenko Andrey <
andrewponomarenko@yandex.ru> wrote:

> 28.06.2016, 03:04, "Greg Stein":
>
> On Sat, Jun 25, 2016 at 11:28 AM, Daniel Shahaf wrote:
>
> Greg Stein wrote on Sat, Jun 25, 2016 at 07:29:11 -0500:
> > I've reviewed the reports, and it looks like we've maintained all our ABI
> > guarantees. The changes are what I would expect.
> >
> > Thank you for this!
>
> +1 on both counts.
>
> Perhaps we could run this on candidate tarballs as part of the release
> process?
>
> At least at major releases, if not every candidate. I haven't explored the
> toolset to see how packaged/automatable it is.
>
> Conceivably, we could have a buildbot run this nightly-ish (weekly?), and
> have it flag issues relative to the prior release.
>
> Cheers,
> -g
>
>
> See http://lvc.github.io/abi-compliance-checker/#ABI_Dumper for
> instructions on how to run the basic analysis tool on the library objects.
>

That is some fantastic work. Thank you!

Re: ABI changes analysis

Posted by Greg Stein <gs...@gmail.com>.
On Sat, Jun 25, 2016 at 11:28 AM, Daniel Shahaf <d....@daniel.shahaf.name>
wrote:

> Greg Stein wrote on Sat, Jun 25, 2016 at 07:29:11 -0500:
> > I've reviewed the reports, and it looks like we've maintained all our ABI
> > guarantees. The changes are what I would expect.
> >
> > Thank you for this!
>
> +1 on both counts.
>
> Perhaps we could run this on candidate tarballs as part of the release
> process?
>

At least at major releases, if not every candidate. I haven't explored the
toolset to see how packaged/automatable it is.

Conceivably, we could have a buildbot run this nightly-ish (weekly?), and
have it flag issues relative to the prior release.

Cheers,
-g

Re: ABI changes analysis

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Sat, Jun 25, 2016 at 07:29:11 -0500:
> I've reviewed the reports, and it looks like we've maintained all our ABI
> guarantees. The changes are what I would expect.
> 
> Thank you for this!

+1 on both counts.

Perhaps we could run this on candidate tarballs as part of the release
process?