Nicolas Williams wrote:
> On Mon, Mar 03, 2008 at 10:39:57AM -0600, Norm Jacobs wrote:
>
>> Nicolas Williams wrote:
>>
>>> On Fri, Feb 29, 2008 at 03:54:39PM -0800, Danek Duvall wrote:
>>>
>>>
>>>> On Fri, Feb 29, 2008 at 04:18:42PM -0600, Nicolas Williams wrote:
>>>>
>>>>
>>>>
>>>>> http://cr.opensolaris.org/~nico//webrev-sqlite3-sfw-the-3rd/
>>>>>
>>>>> - The relevant SQLite3 executables and shared objects are now built
>>>>> differently. Just patch the SQLite3 Makefile.in to build things the
>>>>> Solaris way. This turns out to be a trivial patch by taking
>>>>> advantage of the SQLite3 "amalgamation."
>>>>>
>>>>>
>>>> Is this actually simpler than Norm's supposed one-or-two-line fix? Or did
>>>> that not actually work?
>>>>
>>>>
>>> Norm's fix (I didn't notice a fix) doesn't cause the executable nor the
>>> Tcl bindings to dynamically link with libsqlite3.so -- by default
>>> SQLite3 builds the library into the command and into the bindings.
>>>
>>>
>> Sorry I didn't get back online sooner. I traveling the last half of
>> Friday and opted for spending the weekend with my family. If you patch
>> in these two changes
>>
>> Makefile.in:
>> 595a596
>> > $(LTINSTALL) libtclsqlite3.la `pwd`
>>
>> tclinstall.tcl:
>> 8c8
>> < set LIBFILE .libs/libtclsqlite3[info sharedlibextension]
>> ---
>> > set LIBFILE libtclsqlite3[info sharedlibextension]
>>
>> You can use "make install" and get objects without references to your
>> workspace in them.
>>
>
> If I make no other changes then I don't need to make the above changes
> to avoid getting the .libs directory in the runpath. It is possible
> that the changes you suggest + the changes I was using will do what I
> wanted, but I ended up pursuing a much simpler approach, which you can
> see at the current webrev:
>
> http://cr.opensolaris.org/~nico/webrev-sqlite3-sfw-the-3rd/
>
I guess that the biggest issues with this approach is that it is
relatively foreign to how
many of the other SFW components build. There is something to be said
for attempting
to achieve some uniformity in the gate. I realize that not everything
will fit in the box,
but in cases were it just does (or almost does), it would be better if
the more uniform
approach were taken. However, since you are signing up to support this
in perpetuity,
perhaps this is less of an issue ;-) . Though if you are hit by a bus,
someone is going to
be muttering under their breath.
>
>> As for their build creating a monolithic
>> libtclsqlite3.so and executables, it appears that they are using
>> $(LIBOBJ) in more places than they should.
>>
>
> The SQLite community has a penchant for static linking. For one thing
> it means that you're not dependent on someone else possibly changing
> versions of SQLite on you, and for another it means slightly faster
> code. But we're talking about integrating SQLite for Solaris consumers,
> so dynamically linking with libsqlite3.so is the right answer here.
>
I guess, to me, it seems like more of a flaw in their build system that
should be rectified.
If I would like to build and deliver shared libraries, there is no
reason that anything in
the sqlite code base should be statically linking in the objects already
delivered in it's
own shared libraries. "proper" use of libtool/automake/autoconf should
allow me to
choose whether I want static or dynamic linking, but hey, that's just me
being
unreasonable :-) .
>> I will try to look at your webrev later today.
>>
>
> Thanks.
>
> Nico
>
I tried not to duplicate comments that have already been made.
usr/src/lib/sqlite3/Makefile.sfw:
lines 71-81:
You might put one configure option per line and use
CONFIGURE_OPTIONS += --options
instead of line continuation.
You have two --with-tcl options in your CONFIGURE_OPTIONS
It looks like you may not need the following configure options:
--exec-prefix defaults to {prefix}
--bindir defaults to {exec_prefix}/bin
--libdir defaults to {exec_prefix}/lib
--includedir defaults to {exec_prefix}/include
Do you really want to deliver the extra debugging code via
--enable-debug?
You might set CC, CPPFLAGS, and LDFLAGS in the calling env with the
rest if of the env variables you are passing to configure.
lines 172, 183:
If there isn't a compelling reason to keep the original
Makefile.in, I wouldn't
bother. It can always be extracted from the tarball.
lines 193-200:
You might consider unpacking and patching the source once and simply
copying it from $(VER) to $(VER64).
usr/src/lib/sqlite3/Makefile.in.diff:
Your patch to their build system is Solaris specific. It would be
preferable to
make a more generic set of fixes that you send upstream so that we
have a
chance of not having to create a new patch if/when we resync with
the upstream
project.
line 21:
I would think that you would only need to touch target_source
once when you
are done with your for loop
Enabling the readline support is probably a good thing, but I have two
"issues" with
it. First, your ARC case specifically called out that you would be
excluding this support.
Second, how does this impact the licensing / copyright files that you
are using in the
packages? We may need counsel to weigh in and provide some clarity :-\ .
-Norm
