| Message ID | 1336386538-27684-2-git-send-email-golubovsky@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Dmitry, Please squash this patch into the previous one, and send a single fixed patch. A couple more comments below. On Mon, May 07, 2012 at 06:28:58AM -0400, Dmitry wrote: > * git source URL -> http source URL > * AUTOTARGETS -> GENTARGETS > > Signed-off-by: Dmitry <golubovsky@gmail.com> > --- > package/v86d/v86d.mk | 23 +++++++++++------------ > 1 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/package/v86d/v86d.mk b/package/v86d/v86d.mk > index 035797e..5147a2d 100644 > --- a/package/v86d/v86d.mk > +++ b/package/v86d/v86d.mk > @@ -4,23 +4,22 @@ > # > ############################################ > > -V86D_VERSION = v86d-0.1.10 > -V86D_SITE = git://repo.or.cz/v86d.git > - > -# It is necessary to define __i386__ explicitly for successful compilation. > - > -define V86D_BUILD_CMDS > - $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C $(@D) all > -endef > - > -V86D_POST_CONFIGURE_HOOKS += V86D_REALLY_RUN_CONFIGURE > +V86D_VERSION = 0.1.10 > +V86D_SOURCE = v86d-$(V86D_VERSION).tar.bz2 > +V86D_SITE = http://dev.gentoo.org/~spock/projects/uvesafb/archive > > # v86d's configure script is not autoconf-based: parameters > # supplied by Buildroot cause it to produce no result; explicit > # invocation is needed. Since you don't use Buildroot's autoconf support, this comment is no longer relevant. > > -define V86D_REALLY_RUN_CONFIGURE > +define V86D_CONFIGURE_CMDS > (cd $(@D) ; ./configure --default) > endef > > -$(eval $(call AUTOTARGETS)) > +# It is necessary to define __i386__ explicitly for successful compilation. > + > +define V86D_BUILD_CMDS > + $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C $(@D) all You can used $(TARGET_CONFIGURE_OPTS) here as well. baruch > +endef > + > +$(eval $(call GENTARGETS))
Baruch, On Mon, May 7, 2012 at 7:00 AM, Baruch Siach <baruch@tkos.co.il> wrote: >> # v86d's configure script is not autoconf-based: parameters >> # supplied by Buildroot cause it to produce no result; explicit >> # invocation is needed. > > Since you don't use Buildroot's autoconf support, this comment is no longer > relevant. Well, I thought this would be the explanation why there is a configure script, yet it is a GENTARGET. >> + >> +define V86D_BUILD_CMDS >> + $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C $(@D) all > > You can used $(TARGET_CONFIGURE_OPTS) here as well. I tried that, and it did not compile. -D__i386__ should be given explicitly to cc. Thanks.
Hi Dimitry, On Mon, May 07, 2012 at 08:47:10AM -0400, Dimitry Golubovsky wrote: > On Mon, May 7, 2012 at 7:00 AM, Baruch Siach <baruch@tkos.co.il> wrote: > > >> # v86d's configure script is not autoconf-based: parameters > >> # supplied by Buildroot cause it to produce no result; explicit > >> # invocation is needed. > > > > Since you don't use Buildroot's autoconf support, this comment is no longer > > relevant. > > Well, I thought this would be the explanation why there is a configure > script, yet it is a GENTARGET. The comment needs to be reworded, then. Something like: "v86d's configure script is not autoconf-based, so we use GENTARGET". > >> + > >> +define V86D_BUILD_CMDS > >> + $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C $(@D) all > > > > You can used $(TARGET_CONFIGURE_OPTS) here as well. > > I tried that, and it did not compile. -D__i386__ should be given > explicitly to cc. Ah, you're right. I haven't noticed the quotes delimiting the CC variable. baruch
Le Mon, 7 May 2012 08:47:10 -0400, Dimitry Golubovsky <golubovsky@gmail.com> a écrit : > >> +define V86D_BUILD_CMDS > >> + $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C > >> $(@D) all > > > > You can used $(TARGET_CONFIGURE_OPTS) here as well. > > I tried that, and it did not compile. -D__i386__ should be given > explicitly to cc. This -D is rather a CFLAGS, so: $(MAKE) $(TARGET_CONFIGURE_OPTS) CFLAGS="$(TARGET_CFLAGS) -D__i386__" -C $(@D) all Thomas
OK Thanks, I'll try this way. On Mon, May 7, 2012 at 9:45 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: >> I tried that, and it did not compile. -D__i386__ should be given >> explicitly to cc. > > This -D is rather a CFLAGS, so: > > $(MAKE) $(TARGET_CONFIGURE_OPTS) CFLAGS="$(TARGET_CFLAGS) -D__i386__" -C $(@D) all
Hi Thomas, On Mon, May 07, 2012 at 03:45:41PM +0200, Thomas Petazzoni wrote: > Le Mon, 7 May 2012 08:47:10 -0400, > Dimitry Golubovsky <golubovsky@gmail.com> a écrit : > > > >> +define V86D_BUILD_CMDS > > >> + $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C > > >> $(@D) all > > > > > > You can used $(TARGET_CONFIGURE_OPTS) here as well. > > > > I tried that, and it did not compile. -D__i386__ should be given > > explicitly to cc. > > This -D is rather a CFLAGS, so: > > $(MAKE) $(TARGET_CONFIGURE_OPTS) CFLAGS="$(TARGET_CFLAGS) -D__i386__" -C $(@D) all But then you'll have CFLAGS defined twice. Is this OK? baruch
Le Mon, 7 May 2012 16:55:19 +0300, Baruch Siach <baruch@tkos.co.il> a écrit : > > $(MAKE) $(TARGET_CONFIGURE_OPTS) CFLAGS="$(TARGET_CFLAGS) > > -D__i386__" -C $(@D) all > > But then you'll have CFLAGS defined twice. Is this OK? You'll have a first definition of CFLAGS in TARGET_CONFIGURE_OPTS, overriden by the local redefinition. We do this in numerous places in Buildroot already. Thomas
Hi, On Mon, May 7, 2012 at 9:45 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Le Mon, 7 May 2012 08:47:10 -0400, > Dimitry Golubovsky <golubovsky@gmail.com> a écrit : > >> >> +define V86D_BUILD_CMDS >> >> + $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C >> >> $(@D) all >> > >> > You can used $(TARGET_CONFIGURE_OPTS) here as well. >> >> I tried that, and it did not compile. -D__i386__ should be given >> explicitly to cc. > > This -D is rather a CFLAGS, so: > > $(MAKE) $(TARGET_CONFIGURE_OPTS) CFLAGS="$(TARGET_CFLAGS) -D__i386__" -C $(@D) all > No, it does not work this way. Value of $(CC) with -D__i386__ should be passed deeper down to the lowest level Makefile in order for v86d to compile. /40G2/golubovsky/src/buildroot-patching/buildroot/output/host/usr/bin/i586-unknown-linux-uclibc-gcc -D__i386__ -c -pipe -Os -o fpu.o fpu.c In file included from decode.c:41:0: x86emu/x86emui.h:64:20: fatal error: x86emu.h: No such file or directory (the way you proposed) does not work as local flags are lost. But: /40G2/golubovsky/src/buildroot-patching/buildroot/output/host/usr/bin/i586-unknown-linux-uclibc-gcc -D__i386__ -c -I. -I../../include -I../../include/x86emu -o decode.o decode.c v86_x86emu.c: In function 'v86_init': v86_x86emu.c:50:3: warning: initialization from incompatible pointer type v86_x86emu.c:53:3: warning: initialization from incompatible pointer type v86_x86emu.c:57:3: warning: initialization from incompatible pointer type v86_x86emu.c:58:3: warning: initialization from incompatible pointer type v86_x86emu.c:59:3: warning: initialization from incompatible pointer type v86_x86emu.c:60:3: warning: initialization from incompatible pointer type v86_x86emu.c:61:3: warning: initialization from incompatible pointer type v86_x86emu.c:62:3: warning: initialization from incompatible pointer type (my way) works as it is only $CC that is needed by the lower level Makefile. I'll make other changes as recommended. Thanks.
Hi Thomas, On Mon, May 07, 2012 at 04:04:06PM +0200, Thomas Petazzoni wrote: > Le Mon, 7 May 2012 16:55:19 +0300, > Baruch Siach <baruch@tkos.co.il> a écrit : > > > > $(MAKE) $(TARGET_CONFIGURE_OPTS) CFLAGS="$(TARGET_CFLAGS) > > > -D__i386__" -C $(@D) all > > > > But then you'll have CFLAGS defined twice. Is this OK? > > You'll have a first definition of CFLAGS in TARGET_CONFIGURE_OPTS, > overriden by the local redefinition. We do this in numerous places in > Buildroot already. Is this make behaviour documented? I couldn't find any mention of this in the manual. baruch
diff --git a/package/v86d/v86d.mk b/package/v86d/v86d.mk index 035797e..5147a2d 100644 --- a/package/v86d/v86d.mk +++ b/package/v86d/v86d.mk @@ -4,23 +4,22 @@ # ############################################ -V86D_VERSION = v86d-0.1.10 -V86D_SITE = git://repo.or.cz/v86d.git - -# It is necessary to define __i386__ explicitly for successful compilation. - -define V86D_BUILD_CMDS - $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C $(@D) all -endef - -V86D_POST_CONFIGURE_HOOKS += V86D_REALLY_RUN_CONFIGURE +V86D_VERSION = 0.1.10 +V86D_SOURCE = v86d-$(V86D_VERSION).tar.bz2 +V86D_SITE = http://dev.gentoo.org/~spock/projects/uvesafb/archive # v86d's configure script is not autoconf-based: parameters # supplied by Buildroot cause it to produce no result; explicit # invocation is needed. -define V86D_REALLY_RUN_CONFIGURE +define V86D_CONFIGURE_CMDS (cd $(@D) ; ./configure --default) endef -$(eval $(call AUTOTARGETS)) +# It is necessary to define __i386__ explicitly for successful compilation. + +define V86D_BUILD_CMDS + $(MAKE) CC="$(TARGET_CC) -D__i386__" LD="$(TARGET_LD)" -C $(@D) all +endef + +$(eval $(call GENTARGETS))
* git source URL -> http source URL * AUTOTARGETS -> GENTARGETS Signed-off-by: Dmitry <golubovsky@gmail.com> --- package/v86d/v86d.mk | 23 +++++++++++------------ 1 files changed, 11 insertions(+), 12 deletions(-)