magento-hackathon / magento-composer-installer

Composer installer for Magento modules
210 stars 156 forks source link

Installer Ignores Packages that Pre-Date the Addition of the Installer #98

Closed astorm closed 10 years ago

astorm commented 10 years ago

If the installer package is added to a project it ignores the existing packages in the project, and there's no way to install these packages without removing them and reinstalling them. Ideally, the installer should attempt to install any packages that predate its own addition to the project

Steps to Reproduce

  1. Create a composer.json that mathces the file to follow
  2. Run composer.phar install
  3. Add "magento-hackathon/magento-composer-installer": "2.0.0-beta4" to the `require section
  4. Run composer.phar

Expected Behavior: The installed adds the pulsestorm/magento-better404 files to the magento-root-dir project.

Actual Behavior: The installer ignores the pulsestorm/magento-better404 package.

{
    "require": {
        "pulsestorm/magento-better404": "1.*"
    },
    "repositories":[     
        {
            "packagist": false
        },         
        {
            "type":"composer",
            "url":"http://packages.firegento.com"
        }
   ],
    "extra":{
        "magento-root-dir":"/path/to/magento",
        "magento-deploystrategy":"copy",
        "magento-force":"true"
    }     
}
Flyingmana commented 10 years ago

Implementation notes:

this would need an additional state tracking which does not exist yet.

It would make more sense to separate the whole deploy process more from composer and move it into an own command which reads the composer files by itself and only gets triggered automatically during the install/update process but is easier to trigger without composer

tkdb commented 10 years ago

You are adding in the wrong order. If you install a package but you don't have the installer for that package type, then composer won't invoke the installer (as it is not there).

Therefore you've made an error in the order how you added the requirements and you did the install (just flipping in your case does it) or the type must ship with composer already.

So far with composer generally it should be expected behavior that types which don't have an installer don't get that installer run when installing. And after installing, the installer is not run because the package is already installed.

Composer does not support to re-install a dependency.

tkdb commented 10 years ago

@astorm: It would be interesting to know how you've been running over this. Was it by error? Or what is the rationale behind the way you did this?

astorm commented 10 years ago

@tkdb I ran into it when I was trying to add a package to the firegento repository for the first time. I didn't know or understand how composer custom installers worked, and when I first added my package to the repository, I didn't require the magento-hackathon/magento-composer-installer installer (because, again, I didn't know it was needed).

Then I noticed my package wouldn't install — so I started aping what I saw in the other packages, and required magento-hackathon/magento-composer-installer. However, even after I added the magento-hackathon/magento-composer-installer package, nothing installed into my Magento system, (because, as you said, I was doing things in the "wrong order")

However, I didn't know there was an order — and when this didn't work it led me to think either the installer was broken, or that I was doing something radically wrong. This led me to ignore/abandon my project for a few weeks while I worked on other things, until I sat down and went though the setup and install of an extension with a finely toothed comb.

I understand it all now, but I'd like to see more developers using the firegento package repository — both as users and contributors. Making the process of creating and/or using an extension as easy and straight forward as possible would help with this. Having the extension install packages when it itself is installed would go a long ways towards this. If that's not feasible or desired, then at least a warning of some kind

We see there's some packages already installed, please remove and re-add them to trigger the Magento installer

would be a good idea. For a package management repository to succeeded, a majority of its users need to be spared from the details of how the repository runs. I don't understand how CPAN or Ruby Gems work behind the scenes — and I don't need to. Even if I want to create a package for these systems, I'm still spared a lot of their inner workings. That's what's awesome about them.

I'd like to see composer, and Magento composer, reach the same state.

Vinai commented 10 years ago

Just a quick pointer to the deploy-all command offered by the composer-command-integrator tool which can be used to deploy already installed extensions into Magento:

https://github.com/magento-hackathon/composer-command-integrator I agree that it is a good idea to minimize the required amount of knowledge for users though.

astorm commented 10 years ago

Thanks @Vinai — I spent 10 seconds looking for the up-vote button before remembering — right, GitHub, not Stack Exchange :)

tkdb commented 10 years ago

@astorm: Thanks for the detailed feedback. I just went through the process you describe and also had some similar (but not those) issues. I finally solved my not-knowing by creating a test-case with local repositories. This helped me a lot. As it's easy to do, it's perhaps something worth to write down as a small hands-on tutorial. I'm with you that anything that helps to make things more usable are generally a good thing, and I don't want to lower your intentions. For me (the little problems I've been running in while creating "my first extension") however it was more the point that I didn't understood composer than this tool here.

For the suggestion about the message you give, I'm not sure if the installer is able to access the meta-data of all packages. If so, it could take a look for the types it's for, however I'm not sure if gaining that info is already enough to decide to display the warning message you suggest. This might also need changes upstream, e.g. that composer notes down in composer.lock the installer used for a project (I dunno, maybe composer does already this, didn't check).

tkdb commented 10 years ago

@astorm: And I think this FAQ entry is worth in this discussion: Should my module require the Installer (also a good reminder to my own module suggestion)

rbrown commented 10 years ago

Hi, just to let you know I've run into a variant of this issue using capistrano: If a package doesn't depend on magento-composer-installer there's no guarantee which order the packages will be installed by composer. I see that I can call https://github.com/magento-hackathon/composer-command-integrator after composer install to fix this, but it wasn't very clear to me.

tkdb commented 10 years ago

@rbrown: Can you say if that is the case, too when the package does not require but suggests the installer as package?

rbrown commented 10 years ago

Yeah, I did try that, suggest doesn't affect dependancy resolution, so the order remains random.

tkdb commented 10 years ago

@rbrown: Thanks for the feedback. And do you require the composer-installer package your own next to having all other magento extension packages suggest the installer?

rbrown commented 10 years ago

Yeah, sorry, tried that too, it doesn't matter because the package that doesn't depend on the installer still doesn't depend on the installer, so it can be installed whenever

tkdb commented 10 years ago

Yes, I can imagine, I had some hope that composer would take the suggestion into account. If for example a package suggests magento-composer-installer and the main composer.json does require it, that composer would prefer to fulfil the requirement first and then install those packages that have it as a suggested dependency. If this isn't the case with composer it might be worth to suggest this upstream - at least this way of processing would feel "natural" to me personally.

Edit: Now as I write this down, I'm just thinking that even if the magento-composer-installer is installed first but within the same phase, that this might not even solve your problem. Just let me know.

Edit2: It just popped into my mind that there is something called composer meta package which I don't know if it is suitable in this situation or not.

Flyingmana commented 10 years ago

fixed in 3.0.0-alpha.2