Re: sqlite 3.8.11.1

[Paul Pereira]

> The project goals are simply and plainly incompatible.  Sqlite is
> too much about database featurism, accepting considerable bloat
> into the codebase, while mandoc needs to focus on minimalism and

A good fraction of these changes relate to the FTS5 code, which would
not be compiled in to base.  I would not characterize those changes as
feature creep, but rather a fundamental correction to the previous
search algorithm, which was broken in that it did not scale well to
moderate sized (multi-GB) corpuses with conjunctive queries.  If,
based on compile time flags, the FTS code is disabled, I am not sure
that everything within those pre-processor flags should be considered
when assessing the size of the patch or feature creep.

I cannot comment on the (many) other changes requiring review, except
perhaps the null-pointer bug I found in FTS4.

Re: sqlite 3.8.11.1

[Miod Vallat]

> Hi,
> 
> thanks to the hard work of jturner@, here's a 650kb gzipped update to
> sqlite 3.8.11.1, bumping shlib to 31.0. This is needed for upcoming
> firefox 41 update, but anyone is welcome to look into the update itself.
> 
> Not attaching the diff because of the size, available at
> http://rhaalovely.net/~landry/stuff/sqlite-3.8.11.1.diff.gz

Do we really need to import lib/libsqlite3/ext/fts5/test/ which amounts
for half of the diff?

Re: sqlite 3.8.11.1

[Bob Beck]

Why do we have unreviewable diffs to software in base?



On Wed, Sep 09, 2015 at 10:25:24AM +0200, Landry Breuil wrote:
> Hi,
> 
> thanks to the hard work of jturner@, here's a 650kb gzipped update to
> sqlite 3.8.11.1, bumping shlib to 31.0. This is needed for upcoming
> firefox 41 update, but anyone is welcome to look into the update itself.
> 
> Not attaching the diff because of the size, available at
> http://rhaalovely.net/~landry/stuff/sqlite-3.8.11.1.diff.gz
> 
> Landry
> 

Re: sqlite 3.8.11.1

[Theo de Raadt]

> > Not attaching the diff because of the size, available at
> > http://rhaalovely.net/~landry/stuff/sqlite-3.8.11.1.diff.gz
> 
> And i forgot to mention that it went in an amd64 bulk build without
> fallout, thanks to ajacoutot@

So, one architecture has been tested.  Now there is a request for OK
to commit -- based only upon that one test.

That will put other people in the situation of being first to build on
all the other architectures.  If something breaks during build, then
who has to deal with that?  It seems like potential costs are being
foisted on others.  That is not our way.

I'm sorry, but I'm putting my foot down here.  The folk who wanted
sqlite in the base in the first place have to do sufficient testing.
Owning that part of the tree requires better process than a update
splats.

Since sqlite went into base, the standards are higher.  Meet that
standard.  Test it to same standard we use when doing library changes,
or leave it alone, or prepare to move it back out of base.

sqlite 3.8.11.1

[Landry Breuil]

Hi,

thanks to the hard work of jturner@, here's a 650kb gzipped update to
sqlite 3.8.11.1, bumping shlib to 31.0. This is needed for upcoming
firefox 41 update, but anyone is welcome to look into the update itself.

Not attaching the diff because of the size, available at
http://rhaalovely.net/~landry/stuff/sqlite-3.8.11.1.diff.gz

Landry

Re: sqlite 3.8.11.1

[Theo de Raadt]

This is a really stupid way to treat the base source tree.

> thanks to the hard work of jturner@, here's a 650kb gzipped update to
> sqlite 3.8.11.1, bumping shlib to 31.0. This is needed for upcoming
> firefox 41 update, but anyone is welcome to look into the update itself.
> 
> Not attaching the diff because of the size, available at
> http://rhaalovely.net/~landry/stuff/sqlite-3.8.11.1.diff.gz
> 
> Landry
> 

Re: sqlite 3.8.11.1

[Miod Vallat]

> > When espie@ imported sqlite he wanted to follow upstream so he imported
> > what was distrubuted with sqlite. Since then we do tagged (based on the
> > sqlite version) imports whenever we do an update. So when a diff is sent
> > out it includes all new files in that sqlite release. In this case there
> > is a new fts5 backend which contains a lot of tests (which we never
> > run). We also haven't enabled the fts5 backend at this time.
> > 
> > Now we could change strategies and I could only create a diff of the
> > changes we actually want and then remove all these extra files from our
> > tree and the use commit rather then import going forward.
> > 
> > I would be fine with this as it would make each update more manageable
> > but I'm not sure what espie@ original goals where with following
> > upstream.
> 
> With nsd/unbound we have been removing the unused files. There are
> arguments in either direction of course.

We generally have been removing large chunks of 3rd party code we are
not using, including test suites or operating system specific
directories. We don't want to go as deep as cherry-picking every file,
because that becomes impossible to maintain.

But dropping a 3 megabyte diff, of which more than half is new code
which won't be used, is a bit too much for my taste.

Re: sqlite 3.8.11.1

[Amit Kulkarni]

On Wed, Sep 9, 2015 at 11:12 AM, Miod Vallat  wrote:

> > > When espie@ imported sqlite he wanted to follow upstream so he
> imported
> > > what was distrubuted with sqlite. Since then we do tagged (based on the
> > > sqlite version) imports whenever we do an update. So when a diff is
> sent
> > > out it includes all new files in that sqlite release. In this case
> there
> > > is a new fts5 backend which contains a lot of tests (which we never
> > > run). We also haven't enabled the fts5 backend at this time.
> > >
>

AFAIK, the original rationale for importing sqlite into base was for
storing the database table (INDEX?) for building ports using dpb. It can be
switched to a port module with some pains.

Re: sqlite 3.8.11.1

[Stuart Henderson]

On 2015/09/09 11:47, Amit Kulkarni wrote:
> AFAIK, the original rationale for importing sqlite into base was for
> storing the database table (INDEX?) for building ports using dpb. It
> can be switched to a port module with some pains.

mandoc uses it.

Re: sqlite 3.8.11.1

[Ingo Schwarze]

Hi,

Stuart Henderson wrote on Wed, Sep 09, 2015 at 05:51:18PM +0100:
> On 2015/09/09 11:47, Amit Kulkarni wrote:

>> AFAIK, the original rationale for importing sqlite into base was for
>> storing the database table (INDEX?) for building ports using dpb. It
>> can be switched to a port module with some pains.

> mandoc uses it.

True, or more precisely, man(1), apropos(1), whatis(1), and makewhatis(8)
use it, mandoc(1) itself does not.

That definitely wasn't the original rationale to import sqlite,
though.  If i remember the discussion back then correctly, people
said:

 1. It will be useful for all kinds of cool stuff in base.
Once we have it, people will use it at various places.
 2. Code quality is not too bad.
 3. Code size is not excessive.
 4. The chief developer is very serious and cooperative.

I don't remember anybody challenging any of that at that time, and
i don't remember anybody ever challenging point 4 at any time.  Once
it was in, Kristaps got excited about point 1 above and used it to
replace db(3), and indeed, that solved some minor problems (for
example with byte order and races) and made it marginally easier
to integrate and polish some cool new features.  When i started
work to clean up mandoc-sqlite integration during t2k13, mostly
motivated by the perspective of using a database concept and
implementation less dated than db(3), i didn't question points 2
and 3 above, i neglected to consider whether points 2, 3, and 4 are
sufficient reasons that depending on it does no harm, and i neglected
to do any kind of code audit or review of development speed, goals,
and practices.  Since about s2k14, i bitterly rue that neglect.
I definitely wouldn't use sqlite again for the mandoc toolbox.
The project goals are simply and plainly incompatible.  Sqlite is
too much about database featurism, accepting considerable bloat
into the codebase, while mandoc needs to focus on minimalism and
security and is only harmed by advanced database features and in
particular by performance optimizations complicating the code.

That said, i'm considering half a dozen different options to get
rid of the mandoc dependency on sqlite.  But all of them imply
considerable effort and/or considerable loss of features.
Of course, features lost might get restored later if it comes
to that.

No definite plans exist yet.  Maybe, the dependency on sqlite
might even remain.  Who knows at this point.

Note that i'm just saying this publicly to avoid misunderstandings
about the relationship between mandoc and sqlite.  I will not
participate in a public discussion of these matters, but will discuss
this only among OpenBSD developers, and maybe privately with package
maintainers in other operating systems, for example, but no necessarily
limited to, FreeBSD, pkgsrc, illumos, and Alpine, Void, and Crux
Linux.

Yours,
  Ingo