move config outside of main file

[Former user]

Originally reported on Google Code with ID 181

I have suggestion to move config outside of the main script and use logic of 2 configs
: sample and local user's

└───phpliteAdmin
        phpliteadmin.config.php
        phpliteadmin.config.sample.php
        phpliteadmin.php
        README.txt


where config will contain all data between

//BEGIN USER-DEFINED VARIABLES
//////////////////////////////
and
////////////////////////////
//END USER-DEFINED VARIABLES

Reported by ykorotia on 2013-02-26 11:23:51

<hr> * Attachment: config.png<br>

[Christopher Kramer]

Yes, clearly has advantages.
Currently we don't do it because of the "only 1 file you can move around"-approach.
But perhaps we should give this another thought.

I see these advantages:
- update is easier
- user sees the place to edit config more easily
- users are used to config-files like this

I see these disadvantages:
- breaking the 1-file-approach a bit
- you need to move the config-file when you move the source-file

I am wondering if it makes sense to make this optional. I guess not.

Overall, I think the advantages are higher and we should go for it.
Any other opinions?

Reported by crazy4chrissi on 2013-02-26 11:46:58

[Former user]

it's good you stick with 1 file. here's another way, with embeded config, but with extended
local if available.

1 bad thing is - if new parameter is introduced we won't be notified.

By now the best way is array/config merging, which is a pain to add here. But again,
it will give people ability to work with trunk version and make patches easier

Reported by ykorotia on 2013-02-26 12:18:00

<hr> * Attachment: on-demand.png<br>

[Former user]

"1 bad thing is - if new parameter is introduced we won't be notified."
can be fixed by mantaining sample config file which can be removed if user wants

Reported by ykorotia on 2013-02-26 12:25:12

<hr> * Attachment: on-demand-with-sample.png<br>

[dreadnaut]

I wrote extensively about this in a (sad and lonely!) post on the mailing list.

In short, an optional external configuration file would be great, but in my opinion
these points are important:
- it should not be php code (user friendly, does not break in case of syntax errors)
- it should be writable from PLA (to fix theme or other options, better than cookies)
- it should be easy to revert/extract defaults from the code (failsafe, user friendly)
- overriding defaults should be value-by-value, not "either we use the internal config
or the external one" (missing value should not break PLA, also less trust in external
files)
- there should be a clear convention regarding which configuration values have priority,
depending on their source (defaults, config, session, cookies)

Reported by dreadnaut on 2013-02-26 15:11:13

[dreadnaut]

Whops, I didn't want to remove the CC: names —reverted.

Reported by dreadnaut on 2013-02-26 15:14:00

[Former user]

not sure what you mean under user friendly. This project use devs/admins, I've never
seen regular user who manages database. Even if user, right now he changes this config
in php code. hehe

what is PLA?

any "failsafe" code generates hundreds lines of code.

about value by value - it is done by array merge. Config will be in php, but who really
cares? XML is not easier


full config substitution is the easiest way (so I did it for myself already locally).
Array merge requires user variables to be refactored (aka grouped, renamed, etc..).
Also how are you going to manage user's functions?

Reported by ykorotia on 2013-02-26 15:20:44

[dreadnaut]

PLA is phpLiteAdmin, and phpLiteAdmin's users are indeed developers and admins. They
are still called 'users' though :)

"Failsafe" just means that the program will not stop or break if its configuration
is broken or missing. It's just a matter of having defaults and not relying on parsing
php —which can be done in about ten lines, not "hundreds".

User functions are another issue, as they have to be php. I'd like to see a survey
on their use.

Anyway, go read my post for more details maybe, the group archives are open:
http://groups.google.com/group/phpliteadmin/

Reported by dreadnaut on 2013-02-26 15:36:48

[Christopher Kramer]

Sorry dreadnaut. I had in mind that somebody had already requested this, but could not
find an issue about it. And I forgot it was in the mailing list. (So I just added some
guys to CC who I thought might have mentioned this earlier)
And sorry nobody answered in the Mailing list...

So probably it's now time to discuss it. So we all agree that having external configuration
files would be great (at least optional).

Now the question is whether we go for a simple php-file or something like ini-files,
xml-files or something custom.

I think all approaches have advantages and disadvantages. So let's discuss them.

"User friendly" really depends on who the user is. And as ykorotia wrote, PLA's users
are mainly php-developers. So they should be quite comfortable with php-files. PhpMyAdmin
uses a php-config-file like this as well, and they have similar users ;-)

Personally, I feel more comfortable with a php-config-file than an ini-file for example.
Because I am 100% sure how php-syntax works. With ini-files, I always wonder how white-space
is treated (does it matter if I add a tab here? how can I add a line-break in a value?),
how comments can be done (# or ; in here?) and so on.
Of course we could also argue php-developers & admins should know ini-files from php.ini

I agree that adding 100s of LOC for config handling is not appropriate in this project,
but PHP comes with pretty much everything we need to code ini (parse_ini_file), xml
or even our own custom format in a small bunch of lines.

"Should be writable from PLA". Hmm. Yes, agreed. Could also be done with php-based
config-files in theory, but this is clearly in favour of other approaches like ini.

"should be easy to revert/extract defaults from the code"
No problem with any of the solutions in my opinion.

"overriding defaults should be value-by-value"
No problem with any of the solutions in my opinion. If we continue using php variables,
we could simply override default values with included user-values.

"there should be a clear convention regarding which configuration values have priority,
depending on their source"
I guess this priority is obvious and therefore clear with any approach (from user perspective).
From developer perspective, it would be better to encapsulate this so we don't have
to check session, cookie and so on manually every time. Agreed.

Reported by crazy4chrissi on 2013-02-26 18:09:01

[Christopher Kramer]

Ah, another thing that came to my mind:

We could store settings in a SQLite-DB. Just to discuss, I don't really like this solution
very much myself ;-)

Reported by crazy4chrissi on 2013-02-26 18:12:28

[dreadnaut]

Come on, let's not make it necessary to use PLA to edit PLA's settings :)  A text editor
should be enough.

Ideally, *nano* (or notepad) should be enough —the basic tools you will have on a computer,
through ssh, on the go, on a server with little software installed, on a NAS or mobile,
etc.  Keep It Simple is always true.

Reported by dreadnaut on 2013-02-26 18:16:29

[Christopher Kramer]

haha ;-)
Sorry, maybe just forget my last post, it was more meant to provoke ;-)
(would make it pretty hard to reset the password for example ;-) )

Reported by crazy4chrissi on 2013-02-26 18:19:27

[Christopher Kramer]

So I think we should either go for ini-files or php-files.
I don't think a custom-solution is a good idea. Always error-prone to reinvent the
wheel and users get unsure about how to use it.

So currently I think ini-files might probably be the best solution for PLA.

Some more suggestions just for completeness:
- use Pear Config: http://pear.php.net/package/Config
Problem: Dependency on a comparably huge library. Pear often not installed.
- use JSON
Not so pretty (and user-friendly), but really nice to handle from PHP and other languages.

Another argument for the php-based solution:
It makes it possible to code a dynamic configuration. You could for example imagine
passwords, languages or database-paths coming from some database. You could code this
easily with some lines of php code.
With a configuration-file like an ini-file, you'd need to overwrite the ini-file from
a cronjob or something like this.
(Not an important advantage because lot's of users don't do this kind of things, but
I like flexible systems...)

Reported by crazy4chrissi on 2013-02-26 18:34:00

[dreadnaut]

As, sorry, I took it too seriously :-p

And shocked by your message, I had somehow missed your previous post above O_O. I'll
clarify just one point in response to that and then wait for what the others have to
say ( please say something! anything! :) )

| "Should be writable from PLA". Hmm. Yes, agreed. Could also be done with
| php-based config-files in theory, but this is clearly in favour of other
| approaches like ini.

Php has var_export() which makes .php config files as easy as .ini ones. I don't like
.ini files myself, nor the way parse_ini_files() behaves in many cases (I consider
it broken and usually avoid it in my projects).

The main reason why I'd prefer a "non-php config" is error handling: incorrect code
might give warnings, return null values, or results in Errors that stop execution —all
stuff that we will have trouble handling. Even worse, these errors might be **invisible**
depending on the local php configuration, and result in unexpected behaviour or a nice
white page.

These are issues which would be present in the current version too, but I expect an
external file to be more vulnerable.

Plus, the fact the current users are programmers doesn't mean that we should keep it
difficult, if we don't need to. That would only ensure that future users will STILL
be programmers, it doesn't exactly open up to less experienced users.

In any case, I suppose the questions are "do we need *php* config files? does it make
things safer/easier/better along any dimensions? is there anything better?" disregarding
the fact that we like them more or less :)

Reported by dreadnaut on 2013-02-26 18:55:56

[Former user]

that's how I see it

Reported by ykorotia on 2013-02-26 18:56:24

<hr> * Attachment: config-merge.png<br> * Attachment: sqliteadmin2.php * Attachment: config.php * Attachment: config-merge(Echo).png<br>

[Christopher Kramer]

> The main reason why I'd prefer a "non-php config" is error handling: incorrect code

> might give warnings, return null values, or results in Errors that stop execution

> —all stuff that we will have trouble handling. Even worse, these errors might be

> **invisible** depending on the local php configuration, and result in unexpected

> behaviour or a nice white page
Yes, that's correct. I mean, we could turn display_errors on before including the external
file and off afterwards to make config-errors appear, but this only solves part of
the issue.

> "do we need *php* config files? does it make things safer/easier/better along any

> dimensions? is there anything better?

Safer: Yes and No. "No" first: I think php-config files make things less safe, especially
if the file is writable. var_export() makes it quite safe what *we* write, but we still
have an executable file which is writable. Some security issue (of another script for
example) could make it possible to write some code into it and execute it. This is
not possible with ini-files or other non-executable files.

"Yes, php-files are safer": PHP-Code is executed when opened in a browser, not shown.
Other files such as ini-files are shown. This is problematic as we store passwords
and paths in there. So we cannot simply put some ini-file there without protection.
And protection requires .htaccess to work for example. This would make another file,
and .htacess doesn't work on lots of hosts. Clearly an issue we need to discuss.

Easier: Yes, I think so. Basically, it's an include to read and some var_exports (in
a loop) to write. And as you said, parse_ini_file is not so pretty and there is no
out-of-the-box php-function to write ini-files as far as I know, so writing would require
some lines of code.

Better: Well, that's what we discuss.

@ykorotia: What's the advantage of arrays you think is important here? I mean, compared
to variables.

Reported by crazy4chrissi on 2013-02-26 19:17:13

[Former user]

@it possible to write some code into it and execute it@
hehe. if admin is not noob he will give only read access to file by filesystem and
that's it. if admin is noob, why should you care?

@What's the advantage of arrays you think is important here? I mean, compared to variables.@

MERGING .. and PHP is arrays, you don't have to parse anything, it's NATIVE, while
all other formats are aliens.

INI files are kind of dinosaurs, it's MS-DOS.. are you serious about it, or it's kind
of joke I don't know?

Reported by ykorotia on 2013-02-26 20:24:17

[dreadnaut]

If we end up encapsulating everything in a class, we will probably have a configuration
arrays anyway. Or more than one (defaults, config, $_COOKIE, $_SESSION) which are merged
depending on priority. It also reduces writing to a single var_export() and makes reading
easier (include-d files are executed in the current scope; unless we include the file
in the global scope, created variables will not be global and things become more complicated...)

Also, Just an idea for a possible hybrid .php solution which gives no output when run
directly (except an error). Not very clean IMO, but at least we know it's possible.

-----
<?php return PLAConfig::load(__FILE__); ?>

# non-php config follows
password     12345
directory    dbs/
----- 

Reported by dreadnaut on 2013-02-26 20:30:44

[Former user]

Can you explain how it is going to work?

Reported by ykorotia on 2013-02-26 20:36:37

[dreadnaut]

Only these two points are important:
- if you 'return' from an included file, the remaining line of the file are not executed
and therefore not sent to output;
- if you run the file without including it, php will halt execution because the class
does not exist, sending only a "Fatal error" to output.

You can then build a .php file which contains non-php configuration.

Reported by dreadnaut on 2013-02-26 20:43:22

[Christopher Kramer]

> @it possible to write some code into it and execute it@
> hehe. if admin is not noob he will give only read access to file by filesystem and

> that's it. if admin is noob, why should you care?
Because we were talking about being able to write to the file from PLA. So you could
change a setting in PLA's GUI and save the configuration to the file. And then we need
to make it writable, no matter how smart the admin is.

> @What's the advantage of arrays you think is important here? I mean, compared to
variables.@
> MERGING .. and PHP is arrays, you don't have to parse anything, it's NATIVE, while

> all other formats are aliens.
I meant PHP-arrays vs. PHP variables. You don't even need to merge anything with variables.
You just load them in the correct order and if they are defined in the included file,
they will overwrite the default ones. If one is not defined, the default is used. So
merging is rather a disadvantage of arrays because it is overhead (which is not really
true either, you can use overwriting if you use the right syntax as well). Variables
are native as well.

> INI files are kind of dinosaurs, it's MS-DOS.. are you serious about it, or it's

> kind of joke I don't know?
No, I was not joking about inifiles. Arrays are dinosours as well. Just because something
is old, doesn't necessarily mean it's bad. If we argue like that, using php-files is
the wrong way as well, we would need to store the config as xml in the cloud just to
be up-to-date.

@dreadnaut:
Hmm. And in PLAConfig:load(), you parse this file? Why don't you just place a <?php
exit; ?> in the first line and don't include it at all, but parse it straight away?

But I think people will get confused if the file-extension is .php and the content
actually is something we parse ourself.

Reported by crazy4chrissi on 2013-02-26 21:51:02

[dreadnaut]

@chris: nevermind, it was an intricate example of something that could be parsed, included,
be php, be non-php, etc.

Anyway, I slept on it, and changed my mind a bit.

1) I realised that I *hate* when software overwrites my configuration files: comments
are gone, the ordering messed up, etc. We should let configuration files be what they
are, and that is read-only; we can find another place to store data, e.g. a different
phpliteadmin.data.php.

2) Configuration should be a .php file for security and therefore php code; we can
make it simple and readable anyway. Variables would probably be less error prone than
arrays, and we can use something like compact() if we want to handle them as arrays.

We can start by simply including an optional external file, and later improve on the
internal interface to configuration and conf. source preference that I mentioned previously.

Reported by dreadnaut on 2013-02-27 13:23:28

[Christopher Kramer]

Okay.
I can live very well with a read-only php-file included defining config-variables.
So we basically only move the declarations we already have into an external file. That's
more or less what ykorotia initially proposed ;-)

But I think having a sample and a real config-file is not necessary.
I think we should just provide a config-file along with PLA and users edit this config
file. If they mess it up, they can simply download the original config file again.
And if we introduce new config variables in new versions, we check with isset() if
they are there and print a message if they are missing (and use a default as long as
they are missing).

Everyone agrees?

Reported by crazy4chrissi on 2013-02-27 14:39:23

[dreadnaut]

Since we are keeping the config section inside the main file that can be used as sample;
however, it is common to have an external sample file to copy or for reference. I have
no preference though.

For the external config file, I would avoid checking with isset() and allow for partial
configuration. People will probably be happy with default values, but they might have
a two-lines external config with custom values, e.g.

<?php
$directory = 'dbs/';
$password = '12345';

If new variables are introduced, people will notice them if/when they need them. We
just need to write decent release notes :)

Reported by dreadnaut on 2013-02-27 17:59:56 - Status changed: Accepted

[Christopher Kramer]

Okay, right.

>it is common to have an external sample file to copy or for reference 
Yes, but as we have a 1-file-approach, I guess spending another file on a sample config-file
is not necessary.

So as we now seem to agree on the requirements, I'd say patches are welcome ;-)

Reported by crazy4chrissi on 2013-02-27 19:39:34

[dreadnaut]

Do we want anything more complex than adding this around line 375?

$config_filename = './phpliteadmin.config.php';
if (is_readable($config_filename))
  include $config_filename;

Reported by dreadnaut on 2013-02-27 19:52:26

[Christopher Kramer]

I guess not. But I think we should really make clear to users that this file is included
and overwrites settings defined in phpliteadmin.php. So I think we should place it
above the language-constants around line 110, so users see it. And we should place
some comments somewhere around lines 36 and 108 that tell the user that he might want
to edit the config-file instead of the source file.

Especially users that know phpliteadmin will edit phpliteadmin.php and might be confused
because their config-settings get overwritten with default-settings.

So maybe it really is a good idea to make PLA come with a sample config-file only that
needs to be renamed to be used.
This also solves the "should be provide a sample additionally" question because the
user can then decide whether he copies the sample or edits and renames it.

Reported by crazy4chrissi on 2013-02-27 20:03:06

[Former user]

here it is

Reported by ykorotia on 2013-02-27 21:04:30

<hr> * Attachment: config-with-late-params-substitute.patch

[dreadnaut]

| Custom functions MUST BE configured/declared in phpliteadmin file directly!

Er, why?
Also, could you switch all double quotes to single quotes? They are a tiny bit faster
:)

Reported by dreadnaut on 2013-02-27 21:17:07

[Former user]

I did true copy-paste. Same quotes as in main file.

because you cannot redeclare functions. Of course, you can make function_exists(...)
bla-bla. I did as I did. I cannot get your aproach with customs functions, so that's
it

do what ever you want

regards

Reported by ykorotia on 2013-02-27 21:22:08

[Christopher Kramer]

> Also, could you switch all double quotes to single quotes? They are a tiny bit faster
:)

LOL. Yeah, well, I thought so myself, but there is much discussion about this. Lots
of people claim double quotes are even faster. For example have a look at these:
http://nikic.github.com/2012/01/09/Disproving-the-Single-Quotes-Performance-Myth.html
http://phpbench.com/
http://micro-optimization.com/single-vs-double-quotes

I guess the real point why we should use single quotes for configuration is because
people that don't know PHP might otherwise do something like this:
$cookie_name = "pla3412$thiswilldo{$strange}things";

Probably that's the reason why $cookie_name currently uses single quotes and all others
use double quotes, but I am not sure.

What I think is more important is to use stuff consistently. We maybe should have a
look at this sometime, but that's another issue.

> Custom functions MUST BE configured/declared in phpliteadmin file directly!
Yeah, indeed functions cannot be redeclared. Maybe we should put an "if (function_exists(..)"
around the functions we provide as examples.
Or just say something like "make sure you don't declare the same function in phpliteadmin.php
and phpliteadmin.config.php"

Reported by crazy4chrissi on 2013-02-27 22:21:02

[dreadnaut]

| LOL. Yeah, well, I thought so myself, but there is much discussion about this.
| Lots of people claim double quotes are even faster.

I have seen those pages myself through the years, and they usually amount to bad science.
You can check yourself: take the code from this http://micro-optimization.com/single-vs-double-quotes
and run it. Then swap the calls to f1() and f2(), so that double quotes are tested
before single quotes, and run it again.

However, the timing difference is so small that there is no practical use in forcing
one style over the other, hence the smilie in my previous message, and this one :)

Single-quote strings are good practice for the reason you point out, and for static
HTML strings where double quotes may appear for attributes.


About user functions: since we now have a sample configuration, do we still need sample
functions inside the main code? We could move them to the sample config (together with
the $databases sample array?) and reduce the phplisteadmin.php's size.

Reported by dreadnaut on 2013-02-27 23:04:07

[Christopher Kramer]

> swap the calls to f1() and f2(), so that double quotes are tested before single quotes,
and run it again.
Yeah, I know people make mistakes like this. In fact it doesn't matter at all - it
is not worth even thinking about this if it is only several nano seconds. If we write
code where this would matter, we should use another language, not php.

> About user functions: since we now have a sample configuration, do we still need
sample functions inside the main code? We could move them to the sample config (together
with the $databases sample array?) and reduce the phplisteadmin.php's size.
Yeah, sounds reasonable.

Reported by crazy4chrissi on 2013-02-27 23:10:57

[dreadnaut]

Ok, extending on ykorotia's: a patch to remove user functions from the main file and
include the external config (where I swapped file_exists() for is_readable()) and a
config file with a slightly different option ordering.

Also, we could change the $database config array to 'name' => 'filename'. I know the
current format is used later, but this would make more sense and we can convert it
to a sub-array later.

And if the key is an integer, we could use the filename as name, so people could write:

$databases = array(
 'myfile.sqlite',
 'othername.sqlite',
 'shortname' => 'verylongpath/loooongpath/longereven/sillyname.sqlite',
);

Reported by dreadnaut on 2013-02-27 23:36:03

[dreadnaut]

Uhm, attachment fail?

Reported by dreadnaut on 2013-02-27 23:37:30 - Status changed: Started - Labels added: Usability

<hr> * Attachment: config.diff * Attachment: phpliteadmin.config.sample.php

[Christopher Kramer]

Thanks. I really like this solution. Feel free to commit it.

Regarding the change of $databases: Well, I guess only a few people actually still
use this method. I agree that name => filename or filename only would be easier.

If you feel that it's worth it:
Please open a separate issue for it and copy the relevant posts there.
And if you like, write a patch for it. But please make it backwards compatible. So
it should loop through the array and distinguish all three possibilities:
1. key is int and value is string => value is path, create name automatically from
filename
2. key is int and value is array => old method used. Use name and path elements of
the value-array. (if they don't exist, error)
3. key is string and value is string => use key as name and value as path
4. else (e.g. value is null): display error message (and continue looping?)

Reported by crazy4chrissi on 2013-02-28 21:56:22

[Christopher Kramer]

Reported by crazy4chrissi on 2013-02-28 21:56:43

[dreadnaut]

This issue was closed by revision r345.

Reported by dreadnaut on 2013-03-01 18:05:22 - Status changed: Fixed

[dreadnaut]

This issue was closed by revision r346.

Reported by dreadnaut on 2013-03-01 18:06:44

[dreadnaut]

Committed, in two parts because I forgot to move one of the two files :|

I also added a new wiki page where we should add proper documentation. I'll file a
new issue for that, but you can find it here: http://code.google.com/p/phpliteadmin/wiki/Configuration

Reported by dreadnaut on 2013-03-01 18:10:27

[Christopher Kramer]

Thanks.

Reported by crazy4chrissi on 2013-03-02 17:35:34