Item8640: Multiple issues when installing Extensions with bin/configure

There is duplicated code between tools/extender.pl and lib/Foswiki/Configure/UIs/EXTEND.pm which are inconsistent in operation:

(Items flagged with when fixes committed.)

• The web version of Extension installation doesn't backup any files (trunk)
• The web version no longer does a checkin of Topics and Attachments (trunk)
• The shell version overwrites files with read-only permission - the web version fails with an error. (trunk)
  • Shell version is correct - overwrite readonly files. Some plugins mark files readonly in their manifests.
• The web version currently ignores the manifest embedded in the _installer file. (trunk / release)
• The web version doesn't set file permissions. So files get default permissions (trunk / release)
  • In an apache suexec environment, files and directories get "safe" permissions of 600 (trunk / release)
  • Reported on IRC that FreeBSD also doesn't set the executable permission for files installed into bin
• Handling non-standard directory locations and Web names is inconsistent. (trunk)
• bin scripts loose customization of the #! shebang line (trunk / release)
• Uninstall doesn't remove rcs files. ( trunk / release )

This task will track some additional refactoring of tools/extender.pl and lib/Foswiki/Configure/UIs/EXTEND.pm

• Pull common operations into lib/Foswiki/Configure/Util.pm
• Implement common backup once ImproveExtensionBackupProcess is appoved.
• This work planned for trunk only.

Branch appears to have similar issues to trunk.

-- GeorgeClark - 28 Feb 2010

Notes on the rcs *,v files:

The "shell" installer today creates a ,v file for each data or pub file unless the file is flagged as noci in the manifest. The bin/configure installer today does not checkin any files. My changes to the installer will give consistent results regardless of the installation method. It will cause the web installer to create some ,v files, but will reduce the number created by the shell installer.

• New file, no existing history - no checkin. (shell would have created a checkin)
• Existing file, no history - checkin unless flagged noci - manifest in control. (web would have missed the checkin) o This allows the user to view the changes installed by an extension
• Existing file with existing ,v file, Always checkin (web would have resulted in out-of-sync .txt and .txt,v ) o Very important to avoid auto-revert caused by the new Store implementation which trusts the rcs file over the txt file.
• Attachments in pub will be treated under the same rules.

I didn't put any of the above comments on the rcs files into a proposal because I considered the inconsistent behavior between shell and web installers a bug. Plus the new store behavior on trunk could cause major issues if .txt file is changed without updating the existing .txt,v file.

-- GeorgeClark - 12 Mar 2010

I've moved the Extension install / uninstall and other extension utility functions into a new Foswiki::Configure::Package class.

-- GeorgeClark - 17 Mar 2010

Hi George, nice work. I just tried in my svn installed trunk, and tools/extension_installer TagMePlugin seemed to work well.

However it said that FilterPlugin and JQueryPlugin were required dependencies, when they are supposed to be optional. This could be an old bug though.

-- PaulHarvey - 05 Apr 2010

Hi Paul. This appears to be an issue with Dependency.pm and BuildContrib. The "optional/required" is only part of the description, it is not parsed out when the Dependency file is processed during the build, and is not used by Dependency.pm when deps are checked. I can easily add the description to the report, so that it is shown, but we'd need to think through how to handle the "optional" deps. We don't have an easy way to prompt for the web installer. The shell installer could be changed to prompt, and pass through a flag to skip optional deps.

-- GeorgeClark - 05 Apr 2010

I've checked in a change to Package.pm to add the dependency description to the report. This tells the user that the dependency is optional, but it also says that the dependency is required - which should probably be added to Tasks.Item8826

-- GeorgeClark - 05 Apr 2010

Re-opening because George used expect_failure when he shouldn't have. expect_failure marks the test as bound to fail, therefore it should only be used when making a test. Here, the test was wrapped into something like:

if( eval "use Some::Module; 1" ) {
  ... do the test
}
else {
  expect_failure
}
cleanup;
return;

Therefore, either we test for the module, set the expect_failure, AND do the test, or have it fail, or we simply don't expect failure.

Looking at how expect_failure is used in other tests, I simply added a $this->assert(0) to force the failure without doing the test (SvenDowideit will be happy :))

-- OlivierRaginel - 30 Apr 2010

Commenting is disabled while not logged in