Re: [PATCH 5/18] Check if cc is from apple toolchain

[Pavel Roskin]

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.

> -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.

> +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.

> +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.

> +AC_MSG_RESULT([$TARGET_IMG_LDFLAGS_AC])

That's out of place.

> +if test "x$TARGET_APPLE_CC" != x1 ; then
>  grub_PROG_OBJCOPY_ABSOLUTE
> +fi

What's wrong with it?  I actually think that we could drop that test
entirely and use strip instead of objcopy in i386-pc.

> +AC_SUBST(ASFLAGS)

Maybe there is a way to detect Apple by the macros i

> +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?

I thought somebody would review the patches and object, but in this
case, nobody looked :-(

-- 
Regards,
Pavel Roskin