Patchwork Pseudo patch/change, sanity check requested

login
register
mail settings
Submitter Peter Seebach
Date Nov. 5, 2012, 8:05 p.m.
Message ID <20121105140501.78255c6a@e6410-2>
Download mbox | patch
Permalink /patch/38791/
State New
Headers show

Comments

Peter Seebach - Nov. 5, 2012, 8:05 p.m.
Okay, this is a little odd.

We had a build failure of pseudo for a 64-bit target. I looked. I found
an easy fix.

I'm sending this out to the list before I check it in, for one simple
reason: It is absolutely obvious to me that, without this fix, the
--enable-static-sqlite feature should never have worked on 64-bit
systems.

Obviously, it's been in use for months. Which means there's SOMETHING
going on which I don't understand. And I am soliciting feedback because
I am not comfortable checking in a fix I don't understand, and I
recognize that changes to pseudo are relatively high risk.

What I'm looking for:
1. Reports from anyone who's been bitten by this.
2. Feedback on how it could be that the build system has worked despite
the obvious bug.

The bug is simply that --enable-static-sqlite has been hard-coding the
path to $(SQLITE)/lib/libsqlite3.a. This obviously can't work on
systems where the library directory is lib64. So why haven't we seen it
before last week?

-s

---
commit b6c2409d786e53f62accc1c7a6538f39dd0ab601
Author: Peter Seebach <peter.seebach@windriver.com>
Date:   Fri Nov 2 19:55:57 2012 -0500

    Makefile.in/configure: Use $(LIB), not hardcoded lib, for sqlite
    
    It turns out that the -L usage mostly doesn't matter (usually something
    else has requested the right directory, or it's the default), but the
    explicit path to libsqlite3.a hardcoded "lib", and on some systems
    it should be something else, such as "lib64". Solution: Use $(LIB) for
    that directory.
    
    Note that this may not resolve things if, say, you're doing MIPS n32 on a
    target where that lives in /usr/lib32, but I think in that case you'd
    be specifying $libdir, so it should still work out.
    
    Also added --with-static-sqlite=/path as an option in case people need
    to further outsmart this.
    
    Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
Mark Hatle - Nov. 6, 2012, 9:47 a.m.
On 11/5/12 9:05 PM, Peter Seebach wrote:
> Okay, this is a little odd.
>
> We had a build failure of pseudo for a 64-bit target. I looked. I found
> an easy fix.
>
> I'm sending this out to the list before I check it in, for one simple
> reason: It is absolutely obvious to me that, without this fix, the
> --enable-static-sqlite feature should never have worked on 64-bit
> systems.
>
> Obviously, it's been in use for months. Which means there's SOMETHING
> going on which I don't understand. And I am soliciting feedback because
> I am not comfortable checking in a fix I don't understand, and I
> recognize that changes to pseudo are relatively high risk.
>
> What I'm looking for:
> 1. Reports from anyone who's been bitten by this.
> 2. Feedback on how it could be that the build system has worked despite
> the obvious bug.
>
> The bug is simply that --enable-static-sqlite has been hard-coding the
> path to $(SQLITE)/lib/libsqlite3.a. This obviously can't work on
> systems where the library directory is lib64. So why haven't we seen it
> before last week?

Within OpenEmbedded-core, we build a local version of sqlite.  This local
version (native sysroot) always lives in the 'lib' directory path at this time.
   If the local sqlite is not built, then pseudo should not have been built.

--Mark


> -s
>
> ---
> commit b6c2409d786e53f62accc1c7a6538f39dd0ab601
> Author: Peter Seebach <peter.seebach@windriver.com>
> Date:   Fri Nov 2 19:55:57 2012 -0500
>
>      Makefile.in/configure: Use $(LIB), not hardcoded lib, for sqlite
>
>      It turns out that the -L usage mostly doesn't matter (usually something
>      else has requested the right directory, or it's the default), but the
>      explicit path to libsqlite3.a hardcoded "lib", and on some systems
>      it should be something else, such as "lib64". Solution: Use $(LIB) for
>      that directory.
>
>      Note that this may not resolve things if, say, you're doing MIPS n32 on a
>      target where that lives in /usr/lib32, but I think in that case you'd
>      be specifying $libdir, so it should still work out.
>
>      Also added --with-static-sqlite=/path as an option in case people need
>      to further outsmart this.
>
>      Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
>
> diff --git a/Makefile.in b/Makefile.in
> index 83fc89f..56ea5e2 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -38,7 +38,7 @@ CFLAGS_BASE=-pipe -std=gnu99 -Wall -W -Wextra
>   CFLAGS_CODE=-fPIC -D_LARGEFILE64_SOURCE -D_ATFILE_SOURCE $(ARCH_FLAGS)
>   CFLAGS_DEFS=-DPSEUDO_PREFIX='"$(PREFIX)"' -DPSEUDO_SUFFIX='"$(SUFFIX)"' -DPSEUDO_BINDIR='"$(BIN)"' -DPSEUDO_LIBDIR='"$(LIB)"' -DPSEUDO_LOCALSTATEDIR='"$(LOCALSTATE)"' -DPSEUDO_VERSION='"$(VERSION)"'
>   CFLAGS_DEBUG=-O2 -g
> -CFLAGS_SQL=-L$(SQLITE)/lib -I$(SQLITE)/include $(RPATH)
> +CFLAGS_SQL=-L$(SQLITE)/$(LIB) -I$(SQLITE)/include $(RPATH)
>   CFLAGS_PSEUDO=$(CFLAGS_BASE) $(CFLAGS_CODE) $(CFLAGS_DEFS) \
>   	$(CFLAGS_DEBUG) $(CFLAGS_SQL)
>
> diff --git a/configure b/configure
> index 4a41943..c536f58 100755
> --- a/configure
> +++ b/configure
> @@ -37,6 +37,7 @@ usage()
>       echo >&2 "           [--suffix=...]"
>       echo >&2 "           [--with-sqlite=...]"
>       echo >&2 "           [--enable-static-sqlite]"
> +    echo >&2 "           [--with-static-sqlite=...]"
>       echo >&2 "           [--with-rpath=...|--without-rpath]"
>       echo >&2 "           [--cflags='']"
>       echo >&2 "           [--bits=32|64]"
> @@ -58,14 +59,19 @@ do
>       --libdir=*)
>           opt_libdir=${arg#--libdir=}
>           ;;
> +    --with-static-sqlite=*)
> +        opt_sqlite_ldarg=${arg#--with-static-sqlite=}
> +        sqlite_ldarg=$opt_sqlite_ldarg
> +        use_maybe_rpath=false
> +        ;;
>       --enable-static-sqlite)
> -        sqlite_ldarg='$(SQLITE)/lib/libsqlite3.a'
> +        sqlite_ldarg='$(SQLITE)/$(LIB)/libsqlite3.a'
>           use_maybe_rpath=false
>           ;;
>       --with-sqlite=*)
>           opt_sqlite=${arg#--with-sqlite=}
>           # assign new value if unset
> -        maybe_rpath='-Wl,-R$(SQLITE)/lib'
> +        maybe_rpath='-Wl,-R$(SQLITE)/$(LIB)'
>           ;;
>       --without-rpath)
>           opt_rpath=''
>

Patch

diff --git a/Makefile.in b/Makefile.in
index 83fc89f..56ea5e2 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -38,7 +38,7 @@  CFLAGS_BASE=-pipe -std=gnu99 -Wall -W -Wextra
 CFLAGS_CODE=-fPIC -D_LARGEFILE64_SOURCE -D_ATFILE_SOURCE $(ARCH_FLAGS)
 CFLAGS_DEFS=-DPSEUDO_PREFIX='"$(PREFIX)"' -DPSEUDO_SUFFIX='"$(SUFFIX)"' -DPSEUDO_BINDIR='"$(BIN)"' -DPSEUDO_LIBDIR='"$(LIB)"' -DPSEUDO_LOCALSTATEDIR='"$(LOCALSTATE)"' -DPSEUDO_VERSION='"$(VERSION)"'
 CFLAGS_DEBUG=-O2 -g
-CFLAGS_SQL=-L$(SQLITE)/lib -I$(SQLITE)/include $(RPATH)
+CFLAGS_SQL=-L$(SQLITE)/$(LIB) -I$(SQLITE)/include $(RPATH)
 CFLAGS_PSEUDO=$(CFLAGS_BASE) $(CFLAGS_CODE) $(CFLAGS_DEFS) \
 	$(CFLAGS_DEBUG) $(CFLAGS_SQL)
 
diff --git a/configure b/configure
index 4a41943..c536f58 100755
--- a/configure
+++ b/configure
@@ -37,6 +37,7 @@  usage()
     echo >&2 "           [--suffix=...]"
     echo >&2 "           [--with-sqlite=...]"
     echo >&2 "           [--enable-static-sqlite]"
+    echo >&2 "           [--with-static-sqlite=...]"
     echo >&2 "           [--with-rpath=...|--without-rpath]"
     echo >&2 "           [--cflags='']"
     echo >&2 "           [--bits=32|64]"
@@ -58,14 +59,19 @@  do
     --libdir=*)
         opt_libdir=${arg#--libdir=}
         ;;
+    --with-static-sqlite=*)
+        opt_sqlite_ldarg=${arg#--with-static-sqlite=}
+        sqlite_ldarg=$opt_sqlite_ldarg
+        use_maybe_rpath=false
+        ;;
     --enable-static-sqlite)
-        sqlite_ldarg='$(SQLITE)/lib/libsqlite3.a'
+        sqlite_ldarg='$(SQLITE)/$(LIB)/libsqlite3.a'
         use_maybe_rpath=false
         ;;
     --with-sqlite=*)
         opt_sqlite=${arg#--with-sqlite=}
         # assign new value if unset
-        maybe_rpath='-Wl,-R$(SQLITE)/lib'
+        maybe_rpath='-Wl,-R$(SQLITE)/$(LIB)'
         ;;
     --without-rpath)
         opt_rpath=''