On Tue, Jun 16, 2009 at 3:20 AM, Pavel Roskin <pro...@gnu.org> wrote:

> On Sat, 2009-05-30 at 16:47 +0200, Vladimir 'phcoder' Serbinenko wrote:
>
> > +dnl check if our compiler is apple cc
> > +dnl because it requires numerous workarounds
> > +AC_DEFUN(grub_apple_cc,
>
> We don't use lowercase names for macros.  They could conflict with
> variable names.
>
Thanks you for your comments. Although I usually understand configure
scripts I'm not used to write them.  Actually I used another test as a
template. I'll check which exactly and so we can fix both of them

>
> > -boot_img_LDFLAGS = $(COMMON_LDFLAGS) $(TARGET_IMG_LDFLAGS)
> > -Wl,-Ttext,7C00
> > +boot_img_LDFLAGS = $(COMMON_LDFLAGS) $(TARGET_IMG_LDFLAGS)7C00
>
> This looks like a hack.
>
Otherwise we would have to make separate variables for every link address.
Would you prefer it with separate variables?

>
> > +if test x$grub_cv_apple_target_cc == xyes ; then
>
> Autoconf uses single "=" in test.  I believe old versions of bash would
> not accept "==".  I have applied the fix for that.
>
Thanks

>
> > +else
> > +  TARGET_APPLE_CC=0
> > +# Use linker script if present, otherwise use builtin -N script.
> > +AC_MSG_CHECKING([for option to link raw image])
>
> The formatting is messed up here.
>
Thanks

>
> > +if test "x$TARGET_APPLE_CC" != x1 ; then
> >  grub_PROG_OBJCOPY_ABSOLUTE
> > +fi
>
> What's wrong with it?

There is no objcopy in Apple's toolchain

>
> > +AC_SUBST(ASFLAGS)
>
> Maybe there is a way to detect Apple by the macros i
>
I will look into it

>
> > +ifneq ($(TARGET_APPLE_CC),1)
> >  #{defsym}: #{pre_obj}
> >         $(NM) -g --defined-only -P -p $< | sed 's/^\\([^ ]*\\).*/\\1
> > #{mod_name}/' > $@
> > +else
> > +#{defsym}: #{pre_obj}
> > +       $(NM) -g -P -p $< | grep -E '^[a-zA-Z0-9_]* [TDS]'  | sed 's/^
> > \\([^ ]*\\).*/\\1 #{mod_name}/' > $@
> > +endif
>
> This goes beyond the "Check if cc is from apple toolchain".  It's a
> separate fix that is not explained.  sed can generally do the same
> things as grep.  Is it possible to have an expression that would work
> for GNU and Apple nm?
>
The modification is actually to remove --defined-only argument and the grep
command is here to simulate this --defined-only argument. Perhaps you're
right but imo it's better to use --defined-only rather than grep'ing through
the output and I don't see why because of secondary compiling platform we
should abandon a useful compiling feature on primary platform

>
> I thought somebody would review the patches and object, but in this
> case, nobody looked :-(
>
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to