Reminder about adding new configuration steps (and other potentially significant code)
James E Keenan
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. 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.
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.
Thank you very much.
Jim Keenan (kid51)
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
Andrew Whitworth
these new configure steps or the reasons why they were implemented.
However, what I do know is that the configuration system is the very
first thing that a potential new Parrot developer will interact with
when trying to build Parrot. If Configure.pl is broken (and it's a
non-trivial system) the developer won't even have a makefile around to
play with, much less any hope of getting things working.
I don't have any real input on the process other than to say that if
it's written in the docs that it be done a certain way, and if Jim
(the lead developer on the tool until this point) says it should be
done a certain way, that sounds to me like the way to do it for now.
How difficult would it be to rip those new configure steps (and any
other code that's dependent on those steps) out of trunk and into a
branch or branches? I know it's been a little while since they went
in, but trunk has been pretty quiet for the last few days so it might
not be a huge undertaking.
We have a release coming up in two weeks. We certainly need trunk as
clean and stable as possible by that point. If these configure steps
absolutely should not be part of trunk if they are not stable by that
point.
Thanks,
--Andrew Whitworth
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
Peter Lobsinger
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.
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
Jim Keenan
Let me start from the end of your post first:
>= 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
[snip]
>
>auto::stat
>
[snip]
>
>auto::infnan
>
[snip]
If you had open Trac tickets for each of these additions and had developed them in a branch first, I would have had no quarrel with your approach. I commend you for trying out Parrot on other OS/platform combinations and would like to see others emulate you in this regard. You have considerable insight into many issues affecting Parrot, and other developers have spoken highly of your work. It's only the process that you followed in this case that troubles me.
Now back to the middle of your post.
>
>While these guidelines seem like a good idea on paper, they may not be
>the best in practice.
Whether they're the best possible guidelines is not at issue here. The fact of the matter is that they are *our current guidelines* and ought to be respected for that reason alone. If you feel they are not appropriate, the remedy is: File a Trac ticket!
>
>List postings are subject to warnocking. Tickets even more so.
That disparages the efforts the efforts of our cage cleaners. We periodically go through old tickets and prod developers to get working on them. In particular, as (FBOW) the de facto maintainer of the configuration system, I would have spotted any tickets you had filed re additional configuration steps and responded promptly to them.
>It is
>difficult recruiting testers for all core platforms (let alone
>best-effort platforms like Darwin/PPC).
I don't see the relevance of that statement to the issues at hand.
>Svn branches are clunky, I've
>seen them fail to execute even a fast-forward merge without conflicts.
>
While many of our developers feel that Subversion branches which touch many source code files are ungainly during merging, any branches you would have created for adding configuration steps would only have touched a few files. Parrot developers who have been around for 3 or more years know that in the past we created many branches to add/modify the configuration system and merged them in successfully.
Moreover, even when we transition to git, direct commits to 'master' (or whatever we'll call it) will not be a good development practice.
[snip]
>
>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.
These changes may have been small in terms of the number of files they touched, but since they were made at the *start* of the Parrot configure-build-test-install chain, they are having considerable downstream impact.
More importantly, while "shoot-first-ask-questions-later" may be the standard development approach in other open source projects, and while it may even have been the standard approach in Parrot before, say, 2007, it is clearly *not* Parrot's current preferred development approach. You will find that if you develop in a branch, then post requests for testing and comments, your fellow Parrot developers -- particularly the cage cleaners -- will respond quickly. And if you open Trac tickets to track particular projects, other developers will be able to handily reference your discussion of the issues in the future.
>So if you feel these changes don't belong,
>you are more than welcome to revert them.
>
As whiteknight pointed out in his post to this thread, we have a supported release coming up next week. My hunch is that we won't have enough time to thoroughly study and fix the inf_nan problems before then. In particular, I won't have very much time to work on the Darwin problems until I arrive in the Pacific Northwest next week. So we should consider reverting those changes. But let's see what #parrotsketch today has to say about that.
In the meantime, would you be willing to retroactively open a Trac ticket for each of the three new config steps?
Jonathan Leto
I just posted a passing smoke test for darwin ppc
http://smolder.parrot.org/app/projects/report_details/454
Duke