Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove lib/* from modman #68

Open
mklooss opened this issue Jun 23, 2014 · 15 comments
Open

remove lib/* from modman #68

mklooss opened this issue Jun 23, 2014 · 15 comments

Comments

@mklooss
Copy link
Contributor

mklooss commented Jun 23, 2014

and add a modman script in to this Credis Repository.

so i think it is easier to handle this in projects

@sergis
Copy link

sergis commented Jan 22, 2015

I've the same issue with magento-composer-installer.
It makes lib/Credis empty instead of library fetch
I suppose it's better use composer 'require' for redis library

@mklooss
Copy link
Contributor Author

mklooss commented Jan 22, 2015

@colinmollenhour
Copy link
Owner

@Mkloos or @davidverholen Can you propose a solution that meets the following requirements:

  • Installs neatly with standalone modman
  • Installs neatly with composer
  • Overrides Magento core without clobbering the core files

I don't use composer so don't really know the best way to accomplish this. Note, since composer doesn't use submodules I see no harm in keeping the submodules intact for proper modman support.

@davidverholen
Copy link

the problem is, that the magento composer installer overrides the lib/credis folder with a symlink to an empty folder, because it will not install the submodule (since composer has its own mechanism to require dependencies dynamically, which is way better then using submodules)

it should be possible, to use the composer.json map property to make a composer specific mapping, since the magento composer installer from magento-hackathon first looks for this property and only if it is not there it looks for package.xml and finally modman file.

Then you could require the credis library in the composer.json.

The only Problem left would be, that the credis library could not be loaded, since it would not be deployed to the lib dir. You could solve this, by changing the package type of the credis library to magento-module and add a composer mapping there.

Since the magento composer installer class is extended from composers library installer, and there is a fallback to the library installer, it also should not make any difference when the library is installed somewhere, where no magento composer installer is present

@colinmollenhour
Copy link
Owner

The Credis code is in no way specific to Magento so I'd like to avoid making the composer file depend on magento-composer-installer.. Is there any way to do this?

Everyone is always saying that composer is so great at handling dependencies and submodules suck but it seems composer is very limited without the magento-composer-installer...

@davidverholen
Copy link

it's not composer but magento that makes it so difficult here. Putting the credis library in the core without updating it periodically. Composer is the best thing that could happen to php developers.

We use a module for magento that can autoload libraries that are installed by composer. With this module I would just have to delete the credis Library from Magento lib to load the right one (with automatic updates over composer).

So it may be possible to keep your modman mapping, so the magento credis library is deleted and then use the hackathon psr0autoloader (for example) to load the credis library over composer.

However, I think I will keep using forks to Install this module till magento2 (maybe even you will use composer then? ;) )

@colinmollenhour
Copy link
Owner

So can composer be used to map the Credis root to app/code/community/Credis/? This way Magento would use it over the lib/Credis copy.

@davidverholen
Copy link

yes, but it has to use another installer, since the default installer only creates a copy in the vendor dir. To use another installer you have to change the package type to (for example) magento-module

@ayalon
Copy link

ayalon commented May 13, 2015

I have the same problem with composer and the empty folder in lib/Credis.
@mklooss: I saw, that you forked and made some changes.
How can I use your fork in my magento installation? I added your URL to the vcs parameter but it always takes the original repository.

Could you provide your composer.json how to use the forked fixed version?

@ambimax
Copy link

ambimax commented Oct 18, 2015

You can add scripts to your main composer.json taking care of submodules:

    "scripts": {
        "post-install-cmd": [
            "cd .modman/cache-backend-redis/ && git submodule update --init --recursive"
        ],
        "post-update-cmd": [
            "cd .modman/cache-backend-redis/ && git submodule update --init --recursive"
        ]
    }

@danelowe
Copy link

I ended up making a script to sort out the submodule init. If I get bored in the weekend, I might learn how to create a full composer plugin.

  "autoload": {
    "psr-4": {
      "TradeTestedComposer\\": "tools/composer/tradetested/src/"
    }
  },
  "scripts": {
    "post-package-install": "TradeTestedComposer\\Scripts::postPackageInstall",
    "post-package-update": "TradeTestedComposer\\Scripts::postPackageInstall"
  },
<?php
namespace TradeTestedComposer;
use Composer\Installer\PackageEvent;
use Composer\Util\ProcessExecutor;
use Composer\Util\FileSystem;
use Composer\Util\Git;
class Scripts
{

    public static function postPackageInstall(PackageEvent $event)
    {
        $operation = $event->getOperation();
        /** @var Composer\Package $package */
        $package = ($operation->getJobType() == 'install') ? $operation->getPackage() : $operation->getTargetPackage();
        if (($package->getInstallationSource() == 'source') && ($package->getSourceType() == 'git')) {
            $path = $event->getComposer()->getInstallationManager()
                ->getInstaller($package->getType())
                ->getInstallPath($package);
            $io = $event->getIO();
            $process = new  ProcessExecutor($io);
            $output = '';
            $process->execute('git submodule update --init', $output, $path);
        }
    }
}

EcomDev_PHPUnit also uses submodules.

@peterjaap
Copy link

Can this finally be fixed?

@colinmollenhour
Copy link
Owner

@peterjaap Do you have a proposed solution that doesn't break standalone modman installs? I wish it wasn't so hard but with composer using the modman file but only partially supporting modman's functionality there is a catch-22. Modman depends on submodules for dependencies and I don't see any way around that except for perhaps using git subtrees which is also ugly.

@peterjaap
Copy link

peterjaap commented Jun 22, 2017

Proposed solution is;

  • Remove lib from this repo
  • Put colinmollenhour/credis in as a (suggested?) dependency in composer.json
  • Add a modman file to colinmollenhour/credis to place the files in lib/Credis
  • Profit!

This way, this extension can be used for Magento <1.8 in conjunction with colinmollenhour/credis and can just colinmollenhour/credis be used for Magento >=1.8 since that has Mage_Cache_Backend_Redis.

For non-Magento projects, the modman file in colinmollenhour/credis will just be ignored so no penalty there.

@mklooss
Copy link
Contributor Author

mklooss commented Jun 23, 2017

your right, but the list is not correct ;-)

Proposed solution is;

  • Remove lib from this repo
  • Put colinmollenhour/credis in as a (suggested?) dependency in composer.json
  • Add a modman file to colinmollenhour/credis to place the files in lib/Credis
  • Profit!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants