Mailing List Archive: Perl: porters

[PATCH] API for the internals

Index | Next | Previous | View Threaded

gerard at ggoossen

Nov 4, 2009, 8:43 AM

Post #1 of 12 (101 views)
Permalink

[PATCH] API for the internals

As access to some part of the internals of core is desired (as for
example shown by the existance of Data-Alias). But we don't want this
to lock the whole core because some module is doing some stuff very
deep in the internals.

To solve this I would like to make a major version specific API.

Attached patch adds the requirement of having "PERL_CORE_5_<version>"
defined when PERL_CORE is defined.

The idea is that modules are allowed to use the internals of the perl
core (at their own risk of course) by defining PERL_CORE and
PERL_CORE_5_<version> (multiple versions can be supported by a module
by having multiple PERL_CORE_<version> definitions)

This means we can allow modules to do all kinds of interesting stuff,
because we have an interface for that, but that this interface is
version specific, allowing the core to be further developed.

You could say that we have an API exposing the internals, following
the tradition of being defined by the implementation.

With this change, normal release goes like this:
- First release candicate/beta is made.
- Modules doing ugly stuff like Abra::Cadabra, break with a compile error.
- Author of Abra::Cadabra makes changes to the module for changes
internal to perl, and adds the "#define PERL_CORE_5_<version>",
makes release. (Changes are the changes to the module were already
made as to work with bleadperl and only adding the define would be
enough).

A quick grep through CPAN showed that there are current 6 modules
having PERL_CORE defined (autobox, Goto-Cached, PersistentPerl,
Data-Alias, Faster, CGI-SpeedyCGI)

Of course the idea is not to break these modules, but just to enforce
that the author checks that nothing of the internals have changed in
such a way as to break the module. It might be that nothing used by
the module has changed and that just adding "#define
PERL_CORE_5_<version>" is enough, or some superficial changes in the
use of the interface might be required, or it might be that there
where significant changes and the core and that significant changes
are required.

Summarizing:
- The core developers promise to keep the core the same within each
major version.
- Module authors defining PERL_CORE promise to make adjustment with
each major perl release.
- Together we live happily ever after.

Gerard

Attachments:

0001-Add-Requirement-for-specific-Perl-version-when-reque.patch (1.80 KB)

nick at ccl4

Nov 4, 2009, 8:51 AM

Post #2 of 12 (96 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Wed, Nov 04, 2009 at 05:43:27PM +0100, Gerard Goossen wrote:

> Summarizing:
> - The core developers promise to keep the core the same within each
> major version.

No. I can't promise this.

Nicholas Clark

demerphq at gmail

Nov 4, 2009, 8:55 AM

Post #3 of 12 (96 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

2009/11/4 Nicholas Clark <nick[at]ccl4.org>:
> On Wed, Nov 04, 2009 at 05:43:27PM +0100, Gerard Goossen wrote:
>
>> Summarizing:
>> - The core developers promise to keep the core the same within each
>> major version.
>
> No. I can't promise this.

We already do.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

nick at ccl4

Nov 4, 2009, 8:58 AM

Post #4 of 12 (96 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Wed, Nov 04, 2009 at 05:55:31PM +0100, demerphq wrote:
> 2009/11/4 Nicholas Clark <nick[at]ccl4.org>:
> > On Wed, Nov 04, 2009 at 05:43:27PM +0100, Gerard Goossen wrote:
> >
> >> Summarizing:
> >> - The core developers promise to keep the core the same within each
> >> major version.
> >
> > No. I can't promise this.
>
> We already do.

No, we don't
We try *hard* not to break things.

But I refuse to promise that we won't *ever*, because I *DO NOT WANT* to
encourage people to think that the guts are theirs to play with.

If the *public* API is lacking, I want people to submit patches to work to
fix *that*. Not to have a perpetual state of "peeking".

Nicholas Clark

zefram at fysh

Nov 4, 2009, 9:05 AM

Post #5 of 12 (96 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

demerphq wrote:
>We already do.

I ran into PAD_MAX having changed between 5.8.8 and 5.8.9. Now, that
constant wasn't even defined in a header, it's only defined in pad.c, but
the value is effectively part of the pad API. That's one of the things
that's currently considered non-public, but seriously needs a public API.

(The module in question is Lexical::Var, and I ended up with this logic:

static PADOFFSET pad_max(void)
{
#if PERL_VERSION_GE(5,9,5)
return I32_MAX;
#elif PERL_VERSION_GE(5,9,0)
return 999999999;
#elif PERL_VERSION_GE(5,8,0)
static PADOFFSET max;
if(!max) {
SV *versv = get_sv("]", 0);
char *verp = SvPV_nolen(versv);
max = strGE(verp, "5.008009") ? I32_MAX : 999999999;
}
return max;
#else /* <5.8.0 */
return 999999999;
#endif /* <5.8.0 */
}

Fun, eh.)

-zefram

doughera at lafayette

Nov 4, 2009, 10:48 AM

Post #6 of 12 (88 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Wed, 4 Nov 2009, Gerard Goossen wrote:

> As access to some part of the internals of core is desired (as for
> example shown by the existance of Data-Alias). But we don't want this
> to lock the whole core because some module is doing some stuff very
> deep in the internals.
>
> To solve this I would like to make a major version specific API.
>
> Attached patch adds the requirement of having "PERL_CORE_5_<version>"
> defined when PERL_CORE is defined.

I note that you (deliberately) omitted the subversion, so that you have
something like PERL_CORE_5_12 instead of PERL_CORE_5_12_1 . I think
that's a bad idea since it seems to be making a promise that no private,
internal parts of the core can be changed in a non-compatible way even in
a maintenance release. While that's probably often true, I don't think it
should be policy to require it.

> The idea is that modules are allowed to use the internals of the perl
> core (at their own risk of course) by defining PERL_CORE and
> PERL_CORE_5_<version> (multiple versions can be supported by a module
> by having multiple PERL_CORE_<version> definitions)

> This means we can allow modules to do all kinds of interesting stuff,
> because we have an interface for that, but that this interface is
> version specific, allowing the core to be further developed.

Modules can, of course, already do this by defining PERL_CORE. Since
PERL_REVISION, PERL_VERSION, and PERL_SUBVERSION are already available,
module authors can (and do) make those things version-specific.

> With this change, normal release goes like this:
> - First release candicate/beta is made.
> - Modules doing ugly stuff like Abra::Cadabra, break with a compile error.
> - Author of Abra::Cadabra makes changes to the module for changes
> internal to perl, and adds the "#define PERL_CORE_5_<version>",
> makes release. (Changes are the changes to the module were already
> made as to work with bleadperl and only adding the define would be
> enough).

Module authors can already do this by using the various PERL_* #defines
that already exist in patchlevel.h to only allow compilation on versions
known to work. Whether or not they choose to do so, of course, is a
different matter.

> Of course the idea is not to break these modules, but just to enforce
> that the author checks that nothing of the internals have changed in
> such a way as to break the module. It might be that nothing used by
> the module has changed and that just adding "#define
> PERL_CORE_5_<version>" is enough, or some superficial changes in the
> use of the interface might be required, or it might be that there
> where significant changes and the core and that significant changes
> are required.

If the module author is careful and carefully reviews the changes in the
perl internals to check whether they impact the module, I could see this
being a benefit. If, on the other hand, the module author merely adds
#define PERL_CORE_<version> and runs the test suite to "see if anything
breaks", then the extra hoops haven't really done any good and have simply
added additional makework.

Of course, the module author can already enforce the breakage by using the
existing #defines in patchlevel.h.

> Summarizing:
> - The core developers promise to keep the core the same within each
> major version.

No, I think that's not a good idea.

> - Module authors defining PERL_CORE promise to make adjustment with
> each major perl release.

I think they may have to make them with each major or minor release.

> - Together we live happily ever after.

Indeed, that's a noble goal! Another way to work towards that goal might
be to work to develop an API that provides whatever was missing and
required the PERL_CORE stuff in the first place.

--
Andy Dougherty doughera[at]lafayette.edu

gerard at ggoossen

Nov 5, 2009, 1:28 PM

Post #7 of 12 (81 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Wed, Nov 04, 2009 at 04:58:44PM +0000, Nicholas Clark wrote:
> On Wed, Nov 04, 2009 at 05:55:31PM +0100, demerphq wrote:
> > 2009/11/4 Nicholas Clark <nick[at]ccl4.org>:
> > > On Wed, Nov 04, 2009 at 05:43:27PM +0100, Gerard Goossen wrote:
> > >
> > >> Summarizing:
> > >> - The core developers promise to keep the core the same within each
> > >> ? major version.
> > >
> > > No. I can't promise this.
> >
> > We already do.
>
> No, we don't
> We try *hard* not to break things.

Probably promise is too strong a word, if we need we can break it
(maybe add require something like #define PERL_CORE_5_<version>b), but
with our current plan of making minor releases only fix cirtical bugs
and build issues, this not breaking it should not be a problem.

> But I refuse to promise that we won't *ever*, because I *DO NOT WANT* to
> encourage people to think that the guts are theirs to play with.

Patch discourages playing with the internals, because it will break
with the next major release. People will be peeking into the
internals, using this at least it is clear that whay they are doing is
problematic and that it will break with the next release.

> If the *public* API is lacking, I want people to submit patches to work to
> fix *that*. Not to have a perpetual state of "peeking".

The patch isn't meant to discourage the development of a public API,
but there will always be people who want access to more stuff, the
patch is meant to regulate that.

It only adds a check for modules which already use PERL_CORE.

Gerard

gerard at ggoossen

Nov 5, 2009, 1:49 PM

Post #8 of 12 (81 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Wed, Nov 04, 2009 at 01:48:32PM -0500, Andy Dougherty wrote:
> On Wed, 4 Nov 2009, Gerard Goossen wrote:
>
> > As access to some part of the internals of core is desired (as for
> > example shown by the existance of Data-Alias). But we don't want this
> > to lock the whole core because some module is doing some stuff very
> > deep in the internals.
> >
> > To solve this I would like to make a major version specific API.
> >
> > Attached patch adds the requirement of having "PERL_CORE_5_<version>"
> > defined when PERL_CORE is defined.
>
> I note that you (deliberately) omitted the subversion, so that you have
> something like PERL_CORE_5_12 instead of PERL_CORE_5_12_1 . I think
> that's a bad idea since it seems to be making a promise that no private,
> internal parts of the core can be changed in a non-compatible way even in
> a maintenance release. While that's probably often true, I don't think it
> should be policy to require it.

I don't like making it the default to be incompatible with each minor
release, becuase if we would make a minor release, only solving some
critical securify issues like bufferoverflows, it would break these
modules.

> [...]
>
> > Of course the idea is not to break these modules, but just to enforce
> > that the author checks that nothing of the internals have changed in
> > such a way as to break the module. It might be that nothing used by
> > the module has changed and that just adding "#define
> > PERL_CORE_5_<version>" is enough, or some superficial changes in the
> > use of the interface might be required, or it might be that there
> > where significant changes and the core and that significant changes
> > are required.
>
> If the module author is careful and carefully reviews the changes in the
> perl internals to check whether they impact the module, I could see this
> being a benefit. If, on the other hand, the module author merely adds
> #define PERL_CORE_<version> and runs the test suite to "see if anything
> breaks", then the extra hoops haven't really done any good and have simply
> added additional makework.

Of course, but we can't force them to review the changes, this way at
least we give them a clear signal that they supposed to check it.

> [...]

Gerard

nick at ccl4

Nov 6, 2009, 5:47 AM

Post #9 of 12 (77 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Thu, Nov 05, 2009 at 10:28:00PM +0100, Gerard Goossen wrote:
> On Wed, Nov 04, 2009 at 04:58:44PM +0000, Nicholas Clark wrote:
> > On Wed, Nov 04, 2009 at 05:55:31PM +0100, demerphq wrote:
> > > 2009/11/4 Nicholas Clark <nick[at]ccl4.org>:
> > > > On Wed, Nov 04, 2009 at 05:43:27PM +0100, Gerard Goossen wrote:
> > > >
> > > >> Summarizing:
> > > >> - The core developers promise to keep the core the same within each
> > > >> ? major version.
> > > >
> > > > No. I can't promise this.
> > >
> > > We already do.
> >
> > No, we don't
> > We try *hard* not to break things.
>
> Probably promise is too strong a word, if we need we can break it
> (maybe add require something like #define PERL_CORE_5_<version>b), but
> with our current plan of making minor releases only fix cirtical bugs
> and build issues, this not breaking it should not be a problem.

I do not share your confidence.

> > But I refuse to promise that we won't *ever*, because I *DO NOT WANT* to
> > encourage people to think that the guts are theirs to play with.
>
> Patch discourages playing with the internals, because it will break
> with the next major release. People will be peeking into the
> internals, using this at least it is clear that whay they are doing is
> problematic and that it will break with the next release.

No, I don't agree. (See below)

> > If the *public* API is lacking, I want people to submit patches to work to
> > fix *that*. Not to have a perpetual state of "peeking".
>
> The patch isn't meant to discourage the development of a public API,
> but there will always be people who want access to more stuff, the
> patch is meant to regulate that.
>
> It only adds a check for modules which already use PERL_CORE.

No, it also legitimises this approach. Which could well increase the number
of modules prodding deeply, which makes it harder to avoid breaking them when
making changes. Specifically:

Right now, if you define PERL_CORE you get to keep both pieces if it breaks.
It's not supported. That doesn't mean that it *will* break, or that we
actively *try* to break it, but it does mean that it's "at risk".

By "try *hard*" not to break things, I meant that before making changes that
I suspected would break things, I'd check Google's codesearch and an unpacked
CPAN, to see what (if anything) was using it. Sure, that does not and can not
find code elsewhere (often referred to as the Darkpan) but it seems a big
enough sample size to make a judgment.

Right now, there are very few modules, which (a) makes it easy to check,
(b) means that the proportion of modules which are at risk of breaking is low.

Providing a blessing that will increase the number of modules coupling tightly
to the implementation in ways that aren't tested in the core itself will

(a) make it harder to check
(b) means that the proportion of modules which are at risk increases

The change proposed INCREASES the work load on the core maintainers.
It externalises the (social) responsibility of CPAN module authors to feed
back on where the API is lacking.

Nicholas Clark

nick at ccl4

Nov 6, 2009, 5:51 AM

Post #10 of 12 (77 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Fri, Nov 06, 2009 at 01:47:02PM +0000, Nicholas Clark wrote:
> On Thu, Nov 05, 2009 at 10:28:00PM +0100, Gerard Goossen wrote:
> > On Wed, Nov 04, 2009 at 04:58:44PM +0000, Nicholas Clark wrote:
> > > On Wed, Nov 04, 2009 at 05:55:31PM +0100, demerphq wrote:
> > > > 2009/11/4 Nicholas Clark <nick[at]ccl4.org>:
> > > > > On Wed, Nov 04, 2009 at 05:43:27PM +0100, Gerard Goossen wrote:
> > > > >
> > > > >> Summarizing:
> > > > >> - The core developers promise to keep the core the same within each
> > > > >> ? major version.
> > > > >
> > > > > No. I can't promise this.
> > > >
> > > > We already do.
> > >
> > > No, we don't
> > > We try *hard* not to break things.
> >
> > Probably promise is too strong a word, if we need we can break it
> > (maybe add require something like #define PERL_CORE_5_<version>b), but
> > with our current plan of making minor releases only fix cirtical bugs
> > and build issues, this not breaking it should not be a problem.
>
> I do not share your confidence.

I forgot to add:

Do not assume that the current plan is what will happen.
Do not assume that even if the current plan does happen, that there won't be
a change of plan in the future.

What you are proposing commits the core to a straight jacket.
To be sure of not breaking the promise, we can't change *anything*.

And in some cases, it would mean

"We fixed that bug. But to do so we had to break the internal API. So
you'll need to upgrade from 5.12.3 to 5.14.1"

Which, of course, is not what any sane sysadmin or release engineer would
consider a sane, acceptable solution.

And to be sure, I'm going to repeat:

> The change proposed INCREASES the work load on the core maintainers.
> It externalises the (social) responsibility of CPAN module authors to feed
> back on where the API is lacking.

Nicholas Clark

nick at ccl4

Nov 16, 2009, 2:18 AM

Post #11 of 12 (30 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

On Wed, Nov 04, 2009 at 05:05:13PM +0000, Zefram wrote:
> demerphq wrote:
> >We already do.
>
> I ran into PAD_MAX having changed between 5.8.8 and 5.8.9. Now, that
> constant wasn't even defined in a header, it's only defined in pad.c, but
> the value is effectively part of the pad API. That's one of the things
> that's currently considered non-public, but seriously needs a public API.
>
> (The module in question is Lexical::Var, and I ended up with this logic:

and I was wondering "why didn't Andreas and Slaven's extensive testing of
CPAN modules spot this?".

Answer - because Lexical::Var was first released after 5.8.9 :-)

Nicholas Clark

zefram at fysh

Nov 16, 2009, 4:21 AM

Post #12 of 12 (30 views)
Permalink

Re: [PATCH] API for the internals [In reply to]

Nicholas Clark wrote:
>Answer - because Lexical::Var was first released after 5.8.9 :-)

Yes, I ran into it backwards.

-zefram

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com