Below are my thoughts on this patch in it's current form. 

I don't like the function of ld_libparrot_soname because it has the
soname mixed up with the linker flags.  I'd rather see something like
ld_soflags and libparrot_soname (I don't have strong feelings about the
names).

Building a shared lib and building a static lib shouldn't be an
either/or operation.  I'd like to be able to build and install both
(assuming your platform supports this) and I think this should be the
default behavior.  That means we probably need some CLI flags like
'--disable-shared' & '--disable-static' (just like autoconf).  Although,
anything other than '--libparrot_is_shared' would be an improvement.

There are tabs mixed with whitespace outside of makefiles.  I've been
biting my tongue about this so far but perhaps it's time to start a
thread this on subject.  Anyways, please remove the tabs from your
patch.

Using ExtUtils::Command for MKDIR is a no brainier but a) I think this
change is unrelated to main intent of this patch and b) I'd rather that
${mkdir} is expanded in root.in instead of setting the command
explicitly (I know this is done other places in root.in, it needs to be
cleaned up).  I've committed b) as r10633.

This won't effect one way or the other if the patch is accepted but I'm
trying to encourage people to update their own CREDITS entry.

I'm also pretty sure that the "rpath" concept won't work on many
platforms, including OSX.  I'd feel a lot better about this patch if it
was demonstrated to work on OSX.  In it's current form it causes link
errors.

Thanks for working on this.

Cheers,

-J

--
On Fri, Dec 23, 2005 at 10:10:05AM -1000, Joshua Hoblitt wrote:
> Nick,
> 
> I'll try to take a look at all of this patch today. Quick questions -
> why is:
> 
> +src/install_config.o                              [main]lib
> 
> being added to MANIFEST.generated?
> 
> -J
> 

Attachment: pgpkiSTmC1CJ7.pgp
Description: PGP signature

Reply via email to