[Koha-devel] RFC - installer changes

[MJ Ray]

"Galen Charlton" <galen.charlton at liblime.com> wrote:
> On 12/14/07, MJ Ray <mjr at phonecoop.coop> wrote:
> > "Galen Charlton" <galen.charlton at liblime.com> wrote:
> > > On 12/13/07, MJ Ray <mjr at phonecoop.coop> wrote:
> > It doesn't at the moment, but accidents will happen.  _get_value()
> > doesn't detect when it is being run non-interactively and may keep
> > asking prompt() and getting the same invalid default value again and
> > again.  Can we add the supplied default value to the valid values,
> > just to be sure?
>
> Yep.

OK, so will you commit something like this, or shall I?

diff --git a/Makefile.PL b/Makefile.PL
index b3b5002..8050c41 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -849,7 +849,7 @@ sub _get_value {
     }
 
     my $val = prompt($msg, $default);
-    while (exists $valid_values->{$key} and not exists $valid_values->{$key}->{$val}) {
+    while (exists $valid_values->{$key} and not exists $valid_values->{$key}->{$val} and ($val!=$default)) {
         my $retry_msg = "Value '$val' is not a valid option.\n";
         $retry_msg .= "Please enter a value";
         $retry_msg .= _add_valid_values_disp($key, $valid_values);

> > > [...]  That said, I have not done any testing using suexec
> > > mode, and would appreciate information from anybody who has.
> >
> > I'm thinking more of the opac rewriting koha's files if any weakness
> > is found... I need to think more about this and hammer on it, unless
> > anyone else has tested it.
>
> The user owning Koha's files should not be the same as the Apache user
> (e.g., www-data on a default Debian install), and I can add suitably
> loud language to that effect in the installer's prompts.
[...]
> I'm inclined to have the installation instructions tell the installer
> to set up ACLs to allow write access to the affected directories
> rather than recommend use of suexec. [...]

(Can't they just use permissions rather than ACLs?) Anyway, some web
hosts will use suexec, so it would be good if we can make sure koha
plays nice with it, even if not recommended.

> > > Therefore, I'm not in favor of putting C4 in Perl's built-in @INC by
> > > default [...]
> >
> > So how does that comply with the Debian Perl policy, for example?
> > http://www.fr.debian.org/doc/packaging-manuals/perl-policy/ch-perl.html#s-paths
>
> That policy seems to be geared to "library" Perl modules, i.e., those
> intended to be shared by more than one application.  That document
> would be more clear if it addressed the distinction between a shared
> Perl module versus one created simply to organize an app's code.  C4
> falls in the later category; there is no practical reason to install
> the C4 modules absent installation of the rest of Koha.

Maybe not absent the rest of Koha, but other scripts (think contrib)
may want to use them later and it would be cleaner if they don't have
to be installed into koha or with koha-specific details that they
don't need otherwise.  I think C4 is more than simply organising
Koha's code - it defines the API used by other developers.

> There are several Debian packages for Perl web applications that
> follow what I'm suggesting and put the application-specific modules
> under /usr/share/<package>/lib, including request-tracker3.6 (i.e.,
> RT) and cgiirc.  While not all Perl web-apps do this, there is enough
> of a precedent.

I'll be disappointed if packaged koha is as much hassle to install
and get running as RT.  (We're keen RT users, but the debian package
doesn't make life much simpler IME.)

[C4-finding scripts]
> The upshot is that now, after doing an installation, no environment
> variables need to be set to use the command-line tools.

Great.  Why should they still have to be set in Apache on single-Koha
hosts?

> > I don't recall why kohaversion.pl was added in August when
> > $C4::Context::VERSION was already there.  Are we stuck with it?
> > Can we replace it with use C4::Context; return $C4::Context::VERSION?
>
> I don't know the history, but I do like having some kind of
> separation, mostly for the trivial reason that the log for
> C4/Context.pm wouldn't get cluttered with commits that are not really
> related to Context.pm itself.

So someone make C4::Version if they fancy it.  It seemed cleaner to me
for C4::Context (which is loaded by everything, to get the
configuration) to host it.

That said, even while the database is in real flux, C4/Context.pm
has had 25 commits during kohaversion.pl's 37, so probably wouldn't
need committing much more frequently.

> > If we're stuck with it, can we use kohaversion.pl with a versionnumber?
> > Does it mean we have to add "use kohaversion.pl ..." calls to every
> > script, rather than appending the version number to use C4::Context?
>
> No, we can still apply the version check to C4::Context by overriding
> the VERSION method -- we would need to do this anyway, as the VERSION
> imported from UNIVERSAL.pm croaks only if the requested version is
> larger than the module's $VERSION. [...]

How? Are you suggesting the C4::Context module will invoke the
kohaversion.pl script?  So script loads module loads script?
If so, this really seems a strange way to do it.

Why would we need to override the VERSION method?  I thought the idea
was to use only.pm.

Very confused about the confusing VERSION system,
-- 
MJ Ray http://mjr.towers.org.uk/email.html tel:+44-844-4437-237 -
Webmaster-developer, statistician, sysadmin, online shop builder,
consumer and workers co-operative member http://www.ttllp.co.uk/ -
Writing on koha, debian, sat TV, Kewstoke http://mjr.towers.org.uk/