[OPEN-ILS-DEV] Rewrite of build-db.sh

[Aaron Joyner]

On Tue, Jul 15, 2008 at 9:05 AM, Dan Scott <denials at gmail.com> wrote:
>
> 2008/7/15 Aaron Joyner <aaron at joyner.ws>:
> > The attached patch is a rewrite of build-db.sh.  I originally intended to
> > just make it gracefully handle missing files, but I got on a roll.  A break
> > down of changes, by bullet points:
> > - validate that the fts-config.sql script exists for the specified database
> > version
> > - if it does not, detect the latest available version, warn the user,
> > suggest aborting, offer to continue w/ the latest available version
> > - rework call of psql to avoid PGPASSWORD on the command line, visible via
> > ps
> > - warn user if psql fails attempting to import any of these files for
> > reasons such as couldn't connect, or file not found (not sql script errors)
> > - provide commented out option to fail on sql script errors, for future
> > convenience when the scripts run cleanly w/o errors
> > - limit line length to 80 characters (a nearly unbreakable habit from work)
> >
> > There aren't a large number of shell scripts in the depot, the only thing
> > which had a sufficiently large amount of style to go by was
> > Open-ILS/examples/oils_ctl.sh.  I adopted that commenting and indenting
> > style, if that's not an appropriate example, please provide suggestions or
> > pointers to a style guide.  oils_ctl.sh is a bash script, but build-db.sh
> > originally defined #!/bin/sh, so I opted to avoid the few bash style
> > elements from oils_ctl.sh that caught my eye (function foo()..., for
> > example), and leave it as a /bin/sh script, figuring it's better for
> > compatibility.
> >
> > I've endeavoured to follow the submitting doc as closely as possible
> > (http://open-ils.org/documentation/contributing.html#Submitting%20a%20Patch),
> > but if I'm missed anything, or you require any additional information, just
> > let me know.
> >
> > Thanks for the interesting project!  I look forward to contributing more
> > code in the future.
> > Aaron S. Joyner
> >
>
> Hah! This is really great, Aaron - I've thought about overhauling
> build-db.sh a few times but never given it a go.
>
> Sadly, we may have a bit of a collision in one small area, as I
> committed an initial PostgreSQL 8.3 full text search schema to trunk
> about 8 hours ago - so if a user doesn't specify a PostgreSQL version
> for your script, they'll get 8.3 (and probably suffer ignominous
> failure).

Yeah, I saw the patch go in and thought briefly about it.  In general,
the detection code currently isn't intended to come into play unless
there *isn't* a version in place.  So, it won't adversely affect 8.2
users at present, assuming they properly specify a database version,
but if there are no major changes in version 8.4, you won't need to
check in a file in the same way that 8.1 and 8.2 had their own
duplicated version of the file.  On the other hand... that's not too
future proof, it's really just intended to help as a "best guess to
use the last version if one doesn't exist."  Note that it's also
currently coupled with really big warnings.  :)


> The approach I had been thinking about was using the database
> credentials to do an initial login to the database to grab the server
> version (SHOW server_version or SHOW server_version_num, whichever is
> easier to parse) and use that to take the decision out of the user's
> hands. Or at least provide a sanity check.
>
> Would you be interested in taking a crack at refining your patch in
> this fashion?

I'll be glad to implement this additional sanity check.  Do you think
it's worth it to remove the database specification from the user,
detect the database version, and warn on a missing file?  I haven't
looked widely enough to see if the database version is used for
anything else in that build chain, my instinct is that can probably be
torn out, and just auto-detected.  If that sounds good, I'll cook up a
patch to do that, instead.

The more I think abou it, the fewer things the user has to enter
(particularly in the current tediously long build/install process),
the better.  I can't think of any reason why the user would want to
specify a different database version at install time, unless he/she
knows the contents of those scripts, in which case he/she's fully
capable of working around the auto detection.  :)

Aaron S. Joyner