[PATCH] enable buildable targets by default

[PATCH] enable buildable targets by default

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

Hello. We often faced breakages of components which aren't enabled by
default. So Robert Millan proposed to enable all buildable tagets by
default. Here is an implementation. Additionally at the end of output
it writes how exactly grub will be built for easier overview

-- 
Regards
Vladimir 'phcoder' Serbinenko

[-- Attachment #2: newconf.diff --]
[-- Type: text/x-diff, Size: 8306 bytes --]

diff --git a/ChangeLog b/ChangeLog
index f3e631c..f56a119 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2009-06-17  Vladimir Serbinenko  <phcoder@gmail.com>
+
+	Enable all targets that can be built by default
+
+	* configure.c: enable efiemu runtime, grub-emu, grub-emu-usb,
+	grub-mkfont and grub-fstest if they can be built
+
 2009-06-16  James Jarvis  <James.Jarvis@ed.ac.uk>
 
 	* commands/help.c GRUB_MOD_INIT(echo): Fix the help output of
diff --git a/configure.ac b/configure.ac
index 371c32d..8b78cde 100644
--- a/configure.ac
+++ b/configure.ac
@@ -430,6 +430,34 @@ AH_BOTTOM([#if defined(__i386__) && !defined(GRUB_UTIL)
 #define NESTED_FUNC_ATTR
 #endif])
 
+AC_ARG_ENABLE([efiemu],
+	      [AS_HELP_STRING([--enable-efiemu],
+                             [build and install the efiemu runtimes (default=guessed)])])
+if test x"$enable_efiemu" = xno ; then
+  efiemu_excuse="explicitely disabled"
+fi
+if test x"$efiemu_excuse" = x ; then
+  AC_CACHE_CHECK([whether options required for efiemu work], grub_cv_cc_no_red_zone, [
+    CFLAGS="$CFLAGS -m64 -mcmodel=large -mno-red-zone -nostdlib -c"
+    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
+		      [grub_cv_cc_efiemu=yes],
+		      [grub_cv_cc_efiemu=no])
+  ])
+  if test x$grub_cv_cc_efiemu = x$no; then
+     efiemu_excuse="compiler doesn't support compiling with -m64 -mcmodel=large -mno-red-zone -nostdlib -c"
+  fi
+fi
+if test x"$enable_efiemu" = xyes && test x"$efiemu_excuse" != x ; then
+  AC_MSG_ERROR([efiemu runtime was explicitely requested but can't be compiled])
+fi
+if test x"$efiemu_excuse" = x ; then
+enable_efiemu=yes
+else
+enable_efiemu=no
+fi
+AC_SUBST([enable_efiemu])
+
+
 # Restore the flags.
 CC="$tmp_CC"
 CFLAGS="$tmp_CFLAGS"
@@ -450,40 +478,75 @@ AC_ARG_ENABLE([mm-debug],
 
 AC_ARG_ENABLE([grub-emu],
 	      [AS_HELP_STRING([--enable-grub-emu],
-                             [build and install the `grub-emu' debugging utility])])
+                             [build and install the `grub-emu' debugging utility (default=guessed)])])
 AC_ARG_ENABLE([grub-emu-usb],
 	      [AS_HELP_STRING([--enable-grub-emu-usb],
-                             [build and install the `grub-emu' debugging utility with USB support])])
-[if [ x"$enable_grub_emu" = xyes ]; then
-  # Check for curses libraries.]
+                             [build and install the `grub-emu' debugging utility with USB support (default=guessed)])])
+if test x"$enable_grub_emu" = xno ; then
+  grub_emu_excuse="explicitely disabled"
+fi
+
+  [# Check for curses libraries.]
+[if [ x"$grub_emu_excuse" = x ]; then ]
   AC_CHECK_LIB([ncurses], [wgetch], [LIBCURSES="-lncurses"],
     [AC_CHECK_LIB([curses], [wgetch], [LIBCURSES="-lcurses"],
-      [AC_MSG_ERROR([(n)curses libraries are required to build `grub-emu'])])])
+      [grub_emu_excuse=["(n)curses libraries are required to build \`grub-emu'"]])])
   AC_SUBST([LIBCURSES])
-
+[fi]
+[if [ x"$grub_emu_excuse" = x ]; then ]
   [# Check for headers.]
   AC_CHECK_HEADERS([ncurses/curses.h], [],
     [AC_CHECK_HEADERS([ncurses.h], [],
       [AC_CHECK_HEADERS([curses.h], [],
-	[AC_MSG_ERROR([(n)curses header files are required to build `grub-emu'])])])])
+	[grub_emu_excuse=["(n)curses header files are required to build \`grub-emu'"]])])])
+[fi]
 
-  [if [ x"$enable_grub_emu_usb" = xyes ]; then
+if test x"$enable_grub_emu" = xyes && test x"$grub_emu_excuse" != x ; then
+  AC_MSG_ERROR([grub-emu was explicitely requested but can't be compiled])
+fi
+if test x"$grub_emu_excuse" = x ; then
+enable_grub_emu=yes
+else
+enable_grub_emu=no
+grub_emu_usb_excuse="grub-emu isn't built"
+fi
+if test x"$enable_grub_emu_usb" = xno ; then
+  grub_emu_usb_excuse="explicitely disabled"
+fi
+[if [ x"$grub_emu_usb_excuse" = x ]; then
     # Check for libusb libraries.]
-    AC_CHECK_LIB([usb], [usb_claim_interface], [LIBUSB="-lusb"],
-      [AC_MSG_ERROR([libusb libraries are required to build `grub-emu' with USB support])])
+AC_CHECK_LIB([usb], [usb_claim_interface], [LIBUSB="-lusb"],
+    [grub_emu_usb_excuse=["libusb libraries are required to build \`grub-emu' with USB support"]])
     AC_SUBST([LIBUSB])
-
-    [# Check for headers.]
+[fi]
+[if [ x"$grub_emu_usb_excuse" = x ]; then
+    # Check for headers.]
     AC_CHECK_HEADERS([usb.h], [],
-      [AC_MSG_ERROR([libusb header file is required to build `grub-emu' with USB support])])
-  [fi]
+      [grub_emu_usb_excuse=["libusb header file is required to build \`grub-emu' with USB support"]])
 [fi]
+if test x"enable_grub_emu_usb" = xyes && test x"$grub_emu_usb_excuse" != x ; then
+  AC_MSG_ERROR([USB support for grub-emu was explicitely requested but can't be compiled])
+fi
+if test x"$grub_emu_usb_excuse" = x ; then
+enable_grub_emu_usb=yes
+else
+enable_grub_emu_usb=no
+fi
+
 AC_SUBST([enable_grub_emu])
 AC_SUBST([enable_grub_emu_usb])
 
 AC_ARG_ENABLE([grub-fstest],
 	      [AS_HELP_STRING([--enable-grub-fstest],
-                             [build and install the `grub-fstest' debugging utility])])
+                             [build and install the `grub-fstest' debugging utility (default=guessed)])])
+if test x"$enable_grub_fstest" = xno ; then
+  grub_fstest_excuse="explicitely disabled"
+fi
+if test x"$grub_fstest_excuse" = x ; then
+enable_grub_fstest=yes
+else
+enable_grub_fstest=no
+fi
 AC_SUBST([enable_grub_fstest])
 
 AC_ARG_ENABLE([grub-pe2elf],
@@ -493,24 +556,32 @@ AC_SUBST([enable_grub_pe2elf])
 
 AC_ARG_ENABLE([grub-mkfont],
 	      [AS_HELP_STRING([--enable-grub-mkfont],
-                             [build and install the `grub-mkfont' utility])])
-if test x"$enable_grub_mkfont" = xyes ; then
+                             [build and install the `grub-mkfont' utility (default=guessed)])])
+if test x"$enable_grub_mkfont" = xno ; then
+  grub_mkfont_excuse="explicitely disabled"
+fi
+
+if test x"$grub_mkfont_excuse" = x ; then
   # Check for freetype libraries.
   AC_CHECK_PROGS([FREETYPE], [freetype-config])
   if test "x$FREETYPE" = x ; then
-    AC_MSG_ERROR([freetype2 libraries are required to build `grub-mkfont'])
+    grub_mkfont_excuse=["freetype2 libraries are required to build \`grub-mkfont'"]
   fi
   freetype_cflags=`freetype-config --cflags`
   freetype_libs=`freetype-config --libs`
 fi
+if test x"$enable_grub_mkfont" = xyes && test x"$grub_mkfont_excuse" != x ; then
+  AC_MSG_ERROR([grub-mkfont was explicitely requested but can't be compiled])
+fi
+if test x"$grub_mkfont_excuse" = x ; then
+enable_grub_mkfont=yes
+else
+enable_grub_mkfont=no
+fi
 AC_SUBST([enable_grub_mkfont])
 AC_SUBST([freetype_cflags])
 AC_SUBST([freetype_libs])
 
-AC_ARG_ENABLE([efiemu],
-	      [AS_HELP_STRING([--enable-efiemu],
-                             [build and install the efiemu runtimes])])
-AC_SUBST([enable_efiemu])
 AC_SUBST(ASFLAGS)
 
 # Output files.
@@ -528,3 +599,53 @@ fi
 AC_CONFIG_FILES([Makefile gensymlist.sh genkernsyms.sh])
 AC_CONFIG_FILES([stamp-h], [echo timestamp > stamp-h])
 AC_OUTPUT
+[
+echo "*******************************************************"
+echo GRUB2 will be compiled with following components:
+echo Platform: "$target_cpu"-"$platform"
+if test "$target_cpu"-"$platform" = i386-pc; then
+if [ x"$enable_lzo" = xyes ]; then
+echo Compression: LZO
+else
+echo Compression: LZMA
+fi
+fi
+if [ x"$grub_emu_excuse" = x ]; then
+echo grub-emu: Yes
+else
+echo grub-emu: No "($grub_emu_excuse)"
+fi
+if [ x"$grub_emu_usb_excuse" = x ]; then
+echo USB support for grub-emu: Yes
+else
+echo USB support for grub-emu: No "($grub_emu_usb_excuse)"
+fi
+if [ x"$enable_mm_debug" = xyes ]; then
+echo With memory debugging: Yes
+else
+echo With memory debugging: No
+fi
+if [ x"$enable_grub_pe2elf" = xyes ]; then
+echo grub-pe2elf will be built and installed
+elif [ x$TARGET_OBJ2ELF = xgrub-pe2elf ]; then
+echo grub-pe2elf will be built but not installed
+else
+echo grub-pe2elf will not be built
+fi
+if [ x"$efiemu_excuse" = x ]; then
+echo efiemu runtime: Yes
+else
+echo efiemu runtime: No "($efiemu_excuse)"
+fi
+if [ x"$grub_fstest_excuse" = x ]; then
+echo grub-fstest: Yes
+else
+echo grub-fstest: No "($grub_fstest_excuse)"
+fi
+if [ x"$grub_mkfont_excuse" = x ]; then
+echo grub-mkfont: Yes
+else
+echo grub-mkfont: No "($grub_mkfont_excuse)"
+fi
+echo "*******************************************************"
+]

Re: [PATCH] enable buildable targets by default

[Vladimir 'phcoder' Serbinenko]

comitted

On Wed, Jun 17, 2009 at 3:34 PM, Vladimir 'phcoder'
Serbinenko<phcoder@gmail.com> wrote:
> Hello. We often faced breakages of components which aren't enabled by
> default. So Robert Millan proposed to enable all buildable tagets by
> default. Here is an implementation. Additionally at the end of output
> it writes how exactly grub will be built for easier overview
>
> --
> Regards
> Vladimir 'phcoder' Serbinenko
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: [PATCH] enable buildable targets by default

[Pavel Roskin]

Quoting Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>:

> comitted

I wish I had time to review it :-(

I have fixed the easy stuff (spelling, wrong variables, use of "$no",  
use of "-c" in CFLAGS).  The remaining problem is that efiemu only  
compiles on i386-pc, but configure not only checks for the necessary  
flags on other platforms, but also announces that it will be built.

Perhaps we can compile efiemu on i386 and x86_64, but certainly not on  
powerpc and sparc.

Once everything is compiled by default, some of the configure options  
become less meaningful, so we can consider removing them.  It's not so  
important to disable some feature as it is to enable it.

-- 
Regards,
Pavel Roskin

Re: [PATCH] enable buildable targets by default

[Pavel Roskin]

On Fri, 2009-07-17 at 11:21 +0200, Vladimir 'phcoder' Serbinenko wrote:
> On Fri, Jul 17, 2009 at 2:37 AM, Pavel Roskin<proski@gnu.org> wrote:
> > Quoting Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>:
> >
> >> comitted
> >
> > I wish I had time to review it :-(
> >
> > I have fixed the easy stuff (spelling, wrong variables, use of "$no",
> Thanks
> > use of "-c" in CFLAGS).
> This was done on purpose so that we don't need gcc-multilib for tests

AC_COMPILE_IFELSE already means that the program is compiled but not
linked.  I tried removing glibc-devel.i586, and everything is still OK.

> > The remaining problem is that efiemu only compiles on
> > i386-pc, but configure not only checks for the necessary flags on other
> > platforms, but also announces that it will be built.
> Actually the check is necessary only for efiemu64. Perhaps we should
> always build efiemu32 and make grub fallback to efiemu32 even on
> 64-bit platform if efiemu64 is unavailable. Then enable-efiemu would
> be renamed to enable-efiemu64

That's a separate issue.  And then there is a problem that efiemu is
undocumented, so users cannot decide whether they need it or not.

> > Perhaps we can compile efiemu on i386 and x86_64, but certainly not on
> > powerpc and sparc.
> >
> > Once everything is compiled by default, some of the configure options become
> > less meaningful, so we can consider removing them.  It's not so important to
> > disable some feature as it is to enable it.
> IMO if user does some not so useful things it's his problems. It's not
> a reason to limit flexibility

I think we should provide meaningful options.  For example, an option to
disable compiling debugging tools, or an option to add extra sanity
checks.  An option to disable efiemu64 is not meaningful for someone who
doesn't know how to use efiemu64.

-- 
Regards,
Pavel Roskin

Re: [PATCH] enable buildable targets by default

[Robert Millan]

On Fri, Jul 17, 2009 at 10:49:09AM -0400, Pavel Roskin wrote:
> Once everything is compiled by default, some of the configure options
> become less meaningful, so we can consider removing them.
> [...]
> I think we should provide meaningful options.  For example, an option to
> disable compiling debugging tools, or an option to add extra sanity
> checks.  An option to disable efiemu64 is not meaningful for someone who
> doesn't know how to use efiemu64.

I fully agree.  The idea with my proposal (we talked this on IRC, I think)
was to simplify things, so I proposed that we enable everything so we don't
have to provide flags, etc.

I prefer if we got rid of the flags, or at least most of them (lumping them
together with a flag to disable debug tools or so), and remove the summary
message as well.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] enable buildable targets by default

[Vladimir 'phcoder' Serbinenko]

On Sat, Jul 18, 2009 at 7:55 PM, Robert Millan<rmh@aybabtu.com> wrote:
> On Fri, Jul 17, 2009 at 10:49:09AM -0400, Pavel Roskin wrote:
>> Once everything is compiled by default, some of the configure options
>> become less meaningful, so we can consider removing them.
>> [...]
>> I think we should provide meaningful options.  For example, an option to
>> disable compiling debugging tools, or an option to add extra sanity
>> checks.  An option to disable efiemu64 is not meaningful for someone who
>> doesn't know how to use efiemu64.
>
> I fully agree.  The idea with my proposal (we talked this on IRC, I think)
> was to simplify things, so I proposed that we enable everything so we don't
> have to provide flags, etc.
The main pro was to avoid frequent breakage as we experienced with grub-emu.
>
> I prefer if we got rid of the flags, or at least most of them (lumping them
> together with a flag to disable debug tools or so),
I'm undecided on this point. On one hand they can be useful if used
correctly on the other hand if removing them would avoid users doing
errors
> and remove the summary
> message as well.
>
However I really think summary message should remain. It avoids
developpers to scroll through tons of messages to find out if the part
they need is built and if it isn't why
> --
> Robert Millan
>
>  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>  how) you may access your data; but nobody's threatening your freedom: we
>  still allow you to remove your data and not access it at all."
>



--
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: [PATCH] enable buildable targets by default

[Pavel Roskin]

On Sat, 2009-07-18 at 23:38 +0200, Vladimir 'phcoder' Serbinenko wrote:

> > I fully agree.  The idea with my proposal (we talked this on IRC, I think)
> > was to simplify things, so I proposed that we enable everything so we don't
> > have to provide flags, etc.
> The main pro was to avoid frequent breakage as we experienced with grub-emu.

I just use a script to build grub for all supported platforms with all
supported options.  It's quite good for finding such errors.  As it
stands now, there are no errors and no warnings.  But it took a while to
fix everything.

I think we should eventually revert to not building debug tools by
default.

> > I prefer if we got rid of the flags, or at least most of them (lumping them
> > together with a flag to disable debug tools or so),

I agree.  grub-emu, grub-fstest and efiemu are debug tools.  Neither is
needed for normal operation on any platform.

> I'm undecided on this point. On one hand they can be useful if used
> correctly on the other hand if removing them would avoid users doing
> errors

What kind of errors do you mean?

> > and remove the summary
> > message as well.
> >
> However I really think summary message should remain. It avoids
> developpers to scroll through tons of messages to find out if the part
> they need is built and if it isn't why

The summary message is OK with me, be we should keep it short and to the
point.

-- 
Regards,
Pavel Roskin

Re: [PATCH] enable buildable targets by default

[Robert Millan]

On Sat, Jul 18, 2009 at 06:54:49PM -0400, Pavel Roskin wrote:
> On Sat, 2009-07-18 at 23:38 +0200, Vladimir 'phcoder' Serbinenko wrote:
> 
> > > I fully agree.  The idea with my proposal (we talked this on IRC, I think)
> > > was to simplify things, so I proposed that we enable everything so we don't
> > > have to provide flags, etc.
> > The main pro was to avoid frequent breakage as we experienced with grub-emu.
> 
> I just use a script to build grub for all supported platforms with all
> supported options.  It's quite good for finding such errors.  As it
> stands now, there are no errors and no warnings.  But it took a while to
> fix everything.
> 
> I think we should eventually revert to not building debug tools by
> default.

Your script can detect regressions once they're committed, but enabling
them by default helps prevents them from being committed in first place.  I
think it's an advantage.

> > > I prefer if we got rid of the flags, or at least most of them (lumping them
> > > together with a flag to disable debug tools or so),
> 
> I agree.  grub-emu, grub-fstest and efiemu are debug tools.  Neither is
> needed for normal operation on any platform.

I think efiemu is needed for some loaders.  It's not a debug tool AFAIK.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."