You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by William A Rowe Jr <wr...@apache.org> on 2022/01/14 00:58:32 UTC

Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
indicates that 1.7.1
isn't releasable as-is. Any hints?

+T apr_pool_find
+T apr_pool_join
+T apr_pool_lock
+T apr_pool_num_bytes
+T apr__pool_unmanage

Preserving whatever has changed on 1.8.x branch for ABI breaking changes.

On Thu, Jan 13, 2022 at 6:36 PM <wr...@apache.org> wrote:
>
> Author: wrowe
> Date: Fri Jan 14 00:36:06 2022
> New Revision: 1897021
>
> URL: http://svn.apache.org/viewvc?rev=1897021&view=rev
> Log:
> Branch 1.8.x due to ABI-breaking commits
>
> Begin 1.8.x development from here, with no need to roll anything back
> from this branch. The 1.7.x branch will need breaking backports all
> rolled back.
>
>
> Added:
>     apr/apr/branches/1.8.x/   (props changed)
>       - copied from r1897020, apr/apr/branches/1.7.x/
>
> Propchange: apr/apr/branches/1.8.x/
> ------------------------------------------------------------------------------
> --- svn:ignore (added)
> +++ svn:ignore Fri Jan 14 00:36:06 2022
> @@ -0,0 +1,52 @@
> +Makefile
> +config.cache
> +config.log
> +config.nice
> +config.status
> +configure
> +libtool
> +apr-config
> +apr-*-config
> +apr-config.out
> +LibD
> +LibNTD
> +LibR
> +LibNTR
> +Debug
> +DebugNT
> +Release
> +ReleaseNT
> +*.ncb
> +*.opt
> +*.plg
> +apr.exp
> +exports.c
> +export_vars.[ch]
> +.libs
> +.deps
> +*.la
> +libapr.rc
> +*.aps
> +*.plg
> +*.dep
> +*.mak
> +*.rc
> +BuildLog.htm
> +*.stc
> +*.stt
> +*.sto
> +*.vcproj
> +*.vcproj.*
> +autom4te.cache
> +ltcf-c.sh
> +build-outputs.mk
> +.make.dirs
> +*.bb
> +*.bbg
> +*.da
> +coverage
> +apr*.pc
> +apr.spec
> +libtool.exe
> +
> +
>
> Propchange: apr/apr/branches/1.8.x/
> ------------------------------------------------------------------------------
> --- svn:mergeinfo (added)
> +++ svn:mergeinfo Fri Jan 14 00:36:06 2022
> @@ -0,0 +1,4 @@
> +/apr/apr/branches/1.4.x:1003369,1101301
> +/apr/apr/trunk:733052,739635,741862,741866-741867,741869,741871,745763-745764,746310,747990,748080,748361,748371,748565,748888,748902,748988,749810,760443,767895,775683,782838,783398,783958,784633,784773,788588,789050,793192-793193,794118,794485,795267,799497,800627,809745,809854,810472,811455,813063,821306,829490,831641,832904,835607,888669,892028,892159,892435,892909,896382,896653,899905,901088,902077,902090,908427,910419,910597,917819,917837-917838,923311,923320,925965,929796,930508,931973,932585,936323,951771,960665,960671,979891,983618,989450,990435,1003338,1044440,1044447,1055657,1072165,1078845,1081462,1081495,1082177,1083038,1083242,1084662,1086695,1088023,1089031,1089129,1089438,1099348,1103310,1183683,1183685-1183686,1183688,1183693,1183698,1213382,1235047,1236970,1237078,1237507,1240472,1340286,1340288,1340470,1341193,1341196,1343233,1343243,1367050,1368819,1370494,1372018,1372022,1372093,1372849,1376957,1384764,1389077,1400200,1402868,1405985,1406690,1420106,1420109,1425
>  356,1428809,1438940,1438957-1438959,1442903,1449568,1456418,1459994,1460179-1460180,1460182,1460184,1460241,1460399,1460405,1462738,1462813,1470186,1470348,1475509,1478905,1478954,1480067,1481186,1481262,1481265,1484271,1487796,1489517,1496407,1502804,1510354,1516261,1523384,1523479,1523484,1523505,1523521,1523604,1523613,1523615,1523844-1523845,1523853,1524014,1524031,1528797,1528809,1529488,1529495,1529515,1529521,1529668,1530786,1530800,1530988,1531554,1531768,1531884,1532022,1533104,1533111,1533979,1534882,1535027,1535157,1536744,1538171,1539374,1539389,1539455,1539603,1541054,1541061,1541486,1541655,1541666,1541744,1542601,1542779,1543033,1543056,1548575,1550907,1551650,1551659,1558905,1559382,1559873,1559975,1561040,1561260,1561265,1561321,1561347,1561356,1561361,1561394,1561555,1571894,1575509,1578420,1587045,1587063,1587543,1587545,1588878,1588937,1589982,1593611,1593614-1593615,1593680,1594684,1594708,1595549,1597797,1597803,1604590,1604596,1604598,1605104,1610854,1611023,1
>  611107,1611110,1611117,1611120,1611125,1611184,1611193,1611466,1611515,1611517,1625173,1626564,1634615,1642159,1648830,1664406,1664447,1664451,1664471,1664769-1664770,1664775,1664904,1664911,1664958,1666341,1666411,1666458,1666611,1667420-1667421,1667423,1667900-1667901,1667903,1667914-1667916,1667962,1669077,1671292,1671329,1671356,1671386,1671389,1671513-1671514,1671957,1672354,1672366,1672495,1672575,1674566,1675644,1675656,1675668,1675967,1675970,1675982,1676013,1683521,1685929,1696140,1696767,1722547,1722557,1726928,1727020,1727160,1727175,1727199,1728957,1732582,1733451,1733594,1733694,1733706,1733708,1733775,1734816,1736552,1738791,1738925,1750374,1755709,1755740,1755746,1755758,1755954,1761279,1762326,1774712,1774973,1775069,1776994,1776998,1788334,1788337,1788929,1789947,1789998,1790045,1790200,1790296,1790302-1790304,1790330-1790331,1790436,1790439,1790444,1790446,1790488,1790521,1790523,1790569,1790632,1791598,1791718,1791728,1792620-1792622,1792625,1792961,1792963,179426
>  6,1797415,1798105,1805380,1806296,1807975,1808039,1808836,1808910,1809649,1810452,1813286,1813330,1814239-1814240,1814326,1814329,1814331,1816527,1816628,1817485,1819857-1819858,1819860-1819861,1819934-1819935,1820080,1820755,1822357,1827534,1832203,1832691,1832985,1834253,1834494,1834541,1836235,1839068,1839493,1839622,1839769,1840372,1841078,1846806,1850087,1850095,1851541-1851542,1854123,1855049,1855347,1855443-1855444,1855839-1855840,1855855,1855864,1855867,1855877,1855949,1856022,1856042-1856043,1856046,1856050,1856063,1856089,1856096,1856178,1856192,1856196,1856756,1856873,1858596,1860057,1863205,1863217,1868129,1868502,1871998,1872195,1872203,1875062,1875065,1877444,1878279,1878340,1878342-1878343,1878354,1878365,1883340,1883666,1883728,1883742,1883749-1883750,1883800-1883802,1883805-1883806,1883868,1883870,1884077-1884078,1887060,1887279,1887481-1887482,1887490,1887506,1888017,1888251,1889604,1890191,1891204,1891310,1892374,1892618,1893198,1893202,1893204,1893445,1894167,189
>  4621-1894622,1894719,1894914,1895106,1895111,1895175,1895178,1895181,1895465,1896535,1896722,1896747,1896782,1896803,1896811,1896933,1896956,1896971,1896990,1896992
> +/apr/apr/trunk/test/testnames.c:1460405
> +/httpd/httpd/trunk:1604590
>
>

Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

Posted by Yann Ylavic <yl...@gmail.com>.
[plus [1] and [2] links..]

On Fri, Jan 14, 2022 at 12:06 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Fri, Jan 14, 2022 at 1:59 AM William A Rowe Jr <wr...@apache.org> wrote:
> >
> > A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
> > indicates that 1.7.1
> > isn't releasable as-is. Any hints?
> >
> > +T apr_pool_find
> > +T apr_pool_join
> > +T apr_pool_lock
> > +T apr_pool_num_bytes
>
> Those come from r1863234 [1], possibly we can let them as empty macros
> in 1.7.x and still add the ones for apr_pool_join() and
> apr_pool_num_bytes() which were missing for !APR_POOL_DEBUG?
> Yet it might make apps using them from 1.7.1 (w/o #ifdef
> APR_POOL_DEBUG) not compilable with 1.7.0 (though still ABI compatible
> with all 1.7.x), but no one would be able to use them as function
> pointers for instance, which would be a ABI/runtime breakage. Could
> that hurt actually?
>
> > +T apr__pool_unmanage
>
> That one is for internal use only (from [2]), not APR_DECLAREd so
> should be fine?

[1] https://svn.apache.org/r1863234
[2] https://svn.apache.org/r1884103

Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

Posted by William A Rowe Jr <wr...@apache.org>.
On Sat, Jan 15, 2022 at 4:37 AM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Sat, Jan 15, 2022 at 2:00 AM William A Rowe Jr <wr...@apache.org> wrote:
> >
> At this point I question the relevance of releasing 1.7.1 at all, I
> think most people will be interested in 1.8.0 and I'd advise distros
> to upgrade to that too. Who will be using 1.7.1? What is the rationale
> for releasing both and how confusing will that be?'
> Hopefully Ivan can address the Windows apr_file_t (as suitable) soon
> enough, in this case could we release 1.8.0 only?

My expectation is it lives 1 1/2 hot seconds before there is something more
interesting.

But, if 1.8.0 release fails, it is a smart fallback, that's all. As
devs, I understand
our hesitancy to offering fallbacks, but there are consumers to consider.

Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

Posted by Yann Ylavic <yl...@gmail.com>.
On Sat, Jan 15, 2022 at 2:00 AM William A Rowe Jr <wr...@apache.org> wrote:
>
> On Fri, Jan 14, 2022 at 5:07 AM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > On Fri, Jan 14, 2022 at 1:59 AM William A Rowe Jr <wr...@apache.org> wrote:
> > >
> > > A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
> > > indicates that 1.7.1
> > > isn't releasable as-is. Any hints?
> > >
> > > +T apr_pool_find
> > > +T apr_pool_join
> > > +T apr_pool_lock
> > > +T apr_pool_num_bytes
> >
> > Those come from r1863234 [1], possibly we can let them as empty macros
> > in 1.7.x and still add the ones for apr_pool_join() and
> > apr_pool_num_bytes() which were missing for !APR_POOL_DEBUG?
> > Yet it might make apps using them from 1.7.1 (w/o #ifdef
> > APR_POOL_DEBUG) not compilable with 1.7.0 (though still ABI compatible
> > with all 1.7.x), but no one would be able to use them as function
> > pointers for instance, which would be a ABI/runtime breakage. Could
> > that hurt actually?
>
> Worse, a macro -> function without a major bump? That's awkward. Remember
> the .so remains loadable throughout apr-1.so lifespan.

Yes I see your point for macro -> function.

>
> Afraid this has to be rejected outright on apr-1.7. There are contracts with our
> developers that they can load the 1.7.0 .so in place of a binary built
> against 1.7.2
> and **nothing bad can happen** (other than the 'whoops - you wanted
> that bug fix.)
> The binaries won't fail to load. This sets us up for failure, and why put macro
> stubs here if they will be no-ops? What purpose is this serving to break what
> has been a pretty reliable solution in developers' hands?

What I'm thinking is to revert r1863234 in 1.7.x, thus back to
r1863233's apr_pools.h code do:

Index: include/apr_pools.h
===================================================================
--- include/apr_pools.h    (revision 1863233)
+++ include/apr_pools.h    (working copy)
@@ -799,6 +799,16 @@ APR_DECLARE(void) apr_pool_lock(apr_pool_t *pool,
 #endif
 #define apr_pool_join(a,b)

+#ifdef apr_pool_find
+#undef apr_pool_find
+#endif
+#define apr_pool_find(mem) (NULL)
+
+#ifdef apr_pool_num_bytes
+#undef apr_pool_num_bytes
+#endif
+#define apr_pool_num_bytes(p, rec) ((apr_size_t)0)
+
 #ifdef apr_pool_lock
 #undef apr_pool_lock
 #endif
--

So yes, that's two new macros so any (new) code using them when
!APR_POOL_DEBUG would build with 1.7.1 but not 1.7.0, though it would
_run_ with both apr-1.7.[01].so (no ABI change).

>
> See https://apr.apache.org/versioning.html
>
> What's the actual reservation about 1.8.0? (After a prompt 1.7.1)?

No reservation if we are able to release faster than we used to lately..
I also advocate for people who work on the APR code, for their changes
to be integrated (as much/reasonable as possible) in the version they
actually use.

>
>
> > > +T apr__pool_unmanage
> >
> > That one is for internal use only (from [2]), not APR_DECLAREd so
> > should be fine?
>
> Still, exported on linux, curious how to avoid that. Even as an
> internal, not an issue
> in 1.8.0, an issue on 1.7.x branch now.

That's no different than other arch/private yet exported functions,
but yes introducing it in a patch release makes an app build with
1.7.1 not runnable with apr-1.7.0.so.

This function serves a real bugfix though, apr threads had real pool
lifetime issues before that (raised by the httpd tests suite built/run
under AddressSanitizer).
I don't remember all the details of the change but possibly for this
particular apr__pool_unmanage() introduction it's only about
apr_thread_detach() correctness, so the other option might be to
APR_ENOTIMPL there in 1.7.1, or leave a known defect, not sure it
helps more..

At this point I question the relevance of releasing 1.7.1 at all, I
think most people will be interested in 1.8.0 and I'd advise distros
to upgrade to that too. Who will be using 1.7.1? What is the rationale
for releasing both and how confusing will that be?
Hopefully Ivan can address the Windows apr_file_t (as suitable) soon
enough, in this case could we release 1.8.0 only?

Cheers;
Yann.

Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

Posted by William A Rowe Jr <wr...@apache.org>.
On Fri, Jan 14, 2022 at 5:07 AM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Fri, Jan 14, 2022 at 1:59 AM William A Rowe Jr <wr...@apache.org> wrote:
> >
> > A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
> > indicates that 1.7.1
> > isn't releasable as-is. Any hints?
> >
> > +T apr_pool_find
> > +T apr_pool_join
> > +T apr_pool_lock
> > +T apr_pool_num_bytes
>
> Those come from r1863234 [1], possibly we can let them as empty macros
> in 1.7.x and still add the ones for apr_pool_join() and
> apr_pool_num_bytes() which were missing for !APR_POOL_DEBUG?
> Yet it might make apps using them from 1.7.1 (w/o #ifdef
> APR_POOL_DEBUG) not compilable with 1.7.0 (though still ABI compatible
> with all 1.7.x), but no one would be able to use them as function
> pointers for instance, which would be a ABI/runtime breakage. Could
> that hurt actually?

Worse, a macro -> function without a major bump? That's awkward. Remember
the .so remains loadable throughout apr-1.so lifespan.

Afraid this has to be rejected outright on apr-1.7. There are contracts with our
developers that they can load the 1.7.0 .so in place of a binary built
against 1.7.2
and **nothing bad can happen** (other than the 'whoops - you wanted
that bug fix.)
The binaries won't fail to load. This sets us up for failure, and why put macro
stubs here if they will be no-ops? What purpose is this serving to break what
has been a pretty reliable solution in developers' hands?

See https://apr.apache.org/versioning.html

What's the actual reservation about 1.8.0? (After a prompt 1.7.1)?


> > +T apr__pool_unmanage
>
> That one is for internal use only (from [2]), not APR_DECLAREd so
> should be fine?

Still, exported on linux, curious how to avoid that. Even as an
internal, not an issue
in 1.8.0, an issue on 1.7.x branch now.

Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Jan 14, 2022 at 1:59 AM William A Rowe Jr <wr...@apache.org> wrote:
>
> A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
> indicates that 1.7.1
> isn't releasable as-is. Any hints?
>
> +T apr_pool_find
> +T apr_pool_join
> +T apr_pool_lock
> +T apr_pool_num_bytes

Those come from r1863234 [1], possibly we can let them as empty macros
in 1.7.x and still add the ones for apr_pool_join() and
apr_pool_num_bytes() which were missing for !APR_POOL_DEBUG?
Yet it might make apps using them from 1.7.1 (w/o #ifdef
APR_POOL_DEBUG) not compilable with 1.7.0 (though still ABI compatible
with all 1.7.x), but no one would be able to use them as function
pointers for instance, which would be a ABI/runtime breakage. Could
that hurt actually?

> +T apr__pool_unmanage

That one is for internal use only (from [2]), not APR_DECLAREd so
should be fine?


Regards;
Yann.

Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Jan 14, 2022 at 1:59 AM William A Rowe Jr <wr...@apache.org> wrote:
>
> A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
> indicates that 1.7.1
> isn't releasable as-is. Any hints?
>
> +T apr_pool_find
> +T apr_pool_join
> +T apr_pool_lock
> +T apr_pool_num_bytes

These should have disappeared with r1897209.

> +T apr__pool_unmanage

And this one with r1897210.


Cheers;
Yann.