[PATCH] Remove CPPFLAGS_XEN

[PATCH] Remove CPPFLAGS_XEN

[Olaf Hering]

A grub for xen can be built with CPPFLAGS="-I/path" ./configure, which
has the benefit that CPPFLAGS gets automatically included in the
Makefiles.  There is no need for a private makefile variable, the xen
headers are equal to headers for other optional components.

For some commands the cppflags have to point to POSIX, otherwise
stdint.h will not be found. Previously it was loaded unconditionally
from /usr/include.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 conf/Makefile.common        |  3 ---
 grub-core/Makefile.core.def | 15 ++++++---------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/conf/Makefile.common b/conf/Makefile.common
index 96e58c9..1990005 100644
--- a/conf/Makefile.common
+++ b/conf/Makefile.common
@@ -18,9 +18,6 @@ if COND_arm64
   CFLAGS_PLATFORM += -mcmodel=large
 endif
 
-#FIXME: discover and check XEN headers
-CPPFLAGS_XEN = -I/usr/include
-
 # Other options
 
 CPPFLAGS_DEFAULT = -DGRUB_FILE=\"$(subst $(srcdir)/,,$<)\"
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index a6101de..10754e7 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -59,9 +59,6 @@ kernel = {
   ia64_efi_ldflags = '-Wl,-r,-d';
   ia64_efi_stripflags = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
-  x86_64_xen_cppflags	   = '$(CPPFLAGS_XEN)';
-  i386_xen_cppflags	   = '$(CPPFLAGS_XEN)';
-
   arm_efi_ldflags          = '-Wl,-r,-d';
   arm_efi_stripflags       = '--strip-unneeded -K start -R .note -R .comment -R .note.gnu.gold-version';
 
@@ -604,7 +601,7 @@ module = {
 module = {
   name = lsxen;
   common = commands/xen/lsxen.c;
-  cppflags = '$(CPPFLAGS_XEN)';
+  cppflags = '$(CPPFLAGS_POSIX)';
 
   enable = xen;
 };
@@ -825,7 +822,7 @@ module = {
   i386_coreboot = lib/i386/halt.c;
   i386_qemu = lib/i386/halt.c;
   xen = lib/xen/halt.c;
-  xen_cppflags = '$(CPPFLAGS_XEN)';
+  xen_cppflags = '$(CPPFLAGS_POSIX)';
   efi = lib/efi/halt.c;
   ieee1275 = lib/ieee1275/halt.c;
   emu = lib/emu/halt.c;
@@ -846,7 +843,7 @@ module = {
   mips_loongson = lib/mips/loongson/reboot.c;
   mips_qemu_mips = lib/mips/qemu_mips/reboot.c;
   xen = lib/xen/reboot.c;
-  xen_cppflags = '$(CPPFLAGS_XEN)';
+  xen_cppflags = '$(CPPFLAGS_POSIX)';
   uboot = lib/uboot/reboot.c;
   common = commands/reboot.c;
 };
@@ -1527,7 +1524,7 @@ module = {
   i386_xen = lib/i386/xen/relocator.S;
   x86_64_xen = lib/x86_64/xen/relocator.S;
   xen = lib/i386/relocator_common_c.c;
-  xen_cppflags = '$(CPPFLAGS_XEN)';
+  xen_cppflags = '$(CPPFLAGS_POSIX)';
 
   extra_dist = lib/i386/relocator_common.S;
   extra_dist = kern/powerpc/cache_flush.S;
@@ -1548,7 +1545,7 @@ module = {
   sparc64_ieee1275 = lib/ieee1275/cmos.c;
   powerpc_ieee1275 = lib/ieee1275/cmos.c;
   xen = lib/xen/datetime.c;
-  xen_cppflags = '$(CPPFLAGS_XEN)';
+  xen_cppflags = '$(CPPFLAGS_POSIX)';
 
   mips_arc = lib/arc/datetime.c;
   enable = noemu;
@@ -1651,7 +1648,7 @@ module = {
   name = linux;
   x86 = loader/i386/linux.c;
   xen = loader/i386/xen.c;
-  xen_cppflags	   = '$(CPPFLAGS_XEN)';
+  xen_cppflags = '$(CPPFLAGS_POSIX)';
   i386_pc = lib/i386/pc/vesa_modes_table.c;
   mips = loader/mips/linux.c;
   powerpc_ieee1275 = loader/powerpc/ieee1275/linux.c;

Re: [PATCH] Remove CPPFLAGS_XEN

[Vladimir 'phcoder' Serbinenko]

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

What is the problem you're trying to solve? We shouldn't require user to
pass additional parameters for common cases of we can avoid it
On May 11, 2015 1:13 PM, "Olaf Hering" <olaf@aepfle.de> wrote:

> A grub for xen can be built with CPPFLAGS="-I/path" ./configure, which
> has the benefit that CPPFLAGS gets automatically included in the
> Makefiles.  There is no need for a private makefile variable, the xen
> headers are equal to headers for other optional components.
>
> For some commands the cppflags have to point to POSIX, otherwise
> stdint.h will not be found. Previously it was loaded unconditionally
> from /usr/include.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  conf/Makefile.common        |  3 ---
>  grub-core/Makefile.core.def | 15 ++++++---------
>  2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/conf/Makefile.common b/conf/Makefile.common
> index 96e58c9..1990005 100644
> --- a/conf/Makefile.common
> +++ b/conf/Makefile.common
> @@ -18,9 +18,6 @@ if COND_arm64
>    CFLAGS_PLATFORM += -mcmodel=large
>  endif
>
> -#FIXME: discover and check XEN headers
> -CPPFLAGS_XEN = -I/usr/include
> -
>  # Other options
>
>  CPPFLAGS_DEFAULT = -DGRUB_FILE=\"$(subst $(srcdir)/,,$<)\"
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index a6101de..10754e7 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -59,9 +59,6 @@ kernel = {
>    ia64_efi_ldflags = '-Wl,-r,-d';
>    ia64_efi_stripflags = '--strip-unneeded -K start -R .note -R .comment
> -R .note.gnu.gold-version';
>
> -  x86_64_xen_cppflags     = '$(CPPFLAGS_XEN)';
> -  i386_xen_cppflags       = '$(CPPFLAGS_XEN)';
> -
>    arm_efi_ldflags          = '-Wl,-r,-d';
>    arm_efi_stripflags       = '--strip-unneeded -K start -R .note -R
> .comment -R .note.gnu.gold-version';
>
> @@ -604,7 +601,7 @@ module = {
>  module = {
>    name = lsxen;
>    common = commands/xen/lsxen.c;
> -  cppflags = '$(CPPFLAGS_XEN)';
> +  cppflags = '$(CPPFLAGS_POSIX)';
>
>    enable = xen;
>  };
> @@ -825,7 +822,7 @@ module = {
>    i386_coreboot = lib/i386/halt.c;
>    i386_qemu = lib/i386/halt.c;
>    xen = lib/xen/halt.c;
> -  xen_cppflags = '$(CPPFLAGS_XEN)';
> +  xen_cppflags = '$(CPPFLAGS_POSIX)';
>    efi = lib/efi/halt.c;
>    ieee1275 = lib/ieee1275/halt.c;
>    emu = lib/emu/halt.c;
> @@ -846,7 +843,7 @@ module = {
>    mips_loongson = lib/mips/loongson/reboot.c;
>    mips_qemu_mips = lib/mips/qemu_mips/reboot.c;
>    xen = lib/xen/reboot.c;
> -  xen_cppflags = '$(CPPFLAGS_XEN)';
> +  xen_cppflags = '$(CPPFLAGS_POSIX)';
>    uboot = lib/uboot/reboot.c;
>    common = commands/reboot.c;
>  };
> @@ -1527,7 +1524,7 @@ module = {
>    i386_xen = lib/i386/xen/relocator.S;
>    x86_64_xen = lib/x86_64/xen/relocator.S;
>    xen = lib/i386/relocator_common_c.c;
> -  xen_cppflags = '$(CPPFLAGS_XEN)';
> +  xen_cppflags = '$(CPPFLAGS_POSIX)';
>
>    extra_dist = lib/i386/relocator_common.S;
>    extra_dist = kern/powerpc/cache_flush.S;
> @@ -1548,7 +1545,7 @@ module = {
>    sparc64_ieee1275 = lib/ieee1275/cmos.c;
>    powerpc_ieee1275 = lib/ieee1275/cmos.c;
>    xen = lib/xen/datetime.c;
> -  xen_cppflags = '$(CPPFLAGS_XEN)';
> +  xen_cppflags = '$(CPPFLAGS_POSIX)';
>
>    mips_arc = lib/arc/datetime.c;
>    enable = noemu;
> @@ -1651,7 +1648,7 @@ module = {
>    name = linux;
>    x86 = loader/i386/linux.c;
>    xen = loader/i386/xen.c;
> -  xen_cppflags    = '$(CPPFLAGS_XEN)';
> +  xen_cppflags = '$(CPPFLAGS_POSIX)';
>    i386_pc = lib/i386/pc/vesa_modes_table.c;
>    mips = loader/mips/linux.c;
>    powerpc_ieee1275 = loader/powerpc/ieee1275/linux.c;
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 4790 bytes --]

Re: [PATCH] Remove CPPFLAGS_XEN

[Andrei Borzenkov]

В Mon, 11 May 2015 11:13:18 +0000
Olaf Hering <olaf@aepfle.de> пишет:

> A grub for xen can be built with CPPFLAGS="-I/path" ./configure, which

CPPFLAGS_XEN is target variable, not host. That (if) CPPFLAGS leaks
into target is a bug, not a feature.

Re: [PATCH] Remove CPPFLAGS_XEN

[Olaf Hering]

On Mon, May 11, Vladimir 'phcoder' Serbinenko wrote:

> What is the problem you're trying to solve? We shouldn't require user to pass
> additional parameters for common cases of we can avoid it

My xen headers are in /odd/path, so I first tried
env CPPFLAGS_XEN=/odd/path/include ./configure && make
This failed because CPPFLAGS_XEN is a private make variable. Later I
looked at ./configure --help and noticed the CPPFLAGS variable, wich is
aoppearently the prefered way to specify them.
And the common case is maybe that /usr/include already has the xen
headers, in which case CPPFLAGS_XEN is not needed. I assume the initial
change should have done what my patch does.

Olaf

Re: [PATCH] Remove CPPFLAGS_XEN

[Vladimir 'phcoder' Serbinenko]

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

GRUB is built with -nostd*. So when we want headers from /usr/include we
can't omit including them
On May 11, 2015 1:30 PM, "Olaf Hering" <olaf@aepfle.de> wrote:

> On Mon, May 11, Vladimir 'phcoder' Serbinenko wrote:
>
> > What is the problem you're trying to solve? We shouldn't require user to
> pass
> > additional parameters for common cases of we can avoid it
>
> My xen headers are in /odd/path, so I first tried
> env CPPFLAGS_XEN=/odd/path/include ./configure && make
> This failed because CPPFLAGS_XEN is a private make variable. Later I
> looked at ./configure --help and noticed the CPPFLAGS variable, wich is
> aoppearently the prefered way to specify them.
> And the common case is maybe that /usr/include already has the xen
> headers, in which case CPPFLAGS_XEN is not needed. I assume the initial
> change should have done what my patch does.
>
> Olaf
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 1475 bytes --]

Re: [PATCH] Remove CPPFLAGS_XEN

[Olaf Hering]

On Mon, May 11, Vladimir 'phcoder' Serbinenko wrote:

> GRUB is built with -nostd*. So when we want headers from /usr/include we can't
> omit including them

Likely, but which stdint.h is required for the changed commands? The one
from the system or the one from gnulib?
Looking at configure I see also TARGET_CPPFLAGS and HOST_CPPFLAGS.
Perhaps the former should be used for grub, I will try that.

Olaf

Re: [PATCH] Remove CPPFLAGS_XEN

[Olaf Hering]

On Mon, May 11, Olaf Hering wrote:

> On Mon, May 11, Vladimir 'phcoder' Serbinenko wrote:
> 
> > GRUB is built with -nostd*. So when we want headers from /usr/include we can't
> > omit including them
> Likely, but which stdint.h is required for the changed commands? The one
> from the system or the one from gnulib?

Any word on that? I think my patch is correct.

> Looking at configure I see also TARGET_CPPFLAGS and HOST_CPPFLAGS.
> Perhaps the former should be used for grub, I will try that.

Setting TARGET_CPPFLAGS works.

Olaf

Re: [PATCH] Remove CPPFLAGS_XEN

[Olaf Hering]

On Wed, May 13, Olaf Hering wrote:

> On Mon, May 11, Olaf Hering wrote:
> > Looking at configure I see also TARGET_CPPFLAGS and HOST_CPPFLAGS.
> > Perhaps the former should be used for grub, I will try that.
> Setting TARGET_CPPFLAGS works.

And after poking further at this, should there be a configure option
with sets the path to the xen headers?
I see configure looks for zfs and lzma. But the libs and headers are
appearently used just for host tools. In case of xen/xen.h they are used
only for the target (if that is the right term). It looks like stdint.h
should come from the internal posix_wrap, and xen/xen.h should come from
whatever location is specified.

Another option would be to include the few required xen things into grub
itself. After all thats what the Linux kernel does, the API is not
supposed to change AFAIK. This would remove the need for a configure
knob or an odd private Makefile variable.

Olaf

Re: [PATCH] Remove CPPFLAGS_XEN

[Andrei Borzenkov]

В Wed, 13 May 2015 17:30:41 +0200
Olaf Hering <olaf@aepfle.de> пишет:

> On Wed, May 13, Olaf Hering wrote:
> 
> > On Mon, May 11, Olaf Hering wrote:
> > > Looking at configure I see also TARGET_CPPFLAGS and HOST_CPPFLAGS.
> > > Perhaps the former should be used for grub, I will try that.
> > Setting TARGET_CPPFLAGS works.
> 
> And after poking further at this, should there be a configure option
> with sets the path to the xen headers?
> I see configure looks for zfs and lzma. But the libs and headers are
> appearently used just for host tools. In case of xen/xen.h they are used
> only for the target (if that is the right term). It looks like stdint.h
> should come from the internal posix_wrap, and xen/xen.h should come from
> whatever location is specified.
> 
> Another option would be to include the few required xen things into grub
> itself.

That sounds like the best approach. We really try to have self
contained environment to build target.

>         After all thats what the Linux kernel does, the API is not
> supposed to change AFAIK. This would remove the need for a configure
> knob or an odd private Makefile variable.
> 
> Olaf
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel