On Wed, 2008-04-23 at 18:40 -0700, James Keenan wrote:
> There are five configuration step classes where the class's runstep()
> method has an internal subroutine called _handle_mswin32(). These
> classes are:
>
> config/auto//crypto.pm
> config/auto//gettext.pm
> config/auto//gmp.pm
> config/auto//opengl.pm
> config/auto//readline.pm
> [...]
> Since this subroutine is declared with the same structure in 5
> different classes, I propose that we refactor it into a method which,
> by analogy with similar subroutines, we would place in
> Parrot::Configure::Step::Methods.
> [...]
> I'll draw up an actual patch in a day or two, but first let me ask if
> this sounds like a good idea or not.
Definitely yes. I had planned to suggest something similar myself,
before getting sidetracked by stomach flu.
Because of the very similar structure between each file, and because
they are all adding flags to the same flag group, we might want to go a
step further. It seems to me that too many hardcoded values appear in
the code blocks, when they should just be in the step's data hash, and
C<Parrot::Configure::Step::Methods> be taught to extract them and do the
right thing:
sub _init {
return {
description => 'Determining if your platform supports OpenGL',
result => '',
test_file => 'config/auto/opengl/opengl.in',
headers => {
macports => 'GL/glut.h',
fink => 'GL/glut.h',
},
libs => {
default => '-lglut -lGLU -lGL',
mswin32_gcc => '-lglut32 -lglu32 -lopengl32',
mswin32 => 'glut.lib glu.lib gl.lib',
darwin => '-framework OpenGL -framework GLUT',
},
};
}
... or something like that. If we standardize on the naming convention
of the F<foo.in> path, we can drop the C<test_file> key and just compute
it.
My happy place is that a library detection config step should only need
to define C<_init>, C<_evaluate_cc_run>, C<_handle_library_found>, and
the C<foo.in> file. An overrideable default implementation of
C<runstep> in the base class should handle all the grotty repeated stuff
for you.
What think you all?
-'f
