grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [PATCH 5/18] Check if cc is from apple toolchain
Date: Tue, 16 Jun 2009 12:44:25 +0200



On Tue, Jun 16, 2009 at 3:20 AM, Pavel Roskin <address@hidden> 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
address@hidden
http://lists.gnu.org/mailman/listinfo/grub-devel



--
Regards
Vladimir 'phcoder' Serbinenko

reply via email to

[Prev in Thread] Current Thread [Next in Thread]