Re: ITR: dbishell - QA upload
Hi Neil,
On Mon, Aug 25, 2008 at 18:45, Neil Williams <codehelp@debian.org> wrote:
> On Wed, 2008-08-20 at 23:44 +0200, Sandro Tosi wrote:
>> > Other sponsors have different requirements, these are mine.
>> > ;-)
>>
>> Fine, I just uploaded -9 revision.
>
> I've been trying to test dbishell and the shell syntax is, frankly,
> completely bizarre and deserves to be MUCH more prominently explained in
> the documentation.
>
> Requiring / as the line end character for compatibility with Oracle? WTF?
>
> Then claiming that the char cannot be set to ';' (which most users would
> expect as the line end char in SQL) again for compatibility with Oracle?
> I mean, Oracle is still non-free so why penalise free software users for
> reasons of compatibility with non-free? (It can actually be set to ; the
> claim is that upstream won't do it but then upstream don't seem active
> anymore, this upstream release was first uploaded 2002-04-04).
>
> It's almost enough to make it worth putting this package into contrib!
>
> Completely counter-intuitive. The manpage says this is configurable but
> does not describe how. The README claims that the EOL variable will
> configure the char but dbishell does not listen to the EOL variable in
> the environment. Trying to set EOL as a variable within dbishell does
> not work and the program insists on using / - it does listen to:
> prompt> $EOL=;
> (The '$' prefix is all important.)
>
> That should be included in the manpage - first possible opportunity. I'd
> even suggest that as Oracle is not free, that the Debian package could
> enforce this behaviour as the default and allow those who want to pay
> for Oracle to set their own $EOL variable. Or maybe a more complex patch
> that provides an option to use a more sensible line end char and
> document that in README.Debian and the changelog. The inability to set
> this as an environment variable is a real PITA because it prevents the
> setting of a sane line end char as an alias (hence the idea of a new
> command line option). That could be an alternative patch - one that
> forces DBIShell/UTIL.pm to listen to $ENV["EOL"] and only use the old
> default if nothing is set in the environment.
>
> A patch to DBIShell/UTIL.pm should do the trick (with corresponding
> changes in the manpage and a note in README.Debian as to why this was
> done.
>
> However, if the manpage (and README.Debian) make it clear that the first
> command new users should probably use is:
> prompt> $EOL=;
> then we can avoid confusing those who might already be used to / (ensure
> the docs reinforce that the $ is essential in that command.) Thankfully,
> it does *not* have to be terminated by the old line end char, the value
> of $EOL becomes effective immediately the variable is set with ENTER.
> (This too should probably be in the docs.)
>
> Overall, I'd much prefer a patch that listens to EOL from the
> environment because then users can use EOL in bash aliases:
> alias dbishell-mysql='EOL=; dbishell --dsn host=localhost --driver
> mysql'
> etc.
>
> DSN strings are horrendously complicated and anything that helps users
> get rid of the need to type the blasted things is good news in my book.
>
> This would then be prominently described in the manpage, alongside the
> description of how to reset the EOL once inside dbishell, as above.
>
> Also, there is an Ubuntu bug that you have actually fixed by packaging
> the README - you can close that bug in the changelog with:
> (LP: #195316) IIRC, (although you might want to double-check the
> syntax.) Follow the link from the PTS to see the bug details.
>
> It is probably worthwhile trying to fix #495978 in Debian too. There is
> no good reason to override libs like that.
>
> However, there is a different problem - you haven't actually fixed the
> original bug: #446834
>
> $ grep GENERAL DBIShell/*
> yes, that catches LICENCE which you have omitted, but did you notice
> that DBIShell/UTIL.pm includes a verbatim copy of the GPL as well?
>
> It is worth removing that in your patch to UTIL.pm.
argh! didn't noticed.
> Any reason not to package at least some of the icons? You could add an
> icon to the debian/menu file, seeing as you use a menu and an icon does
> exist.
>
> The manpage should also state that the driver names are case-sensitive.
> That is bound to catch people out.
>
> I'm not sure what is going on with SQLite - it claims SQLite2 when
> Debian has SQLite0 and SQLite3. Have you managed to get dbishell to
> successfully connect to any sqlite database?
No, because I don't have any sqllite db (that I know of ;) ); I only
tested with my personal mysql dbs and it worked fine.
> If SQLite does not work, it should be clearly stated in the docs. If it
> does, but only for sqlite2 (which no longer exists), then either
> dbishell needs to support sqlite3 or all sqlite2 support should be
> removed/disabled and the docs updated to state as much.
Thanks a lot for your really accurate analysis of this tool!!
> Hmmm, quite a lot to do. Sorry about that.
I'm sorry, but I care nothing about this package, and I've already
dedicated to it too much time than what it might deserve. The only
thing I can do it briefly update the manpage to highlight what you've
found and remove the verbatim licence from DBIShell/UTIL.pm .
> If you don't fancy doing it, maybe dbishell should be dropped from
> Debian - orphaned, dead upstream, not a whole lot better than the
> current clients provided by the database programs themselves (in some
> cases, quite a bit worse than current clients), doesn't support
> scripting...
>
> The package claims:
> "It provides all the functionality of a database specific shell such as
> sqlplus or mysql, but in a database independant manner. In some cases,
> it will be significantly more advanced than the command line database
> shell supplied by your database vendor."
>
> Sadly, IMHO, that might have been true in 2001 when this was first
> packaged for Debian - it is very much untrue in 2008, at least as far as
> I can tell. Usual problem - the others are constantly updated by a large
> dedicated upstream team, dbishell is showing overt signs of bit rot
> after SIX years without upstream activity. IMHO it has been superceded
> (and by a noticeable margin).
>
> If you do fancy doing it, maybe you should do this as an ITA and not as
> a QA upload. It would probably mean trying to take over upstream too
> (whereupon you can reconcile that horrible Oracle-compatibility stuff).
> ;-)
>
> Overall, I'm not entirely convinced that dbishell is a particularly
> useful or friendly package - it is rarely better than using whatever
> client the database provides itself, it has (currently) completely
> counter-intuitive syntax compared to the database clients and appears to
> be dead upstream.
>
> Are you sure it is worth keeping it in Debian? I'm not convinced.
I did this QA package (like the others) on a popcon basis: it has ~800
installation, so maybe someone still likes it or use it, or simply
those are old installations and it's not used anylonger.
Feel free to reassign the O: bug to ftp.debian.org as RM: , if you
believe it's the case; if not, just let me know if you want me to do
those simple & fast changes, and I'll reupload it.
Cheers,
--
Sandro Tosi (aka morph, Morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi
Reply to: