Patchwork [1/3] gpsd: proper usage of prefix and DESTDIR

login
register
mail settings
Submitter Thomas Petazzoni
Date July 24, 2012, 10:10 p.m.
Message ID <1343167832-14198-1-git-send-email-thomas.petazzoni@free-electrons.com>
Download mbox | patch
Permalink /patch/173056/
State Accepted
Headers show

Comments

Thomas Petazzoni - July 24, 2012, 10:10 p.m.
prefix should always be /usr, and destdir must be passed as DESTDIR,
and in the environment, not as a scons argument. Finally, we pass the
sysroot= argument to scons so that it doesn't add -L/usr/lib
parameters when compiling.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/gpsd/gpsd.mk |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Simon Dawson - July 25, 2012, 7:53 a.m.
On 24 July 2012 23:10, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> prefix should always be /usr, and destdir must be passed as DESTDIR,
> and in the environment, not as a scons argument. Finally, we pass the
> sysroot= argument to scons so that it doesn't add -L/usr/lib
> parameters when compiling.

Hi Thomas. The build is now falling over for me:

/opt/tfx/arch/arm/usr/bin/ccache /usr/lib/ccache/gcc -pthread -o
strl-py_2_7_2_final_0.so -c -fno-strict-aliasing -O2 -fPIC -DNDEBUG -g
-O3 -Wall -Wstrict-prototypes
-I/opt/tfx/arch/arm/usr/include/python2.7 strl.c
/opt/tfx/arch/arm/usr/bin/ccache /usr/lib/ccache/gcc -pthread -shared
-L/opt/tfx/arch/arm/lib -L/opt/tfx/arch/arm/usr/lib
-Wl,-rpath,/opt/tfx/arch/arm/usr/lib -o gps/clienthelpers.so
gpsclient-py_2_7_2_final_0.so geoid-py_2_7_2_final_0.so
gpsdclient-py_2_7_2_final_0.so strl-py_2_7_2_final_0.so -L.
-L/opt/tfx/arch/arm/usr/arm-unknown-linux-uclibcgnueabi/sysroot/usr/lib
-lrt
/usr/bin/ld: skipping incompatible
/opt/tfx/arch/arm/usr/arm-unknown-linux-uclibcgnueabi/sysroot/usr/lib/librt.so
when searching for -lrt
/usr/bin/ld: skipping incompatible
/opt/tfx/arch/arm/usr/arm-unknown-linux-uclibcgnueabi/sysroot/usr/lib/librt.a
when searching for -lrt
/usr/bin/ld: cannot find /lib/libpthread.so.0 inside
/usr/bin/ld: cannot find /usr/lib/libpthread_nonshared.a inside
collect2: ld returned 1 exit status
scons: *** [gps/clienthelpers.so] Error 1
scons: building terminated because of errors.

> @@ -223,10 +224,9 @@ endef
>  define GPSD_INSTALL_STAGING_CMDS
>         (cd $(@D); \
>                 $(GPSD_SCONS_ENV) \
> +               DESTDIR=$(TARGET_DIR) \
>                 $(SCONS) \
>                 $(GPSD_SCONS_OPTS) \
> -               destdir=$(STAGING_DIR) \
> -               includedir="$(STAGING_DIR)/usr/include" \
>                 install)
>  endef

Should DESTDIR be set to $(STAGING_DIR), rather than $(TARGET_DIR), here?

Simon.
Thomas Petazzoni - July 25, 2012, 8:22 a.m.
Le Wed, 25 Jul 2012 08:53:54 +0100,
Simon Dawson <spdawson@gmail.com> a écrit :

> Hi Thomas. The build is now falling over for me:

This is the Python support of gpsd, correct? Was it building before my
patches?

Because if you look closely:

> /opt/tfx/arch/arm/usr/bin/ccache /usr/lib/ccache/gcc -pthread -o

It is building with the native compiler gcc and not the cross-compiler,
so obviously it cannot work. 

> > @@ -223,10 +224,9 @@ endef
> >  define GPSD_INSTALL_STAGING_CMDS
> >         (cd $(@D); \
> >                 $(GPSD_SCONS_ENV) \
> > +               DESTDIR=$(TARGET_DIR) \
> >                 $(SCONS) \
> >                 $(GPSD_SCONS_OPTS) \
> > -               destdir=$(STAGING_DIR) \
> > -               includedir="$(STAGING_DIR)/usr/include" \
> >                 install)
> >  endef
> 
> Should DESTDIR be set to $(STAGING_DIR), rather than $(TARGET_DIR), here?

Good point, will fix.

Thomas
Simon Dawson - July 25, 2012, 8:49 a.m.
On 25 July 2012 09:22, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> This is the Python support of gpsd, correct? Was it building before my
> patches?
>
> Because if you look closely:
>
>> /opt/tfx/arch/arm/usr/bin/ccache /usr/lib/ccache/gcc -pthread -o
>
> It is building with the native compiler gcc and not the cross-compiler,
> so obviously it cannot work.

Yes, you're right. It was using the native compiler before your
patches, but somehow managed to get through the build without falling
over. Another problem then.

Simon.
Simon Dawson - July 25, 2012, 8:50 a.m.
Acked-by: Simon Dawson <spdawson@gmail.com>
Thomas Petazzoni - July 25, 2012, 8:55 a.m.
Le Wed, 25 Jul 2012 09:49:35 +0100,
Simon Dawson <spdawson@gmail.com> a écrit :

> Yes, you're right. It was using the native compiler before your
> patches, but somehow managed to get through the build without falling
> over. Another problem then.

Can you share a defconfig that shows the problem? I've tested the build
with BR2_PACKAGE_PYTHON + BR2_PACKAGE_GPSD, and it just builds (even
though the output is completely incorrect: the gpsd Python modules are
installed in the wrong location and are built for the host rather than
the target).

Do you actually need the Python support in gpsd? :-)

Thomas
Simon Dawson - July 25, 2012, 10:35 a.m.
Hi Thomas

On 25 July 2012 09:55, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Can you share a defconfig that shows the problem? I've tested the build
> with BR2_PACKAGE_PYTHON + BR2_PACKAGE_GPSD, and it just builds (even
> though the output is completely incorrect: the gpsd Python modules are
> installed in the wrong location and are built for the host rather than
> the target).

BR2_arm=y
BR2_arm926t=y
BR2_TOOLCHAIN_BUILDROOT_LARGEFILE=y
BR2_TOOLCHAIN_BUILDROOT_INET_IPV6=y
BR2_TOOLCHAIN_BUILDROOT_WCHAR=y
BR2_TOOLCHAIN_BUILDROOT_CXX=y
BR2_PACKAGE_MPFR=y
BR2_PACKAGE_GPSD=y
BR2_PACKAGE_GPSD_NTP_SHM=y
BR2_PACKAGE_PYTHON=y
BR2_PACKAGE_PYTHON_PY_PYC=y
BR2_PACKAGE_PYTHON_NFC=y

> Do you actually need the Python support in gpsd? :-)

I think the Python support is needed for gpsfake, gpscat, and a few
other things. For my particular use case, it's not required.

Simon.
Thomas Petazzoni - July 25, 2012, 2:11 p.m.
Le Wed, 25 Jul 2012 11:35:44 +0100,
Simon Dawson <spdawson@gmail.com> a écrit :

> On 25 July 2012 09:55, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > Can you share a defconfig that shows the problem? I've tested the
> > build with BR2_PACKAGE_PYTHON + BR2_PACKAGE_GPSD, and it just
> > builds (even though the output is completely incorrect: the gpsd
> > Python modules are installed in the wrong location and are built
> > for the host rather than the target).
> 
> BR2_arm=y
> BR2_arm926t=y
> BR2_TOOLCHAIN_BUILDROOT_LARGEFILE=y
> BR2_TOOLCHAIN_BUILDROOT_INET_IPV6=y
> BR2_TOOLCHAIN_BUILDROOT_WCHAR=y
> BR2_TOOLCHAIN_BUILDROOT_CXX=y
> BR2_PACKAGE_MPFR=y
> BR2_PACKAGE_GPSD=y
> BR2_PACKAGE_GPSD_NTP_SHM=y
> BR2_PACKAGE_PYTHON=y
> BR2_PACKAGE_PYTHON_PY_PYC=y
> BR2_PACKAGE_PYTHON_NFC=y

This configuration builds fine here. So I guess the difference is that
I build inside a chroot in which I have almost no development package
installed in the host, and you build in your normal development
machine, on which you have many development packages installed, and
those are influencing the build somehow.

For now, I would suggest that we leave the Python support in gpsd on
the side, until someone who actually needs it steps up to fix the
issues. So I'll force python=no unconditionally. Do you agree with this?

Thomas
Simon Dawson - July 25, 2012, 2:27 p.m.
On 25 July 2012 15:11, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> For now, I would suggest that we leave the Python support in gpsd on
> the side, until someone who actually needs it steps up to fix the
> issues. So I'll force python=no unconditionally. Do you agree with this?

Yes, absolutely.

Simon.
Thomas Petazzoni - July 25, 2012, 4:28 p.m.
Le Wed, 25 Jul 2012 00:10:29 +0200,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a écrit :

> prefix should always be /usr, and destdir must be passed as DESTDIR,
> and in the environment, not as a scons argument. Finally, we pass the
> sysroot= argument to scons so that it doesn't add -L/usr/lib
> parameters when compiling.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Applied all 3 patches, with the TARGET_DIR -> STAGING_DIR fix
mentioned by Simon.

Thomas

Patch

diff --git a/package/gpsd/gpsd.mk b/package/gpsd/gpsd.mk
index 7d1dbcd..759cd9c 100644
--- a/package/gpsd/gpsd.mk
+++ b/package/gpsd/gpsd.mk
@@ -17,8 +17,9 @@  GPSD_SCONS_ENV = $(TARGET_CONFIGURE_OPTS)
 
 GPSD_SCONS_OPTS = \
 	arch=$(ARCH)\
-	prefix=$(TARGET_DIR)/usr\
+	prefix=/usr\
 	chrpath=no\
+	sysroot=$(STAGING_DIR)\
 	strip=no
 
 ifeq ($(BR2_PACKAGE_NCURSES),y)
@@ -210,9 +211,9 @@  endef
 define GPSD_INSTALL_TARGET_CMDS
 	(cd $(@D); \
 		$(GPSD_SCONS_ENV) \
+		DESTDIR=$(TARGET_DIR) \
 		$(SCONS) \
 		$(GPSD_SCONS_OPTS) \
-		destdir=$(TARGET_DIR) \
 		install)
 	if [ ! -f $(TARGET_DIR)/etc/init.d/S50gpsd ]; then \
 		$(INSTALL) -m 0755 -D package/gpsd/S50gpsd $(TARGET_DIR)/etc/init.d/S50gpsd; \
@@ -223,10 +224,9 @@  endef
 define GPSD_INSTALL_STAGING_CMDS
 	(cd $(@D); \
 		$(GPSD_SCONS_ENV) \
+		DESTDIR=$(TARGET_DIR) \
 		$(SCONS) \
 		$(GPSD_SCONS_OPTS) \
-		destdir=$(STAGING_DIR) \
-		includedir="$(STAGING_DIR)/usr/include" \
 		install)
 endef