Split up InstallerScripts

[bwaidelich]

Description

The Base Distribution comes with a set of composer post-* hooks:

    // ...
    "scripts": {
        "post-update-cmd": "Neos\\Flow\\Composer\\InstallerScripts::postUpdateAndInstall",
        "post-install-cmd": "Neos\\Flow\\Composer\\InstallerScripts::postUpdateAndInstall",
        "post-package-update": "Neos\\Flow\\Composer\\InstallerScripts::postPackageUpdateAndInstall",
        "post-package-install": "Neos\\Flow\\Composer\\InstallerScripts::postPackageUpdateAndInstall"
    }
    // ...

Upon composer install/composer update they

1. make sure that the Configuration folder exists
2. make sure that the Data folder exists
3. copy distribution essentials overriding files if they exist already
4. copy distribution defaults keeping files if they already exist
5. rescan packages
6. make sure the flow bootstrap is executable (chmod('flow', 0755))

Upon installation of a single package (i.e. composer require ..) they

1. check if the installed/updated package has a extra.neos.installer-resource-folders section in the composer manifest and, if yes, copies Distribution/Essentials and Distribution/Defaults subfolders like above (for example the neos defaults)
2. check if the installed/updated package has a extra.[typo3/flow].post-install or extra.[neos/flow].post-install section in the composer manifest and, if yes, executes those (I have no idea why we can't use the default scripts.post-update-cmd instead)
3. check if the installed/updated package has a extra.[typo3/flow].manage-resources section in the composer manifest and, if yes, copies copies distribution files from a hard-coded Resources/Private/Installer/ subfolder. (overlaps with 1. and is deprecated since 3.1)

The main issue with that is that a lot of (dummy) files are created upon each composer install (i.e. deployment). If you have a custom Readme.rst with instructions for an Flow/Neos application it will be overridden.

Steps to Reproduce

1. Install a Flow or Neos base distribution
2. Remove/Update the Readme.rst or Upgrading.rst
3. run composer install

Expected behavior

The changes made to the rst files should not be reverted

Actual behavior

The rst files are re-created/overridden with the ones of the flow distribution

Suggestion

Split up the InstallerScripts into smaller, more meaningful commands that can be adjusted per installation. For example

"scripts": {
  "post-install-cmd": [
    "Neos\\Flow\\Composer\\InstallerScripts::createConfigurationTemplate",
    "Neos\\Flow\\Composer\\InstallerScripts::createWebFolder",
    "Neos\\Flow\\Composer\\InstallerScripts::createReadmeAndUpgradingFiles",
    // ...
  ],

[kdambekalns]

I like that idea. There are drawbacks, but people will always find a way around best intentions anyway, so none of that matters. :)

[bwaidelich]

Could you elaborate on the drawbacks you see? Obviously there would be quite some duplication having most of the smaller commands in update & install. But we could always have the current method by default (which internally calls the other methods) but still allowing custom installations to change this..

[kdambekalns]

It's one more thing that people might need to update and/or forget to adjust. One more place where subtle differences in setups could be caused. Those kind of drawbacks…

[bwaidelich]

I see, yes. But as said I think we can keep the existing methods like:

class InstallerScripts
{

    public static function postUpdateAndInstall(Event $event)
    {
        self::createConfigurationTemplate();
        self::createWebFolder();
        self::createReadMeAndUpgradingFiles();
        // ...
    }
}

and even keep those in the composer manifest by default. Apparently this hasn't hurt many people yet - We should still document how to customize this ofc ;)

[sorenmalling]

I will love to help with this for the 6.0 release as I'm currently investigating this subject

[kitsunet]

It would be super cool to actually move these scripts to the distribution, that way we could prevent those nasty warnings that happen until the Flow package was actually installed.

[kdambekalns]

That would leave us with two places to maintain those scripts, though… and if we (need to) update them, people would no longer get them "for free", but would have to manually update them in their distribution.

To avoid the warning, we could ship a "never-changing" script in the distribution(s) that checks for existence and calls only if possible…

[nrueckmann]

Sorry for digging this topic up, but since i wanted to ask a question regarding post-install and post-update i thought this thread seems to be the right spot.

Are those hooks (post-install and post-update) still desired? I haven't found any documentation or usage of them. I'm a bit worried that some packages might do weird stuff during installation like trying to manage content or other things which do not belong the file system.

[kitsunet]

Sorry for digging this topic up, but since i wanted to ask a question regarding post-install and post-update i thought this thread seems to be the right spot.

Are those hooks (post-install and post-update) still desired? I haven't found any documentation or usage of them. I'm a bit worried that some packages might do weird stuff during installation like trying to manage content or other things which do not belong the file system.

These scripts are defined by composer and respectively documented here: https://getcomposer.org/doc/articles/scripts.md

They are only observed in the root composer.json anyways, so packages don't even have a chance to use them for anything.

[nrueckmann]

I think we are talking about different things. I'm talking about potential extra.neos/flow.post-install or extra.neos/flow.post-update fields in composer.json of third party packages. Which will trigger the execution of the defined method.

# Neos.Flow/Classes/Composer/InstallerScripts.php

if ($operation instanceof InstallOperation && isset($packageExtraConfig['neos/flow']['post-install'])) {
    self::runPackageScripts($packageExtraConfig['neos/flow']['post-install']);
}

if ($operation instanceof UpdateOperation && isset($packageExtraConfig['neos/flow']['post-update'])) {
    self::runPackageScripts($packageExtraConfig['neos/flow']['post-update']);
}

[kdambekalns]

That is in fact still a desired feature. As with all "package formats", you could/should check their install scripts if you don't trust the source (enough).

One option to help solve your concern would be to ask before executing those scripts, optionally allowing to inspect them before confirmation. That would then be a fresh (feature request) issue.