Re: package validation (was Re: PHP_Beautifier-0.0.6.1)

Stefan Neufeind wrote:

On 6 Jun 2004 at 17:23, Tomas V.V.Cox wrote:

Greg Beaver wrote:

Tomas V.V.Cox wrote: You really need to check out the code in PEAR-1.4.0dev5.tgz, in particular PEAR_PackageFile and PEAR_ChannelFile classes. A package name validation regex is already in channel.xml and is fully supported by the installer.

IMHO this is not the place, see my opinions below.

Same here. I wonder if additional checks might be useful that you can't do with a regex - e.g. testing for versionnumber and state- combination (linux-kernel-style where an odd number would demand beta- state) or so. Maybe a combination - so we allow for additional checks but make lightweight checks available by a regex.

I've put some thought into why I think this is such a bad idea, and here is my reasoning. This hypothetical problem proposed by Stefan still does not require any complexity on the part of the client-side installer (the PEAR package), but instead on the channel *server* validation code. Once a package is publicly available, it had better be valid. If a package is valid, it had better be installable by the client-side installer. We can't be expected to police non-standard channels in the PEAR installer, that is the responsibility of the people who run the channels. Any attempt to do so will only result in problems with good channel implementations, and here is why: Once code is installed on the user hard drive, it should be considered essentially permanent. In other words, if a channel decides to switch the version validation scheme, the user has to download a whole slew of classes under the system proposed by Tomas. If 5 channels or 10 channels do this, we could be talking a half hour of downloads just to get the validation code. With the implementation I've set up, it will take about 45 seconds to download, even with ultra-slow bandwidth. The more download requirements placed on a user, the less likely that PEAR's channel feature will be useful or popular. In addition, pre-existing releases using the old version numbering scheme would suddenly become invalid. This is a severe case of KISS. In fact, the package name validition regex is required ONLY to validate user typing. It may be possible to modify it to only validate that the typed channel is a valid filename, so that all necessary package name validation is done server-side (again). A package name must be a valid filename, otherwise the registry can't save it (the filename is always packagename.reg). In fact, after reading my own argument, I will probably remove all custom validation, and make it validate against the valid filename regex instead :). Also, are you sure you have taken a look at PEAR_PackageFile and how it does validation? To make it do linux-style version validation, you would code (untested): define('PEAR_PACKAGEFILE_EXTENDED_ERROR_BADVERSION'); class PEAR_PackageFile_Extended extends PEAR_PackageFile {

    function PEAR_PackageFile_Extended()
    {

$this->_errorMessages[PEAR_PACKAGEFILE_EXTENDED_ERROR_BADVERSION] = 'Release version "%version%" is odd, but stability is stable. This is not allowed';

    }

    function validate()
    {
        if (!parent::validate()) {
            return false;
        }
        if (!($this->_packageInfo['release_version'] % 2)) {
            if ($this->_packageInfo['release_state'] == 'stable') {

$this->_stack->addError(PEAR_PACKAGEFILE_EXTENDED_ERROR_BADVERSION, 'error', array('version' => $this->_packageInfo['release_version']));

                $this->_isValid = false;
            }
        }
    }

} $pkg = new PEAR_PackageFile_Extended; $pkg->fromPackageFile('package.xml'); var_dump($pkg->validate()); $pkg = new PEAR_PackageFile; $pkg->fromPackageFile('package.xml'); var_dump($pkg->validate()); output of bool(false)bool(true) Certainly, server-side validation can be easily customized in the pearweb code.

Far better than doing this in pre- and post- scripts is to leverage the existing power of polymorphism. Simply allow users to extend PEAR_PackageFile, and redefine the validation of package version. Most of this stuff should be server-side, and not in the pear packager, as things like this tend to change as the political decisions change.

One idea could be. A new <php_dir>/PEAR/validations/ dir. Inside it there would be one <channel> dir where validations stands. Each channel team writes a package called for example: ChannelValidation_<channel name>, which would contain any number of installable php files at <php_dir>/PEAR/validations/<channel name>.

Maybe it's too late for this discussion already *g* but would a <php_dir>/PEAR/chan_<channelname>/validations structure make sense, or is this out of scope? This way only the PEAR-packages themselves would be located directly under .../PEAR/ for historical reasons. Only drawback: There might still occur name-clashes between PEAR and non-PEAR-packages :-( Well, just some quick thoughts - the class-naming-problem however remains.

class-naming is a different issue, more of a core PHP issue than anything else. I don't think we need to overly regulate where people put files. The installer, for technical reasons, will not allow file conflicts. If a non-standard package conflicts with an existing PEAR package, it will simply not be allowed to install until the conflict is out of the way. People who choose to do this will likely receive TONS of complaints from users, and that will certainly cause change. However, it is easy to specify a different installation directory for a non-standard package, and so this will fix things. Perhaps the only sure solution to this issue is to suggest that internal include statements (like include_once 'DB/mysql.php' to load the mysql driver) in new packages use replacements, as in: if ('@inc@' != '@' . 'inc@') {

    include_once '@inc@/DB/mysql.php';

} else {

    include_once 'DB/mysql.php';

} This solution will allow conflicting packages to be installed into any directory the user wishes, as all include statements will be using absolute rather than relative path. Then, the user can do: require_once 'Subdir/DB.php'; and all the code will work, even if it is installed in require_once 'Subdir1/Subdir2/Subdir3/DB.php'; Conflicting classnames are unlikely if every channel prefixes classnames with the channel name, unless some idiot chooses "DB" or some other package name as the channel name :). Greg