Reminder about adding new configuration steps (and other potentially significant code)

[Peter Lobsinger]

On Tue, Oct 5, 2010 at 8:30 AM, James E Keenan <jkeen at verizon.net> wrote:
> Fellow Parrot developes,
>
> In its discussion of adding new components to the Parrot configuration
> process, docs/configuration.pod contains this language:
>
> "It is strongly recommended that you file a Trac ticket in which you state
> the rationale for adding a new configuration step and sketch
> out what the code in the step would look like."
>
> We included this language in November 2009 to guarantee that developers who
> sought changes in the process would:
>
> * present their rationale for the changes to other Parrot developers;
>
> * provide an opportunity for other developers to review the code, both at
> the level of individual statements and at the level of assessing the
> rationale for the changes;
>
> * allow thorough testing of the code -- typically, in a branch -- on all our
> supported operating systems prior to committing the code to trunk.
>
> As one who has been involved with maintaining Parrot's configuration system
> for over three years, I strongly believe that these procedures should be
> followed.  Unfortunately, three new configuration steps have been added in
> the past week, none of which have had Trac tickets opened for them and none
> of which were developed in branches prior to commitment to trunk.  These new
> steps are:
>
> Sep 29 17:12 config/auto/timespec.pm
> Oct  2 18:45 config/auto/stat.pm
> Oct  2 23:14 config/auto/infnan.pm
>
> The only rationale we have for these new config steps is that presented in
> their commit messages.

As the develop per behind these additions, I apologize if my commit
messages providing the rationale were terse. I had good reasons (see
below) for adding these and they were, to the best of my knowledge at
the time, working at the time they were committed.

> I haven't had time to assess fully the purpose or
> impact of auto::timespec or auto::stat -- or to write t/step/ tests for them
> -- but we are having a lot of problems with auto::infnan; see TT #1813; TT
> #1814; and TT #1815.  I recognize that the developer who added these steps
> is working to resolve the problems -- but we should be facing these problems
> in a branch, not in trunk.

This is exactly one problem. Three tickets were opened, one for each
test in which it manifests, though the diagnostic Have/Want messages
and originating commit are the same. Further, this is occurring on
only one championed platform.

> I want to stress that the configuration system is not written in stone.
>  There may be a strong case for additional configuration steps -- but we
> should hear that case before such steps are added.  Furthermore, we know
> that we have to figure out a way to reduce and ultimately eliminate our
> dependence on Perl 5 %Config in init::defaults and auto::headers.
>
> If you think about it, the recommendations in the docs for best practices in
> creating configuration steps really applies to all our code base.  We know
> that there are vast areas in our code base in need of improvement.  We know
> that in many areas different approaches will be needed.  We want developers
> to be able to plunge in and get to work. But when a developer's proposed
> changes to the code base affects Parrot as a whole, we need to have the code
> and the rationale for the code presented and developed in an orderly manner.
>  The presentation takes place in Trac (perhaps supplemented by posts to this
> list); the development should take place in a branch.

While these guidelines seem like a good idea on paper, they may not be
the best in practice.

List postings are subject to warnocking. Tickets even more so. It is
difficult recruiting testers for all core platforms (let alone
best-effort platforms like Darwin/PPC). Svn branches are clunky, I've
seen them fail to execute even a fast-forward merge without conflicts.

And what does this extensive testing get us? The test suite fails to
identify many glaring bugs. Did you know the tracing and profiling
runcores had been broken for months (I was at fault for that too :( )?
Many of the platforms we test against share a lot of common ground
which hides our assumptions, giving us a false sense of diversity and
a false sense of portability. Did you know test/coretest would hang if
Parrot was built without threads?

All this has caused me to tend towards a
shoot-first-ask-questions-later approach for small changes. We're at
the ask-questions part now. So if you feel these changes don't belong,
you are more than welcome to revert them.

> Thank you very much.
>
> Jim Keenan (kid51)
>
> _______________________________________________
> http://lists.parrot.org/mailman/listinfo/parrot-dev
>

= Rationale For Changes =

Many of my commits in the previous week have been the result of an
attempt to run Parrot on Minix/i386 with the amsterdam compiler kit
(ack). As would be expected this led to the discovery of several
faulty assumptions in Parrot.

auto::timespec

This attempts to determine whether the system defines a struct
timespec by attempting to compile and run a small program that uses
it.

Previously it had been assumed that any platform with threads (i.e.
all platforms on which parrot is tested) had this struct defined,
otherwise not. On platforms that don't support threads, but still
support some standards, this will cause compilation failures.


auto::stat

This attempts to determine whether the system's stat(2) functionality
includes extensions from 4.3BSD over the baseline POSIX requirements
(st_blocks, st_blksize), again by attempting to compile and run a
small program using these features.

Previously, it was assumed that all non-windows platforms supported this.


auto::infnan

This attempts to determine whether the system defines INFINITY and NAN
constants (which will be used if available).

Previously, it was assumed that these constants could be obtained by
dividing values by zero at runtime. This doesn't always work (see TT
#574). SIGFPE from sprintf is confusing to say the least.

This is causing problems because Darwin/PPC does define these
constants, but they do not behave as we expect currently. I am
attempting to tune our expectations appropriately.