Ticket #97 (closed defect: fixed)

Opened 8 years ago

Last modified 3 years ago

New installer: improved interface, better system requirements check

Reported by: dartar Owned by: BrianKoontz
Priority: normal Milestone: 1.3.1
Component: installer Version: 1.1.6.1
Severity: major Keywords: mysql db connection installer setup
Cc: JavaWoman, DotMG

Description (last modified by DarTar) (diff)

Gathered from error reports in different places (including comments on WikkaInstallation, #wikka and IM) - neither new, but worth repeating and putting together, I think:

  • In at least some cases the "detected" base path misses a closing slash (resulting in links that don't work); the installer should check if the derived path has a final slash, and if not, add one, before presenting that to the user.
  • If PHP (and thus Wikka) cannot connect to MySQL the installer just "hangs" after the first screen and printing "Testing configuration"; this seems to be (at least partly) the result of having error messages from the MySQL statements suppressed with a @, which would be OK if the error message were handled afterwards - but this doesn't happen. Other mysql_xxx() calls in the installer also have the @ suppressing error messages. That should either be removed, or all error messages should be picked up and handled by the installer, at the very least by showing the message to the user.
  • Move DeleteCookie() calls to wikka.php : cookies are stored in every visitors' navigator, not only in installer's. Letting DeleteCookie() in install may lead to unability to track some bugs like unable to log out.

This is especially a problem for users new at MySQL or new even at PHP+MySQL for whom it won't be obvious where to start troubleshooting when the installer simply hangs.

--JavaWoman

Related tickets

  • #141 base_url issues
  • #408 upgrading from Wikini
  • #464 Page hang due to missing MySQL libs
  • #518 Advisory message incorrect during upgrade
  • #167 Preconfigured ACL settings during installation (optional)
  • #604 Generation of .htaccess rules, specifically RewriteBase
  • #778 Import from MediaWiki

General discussion

 http://wikkawiki.org/NewInstaller

Change History

  Changed 8 years ago by GiorgosKontopoulos

Look also ticket #141

  Changed 8 years ago by DarTar

  • description modified (diff)

  Changed 8 years ago by DarTar

  • milestone changed from 1.1.6.2 to 1.1.6.3

moving this to 1.1.6.3 for a more principled approach

  Changed 8 years ago by DarTar

  • owner changed from unassigned to DarTar
  • status changed from new to assigned

  Changed 7 years ago by DotMG

  • owner changed from DarTar to DotMG
  • status changed from assigned to new
  • description modified (diff)

  Changed 7 years ago by DarTar

  • description modified (diff)

  Changed 7 years ago by DarTar

  • description modified (diff)
  • summary changed from Installer problems to New installer: improved interface, better system requirements check

  Changed 7 years ago by DarTar

  • description modified (diff)

  Changed 7 years ago by DotMG

  • status changed from new to assigned

To enhance tracking of changes in default pages, those default pages (more precisely, their raw codes) are stored in individual files. With that, when something at a specific line changes, only that line will show in diff, and not the whole page. Default pages were committed in [331] and [332]

Some configurations were removed from the installer, such as geshi, ...

Install is now done in 5 steps :

  • step 1 : Entering database settings
  • step 2 : Checking configuration to see wether the environment can run Wikka perfectly. The environment settings can be downloaded to permit a better troubleshooting when required.
  • step 3 : Further install configurations (ACL settings, site identity, admin profile)
  • step 4 : Install : setting up/updating database and other stuffs
  • step 5 : writing config and when needed, updating .htaccess

todo:

  • CSS
  • Adding proper DeleteCookie() in wikka.php

  Changed 7 years ago by DotMG

  • keywords installer setup added

setup/ files updated and added in [333] and [334].

footer.php and header.php are no longer needed.

A .htaccess has been added to avoid direct loading of files in setup/ folder when mod_rewrite not available. (maybe a not so good idea?)

These changes were committed in [335].

  Changed 7 years ago by DotMG

  • owner changed from DotMG to DarTar
  • status changed from assigned to new

I'm taking related tickets. DarTar, please do the necessary about CSS or delegate it to another one, except me.

TODO: CSS - pre-established type of usage (private, open, personal, ...)

  Changed 7 years ago by DotMG

Created a subticket #458 to fine-tune this.

  Changed 7 years ago by DotMG

Created a subticket #460 to use unix file-format.

  Changed 7 years ago by DarTar

(In [445]) Dropping <tt>handlers</tt> field from pages table: refs #97 and fixes #452

  Changed 7 years ago by DotMG

(In [453]) refs #97 errors in acl settings: used acl_write instead of default_write_acl. fixed now.

  Changed 7 years ago by DotMG

(In [467]) further cleanup. suppressing ADD_CONFIG_ENTRY. refs #97, suite of [466] and [461]

  Changed 7 years ago by DarTar

Same here—unless I'm missing something, I don't understand why ADD_CONFIG_ENTRY should be removed as part of a code cleanup. Could you give some more details? I note incidentally that the new installer adopts a totally different approach to l10n (via the __() method) compared to the rest of the code (where l10n is done through define's), so maybe some preliminary discussion/explanation is in order. Thanks!

  Changed 7 years ago by DarTar

I'll add that since the installer is crucial for all new functionality, it would be good to have a quick reference guide for developers so they know what has to be done everytime a change in the default pages/language file/config settings/table layout is needed.

  Changed 7 years ago by DotMG

Sorry again ...

Since 1.1.7, new config entries are handled automagically by editing libs/Config.class.php, adding

var $newconfig = 'default_value';

There is no need to add them manually, like the old way, so, removing these items is a code cleanup IMO.

Configuration, except for crucial parameters needed in setup (like db settings) should take place outside the installer, and need to be runnable, ie modifiable at any time. This deserves a new ticket. (Handle configs in db table).

  Changed 7 years ago by DarTar

DotMG, can I suggest a minor rewording of server_info.txt?

Wikka Installer: System information
-------------------------------------

Wikka version: 1.1.7

Server:
type: Apache
version: 1.3.33
php_interface: Module
mod_rewrite: 1

Environment:
php: 4.4.4
mysql: 5.0.22-standard

-------------------------------------
System information generated on: 2007-05-18 15:06:09

  Changed 7 years ago by BrianKoontz

  • description modified (diff)

  Changed 7 years ago by BrianKoontz

  • description modified (diff)

  Changed 7 years ago by DarTar

(In [552]) Minor cosmetic changes to the Wizard and typos fixed, refs #97

  Changed 7 years ago by DarTar

Some notes on the latest version [552]:

  • could we keep the first screen just for DB connection check and move the language settings to the second screen?
  • I get the following PHP warning on 3/5: Warning: touch() [function.touch]: Unable to create file wikka.config.php because Permission denied in [...]/trunk/setup/setup.php on line 67. Shouldn't we handle file permission errors internally?
  • What do you think of the proposal above ?

  Changed 7 years ago by DarTar

If an alternate name is specified for HomePage (e.g. HumePuge) on step 3/5 the installer produces an error:

Adding/Updating default page HumePuge... 
FAILED: Default page not found or file not readable (HumePuge, lang/en/defaults/HumePuge.txt)

The installer should create the corresponding page and feed it with default content from HomePage.txt

An alternative approach would be to use aliases as suggested by Olivier here: #340

  Changed 7 years ago by DarTar

(In [561]) Minor changes to the installer: more consistent stylesheet for input fields, minor rewording of descriptions/headings, moving ACL options to the bottom of screen 3/5, fixing issue with dirname not displaying the correct path, refs #97

  Changed 7 years ago by DarTar

(In [562]) Replacing hardcoded table prefix in installer, refs #97 and #241

follow-up: ↓ 29   Changed 7 years ago by DotMG

(In [573]) refs #97

Cleaning code : prefixing touch() with @, suppressing unused codes in writeconfig.php (The suppressed codes were inserted when I misunderstood the words collecting install information. This process is now performed by grabinfo.php, and the suppressed codes don't do nothing (except throwing error message)

in reply to: ↑ 28   Changed 7 years ago by JavaWoman

Replying to DotMG:

(In [573]) refs #97 Cleaning code : prefixing touch() with @

touch() returns a boolean to report failure or success; rather than merely suppressing any error message it would be more useful to actually catch an error situation and handle it (and try again), or report it so the user can remedy the situation.

  Changed 7 years ago by BrianKoontz

mod_rewrite detection is still not working as of the latest trunk version...if this is an O/S specific problem, can we at least bring back manual configuration of mod_rewrite as in the previous installer as a backup?

(My apologies if this is being addressed in another ticket...too many tickets to keep track of!)

  Changed 7 years ago by JavaWoman

  • description modified (diff)

adding #167 as related; would be optional for 1.1.7, but it would be nice if basic configuration of ACLs at least is included.

follow-up: ↓ 33   Changed 7 years ago by JavaWoman

For cookie configuration, the installer has explanatory text (in setup.php) that first refers to "suffix" and then to "prefix" - all in the same paragraph.

in reply to: ↑ 32   Changed 7 years ago by JavaWoman

Replying to JavaWoman:

For cookie configuration, the installer has explanatory text (in setup.php) that first refers to "suffix" and then to "prefix" - all in the same paragraph.

The issue in this comment was resolved in [607].

  Changed 7 years ago by JavaWoman

New index needs to be added to users table - see http://wush.net/trac/wikka/ticket/398#comment:8 and referenced comment

follow-up: ↓ 36   Changed 7 years ago by DarTar

[739] replaces styling for invalid <xmp> element with styling for samp: the installer will have to be modified accordingly.

in reply to: ↑ 35   Changed 7 years ago by JavaWoman

Replying to DarTar:

[739] replaces styling for invalid <xmp> element with styling for samp: the installer will have to be modified accordingly.

DarTar, I searched, but I could not find any actual {{usage}} of <xmp> - including but not limited to the installer - I changed it just in case we would actually want to use markup for an example.

  Changed 7 years ago by DarTar

OK I thought we were still using <xmp> in the last step of the installer (writeconfig.php) but we have a read-only <textarea> instead.

  Changed 7 years ago by DotMG

(In [741]) refs #97

By default, RewriteEngine should be set to Off to ensure that no problem will be encountered before install. If it is set to On and the user tries to install on a subdirectory, the installer may be not reachable (The server issues a 500 error code if the RewriteBase is not correct or absent).

  Changed 7 years ago by DotMG

(In [742]) refs #97

phpdocs will be done soon.

  Changed 7 years ago by DarTar

Do you guys see any harm in logging in automatically the Admin once the installation process is completed?

This would save an extra step and we could then replace the ugly message You need to login and then double-click on any page or click on the Edit link in the page footer to get started. with something more friendly as we used to have before 1.1.6.1 (e.g. Double-click this page to start editing), what do you think?

For upgrades this is probably less important because admins and users are already familiar with the login system.

  Changed 7 years ago by DotMG

(In [750]) refs #97 : minor corrections to setup/setup.php

  Changed 7 years ago by DotMG

(In [751]) refs #97

  • phpdocs
  • moving test of 'writeability' of files at the first step of the installer.
  • in setup/setup.php, the @touch(SITE_CONFIGFILE) is no longer needed as this is done in setup/default.php

  Changed 7 years ago by DotMG

(In [752]) refs #97

  • removed commented out todos and checkes that have been doublechecked to be okay
  • in setup/setup.php, the @touch(SITE_CONFIGFILE) is no longer needed as this is done in setup/default.php
  • setup/grabinfo.php, setup/links.php, changing file CRLF to UNIX mode

  Changed 7 years ago by DotMG

(In [753]) Adding die() on setup/grabinfo.php.

refs #97

  Changed 7 years ago by DarTar

  • cc JavaWoman added

Adding a pointer just in case some of the changes committed by JavaWoman to the 1.1.6.4 installer may be relevant for 1.1.7:

http://wush.net/trac/wikka/ticket/562#comment:11

  Changed 6 years ago by BrianKoontz

mod_rewrite autodetection fails if Wikka is installed in a subdirectory. I believe this is because /setup/test/.htaccess has a hard-coded path that assumed installation in a subdirectory named trunk:

RewriteBase /trunk/setup/test

  Changed 6 years ago by BrianKoontz

(In [829]) Modify RewriteBase in test/.htaccess to accommodate Wikka installations in subdirectories (refs #97).

  Changed 6 years ago by BrianKoontz

Suggestion from #wikka (arfyarf):

[I]f the install could test whether that RewriteBase line was needed in .htaccess it would be convenient too!

  Changed 6 years ago by DarTar

  • cc DotMG added

follow-ups: ↓ 51 ↓ 55   Changed 6 years ago by DotMG

What do you mean by RewriteBase line was needed? That it should not be inserted if not needed or it should be added if needed?

Actually, #97 is really for 1.1.7 We've done so much work on it that it cannot be added to 1.1.6.4. And in 1.1.7, (iow trunk), RewriteBase is added systematically, according to the installation.

in reply to: ↑ 50 ; follow-up: ↓ 54   Changed 6 years ago by BrianKoontz

Replying to DotMG:

What do you mean by RewriteBase line was needed? That it should not be inserted if not needed or it should be added if needed?

That it should be added if needed.

Actually, #97 is really for 1.1.7 We've done so much work on it that it cannot be added to 1.1.6.4. And in 1.1.7, (iow trunk), RewriteBase is added systematically, according to the installation.

Then this is all a moot point, as it appears that the issue is fixed in the 1.1.7 release.

  Changed 6 years ago by DarTar

We do have several 1.1.6.4-specific changes in the installer, so a ticket with a summary of the changes might be useful (to help with the release notes especially).

  Changed 6 years ago by JavaWoman

  • description modified (diff)

in reply to: ↑ 51   Changed 6 years ago by JavaWoman

Replying to BrianKoontz:

Replying to DotMG:

Actually, #97 is really for 1.1.7 We've done so much work on it that it cannot be added to 1.1.6.4. And in 1.1.7, (iow trunk), RewriteBase is added systematically, according to the installation.

Then this is all a moot point, as it appears that the issue is fixed in the 1.1.7 release.

As far as I'm aware detection of the need for and derivation of RewriteBase isn't finalized yet. Even if it is, it's worth mentioning explicitly (not 'moot') so it can be tested explicitly as well.

in reply to: ↑ 50   Changed 6 years ago by JavaWoman

Replying to DotMG:

And in 1.1.7, (iow trunk), RewriteBase is added systematically, according to the installation.

IMO it should be added only if it's actually needed; if that's what you mean by "systematically", OK, but I get the impression it's always added now.

follow-up: ↓ 58   Changed 6 years ago by DotMG

systematically means always added, even if not required. And since it doesn't generate error, why not? Imagine the hoster changes Apache configuration, thinking this would be transparent to webmaster. (So, they move something into VirtualHost). The Wikkawiki installation will then stop working, and the webmaster will have no clue why (until looking at error log).

  Changed 6 years ago by DotMG

(In [844]) Code cleanup for revision [829]

refs #97

Automatic derivation of RewriteBase is done for ./.htaccess in setup/writeconfig.php, and for setup/test/.htaccess, it was done in [829], and cleaned up here.

in reply to: ↑ 56   Changed 6 years ago by JavaWoman

Replying to DotMG:

systematically means always added, even if not required. And since it doesn't generate error, why not? Imagine the hoster changes Apache configuration, thinking this would be transparent to webmaster. (So, they move something into VirtualHost). The Wikkawiki installation will then stop working, and the webmaster will have no clue why (until looking at error log).

The why not is simple: it causes extra processing, even when not needed. It could not become suddenly necessary if the hoster changes Apache configuration: it's needed only if the users' files are aliased into their webroot, so if it wasn't necessary before the only way it could become necessary would be by physically moving all of a user's files on their host system. That would be a very rare possibility, as opposed to the extra processing for every single server request.

If there really were such an architecture change in the hosts system I would imagine they would be providing support to their users for the consequences of such an architecture change.

Why add unnecessary statements (and inefficiency) when the need for RewriteBase is actually easy to detect?

Also keep in mind that there are other webservers (than Apache) or extensions that may use a file called .htaccess; the fewer unnecessary statements our (generated) file contains, the higher the probability it would work, or at least be easier to convert.

  Changed 6 years ago by DotMG

Replying to JavaWoman:

Why add unnecessary statements (and inefficiency) when the need for RewriteBase is actually easy to detect?

I don't understand why RewriteBase is inefficient and how easy it is to detect its need. What extra-processing does it generate and if we could quantify such extra-processing, how much will it be compared to the extra-coding we'll add at the installer? It was quite difficult to detect RewriteEngine availability. I agree that .htaccess need to be as small as possible, but I'd prefer have extra RewriteBase line on thousand installations where it is not needed, and working, ... than have that line forgotten in one installation that required it, and seeing Wikka not working.

  Changed 6 years ago by DotMG

(In [856]) Minor cleanup. refs #97

install.php: Making some possible db error not fatal (when unable to remove the handler field from table_pages, or when some errors on updating table_acls.

writeconfig.php: cleaning up sessions data when install about to be finished.

  Changed 6 years ago by DotMG

(In [863]) Installer: Should initiate an output buffering with grabinfo.

refs #97

  Changed 6 years ago by DotMG

(In [880]) On fresh install, adding default pages CategoryAdmin and DatabaseInfo.

refs #97, #27, #575. See also [876].

  Changed 6 years ago by DotMG

(In [928]) Creating index on field owner to accelerate queries for method LoadPagesByOwner().

refs #398, #97

  Changed 6 years ago by DarTar

(In [1041]) removing "break;" from installer version check, refs #97

  Changed 6 years ago by DarTar

  • description modified (diff)

  Changed 5 years ago by DarTar

  • milestone changed from 1.2 to 1.3

Retargeting to 1.3. Code for this ticket may have already been committed to trunk, from which 1.3 will be branched. Consider backporting urgent issues to 1.2.X

  Changed 4 years ago by BrianKoontz

  • owner changed from DarTar to BrianKoontz
  • status changed from new to accepted

Ported from trunk to 1.2 branch. Fixes include:

  • (In [1520]) More work on installer: Reconstructed default paths, removed site-based config stuff. Refs #912.
  • (In [1521]) Fixed a 404 error during installation (base_url issue). Refs #912.
  • (In [1522]) Fixed autologin problem (missing field in users table). Refs #912.

  Changed 4 years ago by BrianKoontz

  • status changed from accepted to testing

  Changed 4 years ago by BrianKoontz

(In [1523]) Restored installer CSS lost at some point in the past. Refs #912, #97

  Changed 4 years ago by BrianKoontz

(In [1524]) Added missing installer file. Refs #912, #97

  Changed 4 years ago by DarTar

I tried to install a fresh 1.3, the installation process went fine the wiki gives a 404. This happens regardless of the rewrite_mode settings in the config file. I think we should have an override mechanism where by the base_url can *still* be manually set if for any reason the installer fails to detect it.

follow-up: ↓ 73   Changed 4 years ago by BrianKoontz

Interesting...I just did a fresh 1.3 install as well, and had no problems. You mentioned the rewrite_mode settings in the config file: If this was a "fresh" install, where did the config file come from?

That said, I don't doubt there are some quirks with the installer. I'll try a few more installs to see if I can replicate what you're seeing. And yes, I believe a base_url confirmation textbox is a good idea.

in reply to: ↑ 72   Changed 4 years ago by DarTar

Replying to BrianKoontz:

You mentioned the rewrite_mode settings in the config file: If this was a "fresh" install, where did the config file come from?

I mean the new config file created by the installer.

  Changed 4 years ago by NilsLindenberg

base_url wasn't created for my install, either.

  Changed 4 years ago by DarTar

what I don't understand of the latest 1.3 is that, even though there is no way of explicitly setting base_url, if I change rewrite_mode to 0 in the configuration, the wiki should generate a standard URL (ending with wikka.php?wakka=) but it doesn't (it keeps loading /HomePage, and this even if I remove the .htaccess).

  Changed 4 years ago by DarTar

Method Href() in Wakka.class.php still uses a $this->config["base_url"]

  Changed 4 years ago by DarTar

ok - probably found the culprit, wikka.php has been ported to 1.3 but due to the changes in trunk many other methods defined in libs/Wakka.class.php and used by wikka.php are obsolete. I guess that might explain why I cannot get to install 1.3 with or without URL rewriting.

  Changed 4 years ago by DotMG

On 1.2 to 1.3 upgrade, see also [1665] :

Now that update_default_page() is available, we can safely replace long SQL insert in setup/install.php with a simple call to update_default_page(). I've added a parameter $note to setup/inc/functions.inc.php#update_default_page() to which we would pass "Upgrading from 1.2 to 1.3" This commit also adds WikkaConfig page in case of an upgrade. And some merging of ACLS setting was also performed to simplify the install process.

  Changed 4 years ago by BrianKoontz

(In [1667]) Test for existence of error_flag in session array. Refs #97.

  Changed 4 years ago by BrianKoontz

  • status changed from testing to commit

Upgrade from 1.2 to 1.3 works for me.

  Changed 4 years ago by BrianKoontz

  • status changed from commit to closed
  • resolution set to fixed

  Changed 4 years ago by DotMG

(In [1685]) refs #519, #97 : language packs are dynamically checked upon install. the installer will give the ability to choose one from the language packs present as default lang.

  Changed 3 years ago by BrianKoontz

  • milestone changed from 1.3 to 1.3.1

Updated milestone to 1.3.1

Note: See TracTickets for help on using tickets.