franzliedke / studio

A workbench for developing Composer packages.
MIT License
1.14k stars 73 forks source link

Avoid duplicate dependencies #14

Closed franzliedke closed 8 years ago

franzliedke commented 9 years ago

Studio currently suffers from the same problem that Workbench had: libraries in development with some of the same dependencies of the main project can cause conflicts with autoloading.

This needs to be solved by messing around with the Composer-generated files.

Here's my idea:

/cc @kirkbushell

kirkbushell commented 9 years ago

@franzliedke hmmm, I like where you're going with this...

Personally I just don't work in a composer directory, or via workbench. I have a few cli tools that simply synchronize or update where necessary.

I honestly think you may be wasting your time here, but I'm happy to be proven wrong :)

franzliedke commented 9 years ago

Made some progress on this today. Coming this week. =)

RemiCollin commented 9 years ago

What would be the prefered behaviour when the package you're working on is a part of the main project dependency ? I often work on package while developping application, which mean these packages are listed as dependencies in the composer.json. Right now my workaround is to maintain 2 composer.json files, one for the dev version and one for the production app. Is it possible for studio to preffer the managed package over main project dependencies ?

franzliedke commented 9 years ago

While you work on a dependency using Studio, you should not have it in your dependency list in composer.json.

janhartigan commented 9 years ago

So a way around this weirdness is to do this with your composer:

composer config -g prepend-autoloader false

Or alternatively in your local composer json:

composer config prepend-autoloader false

This likely causes issues with other composer plugins because it changes the order of the autoloader stack. However, in my case I don't use any others and that then makes this package work like Workbench. I still think it's a good idea to try to make this package load its autoloader after the primary one. The issue seems to be with how it's modifying Composer's autoload file, but I'm not sure about best practices there. Composer has that return command which confuses things. This is how it normally looks after running the dumpautoload for this package:

// autoload.php @generated by Composer

require_once __DIR__ . '/composer' . '/autoload_real.php';

// @generated by Composer Studio (https://github.com/franzliedke/studio)

require_once __DIR__ . '/../../../API/repo\vendor\autoload.php';

return ComposerAutoloaderInit17d3a149ef691d117c0e0052e62c942d::getLoader();

And this is how it would ideally look if it were to work exactly like Workbench:

// autoload.php @generated by Composer

require_once __DIR__ . '/composer' . '/autoload_real.php';

$loader = ComposerAutoloaderInit17d3a149ef691d117c0e0052e62c942d::getLoader();

// @generated by Composer Studio (https://github.com/franzliedke/studio)

require_once __DIR__ . '/../../../API/repo\vendor\autoload.php';

return $loader;

Thoughts, @franzliedke?

franzliedke commented 9 years ago

@janhartigan I'm confused. The method you linked to takes care of merging the type-specific autoload files (for "psr0", "psr4" etc.) from managed packages.

But the autoload.php file itself is not even touched anymore with these changes.

So what are you suggesting? :)

janhartigan commented 9 years ago

@franzliedke I probably misunderstood how the autoload file is modified by this package. Is it a feature of composer's plugin architecture?

In any case, I was referring to the problem that arises with what @RemiCollin mentioned. I have 3 separate applications that have a composer dependency on one of our shared libraries. These applications all have the composer.json dependency and we can't easily get rid of that. In order to make development easier, we've so far used the Workbench to let composer prepend the shared library's autoloader. This would mean that any time one of our package's classes were loaded composer would prefer the one that was loaded second.

Because of the bit of code I wrote out above, Studio doesn't allow for this unless you tell composer to append (rather than prepend) subsequently-loaded autoloaders. It's the ::getLoader() function, after all, that initializes the spl_autoload_register() method, so with the way it currently is set up, the Studio packages' autoload runs first, and the application's autoload runs after.

janhartigan commented 9 years ago

And just to clarify, this previously worked under Workbench because Workbench was loaded by the Laravel application well after the autoload.php file ran the first getLoader(). This would mean that the Workbench packages' spl_autoload_register() would prepend onto the autoloading stack (as is the default behavior in composer).

janhartigan commented 9 years ago

It's also worth mentioning that I'm not totally against the idea of keeping it the way it is, but perhaps with a note to people about modifying the composer config to append autoloaders. One problem with prepending has always been that it totally bungles autoloaded helper function files by loading the files in the vendor directory before the ones in the workbench. Interested to hear you thoughts on this.

Rjs37 commented 9 years ago

I think until a decision has been made, this behaviour (or lack thereof) should be described in the readme. This confused me quite a bit until I came across this issue and applied the suggested change from @janhartigan

If I do a composer update on the package then all hell breaks loose and I get duplicate dependencies but I'm just avoiding that and only doing the composer updates on my main project. With that it seems to work as I need it to.

EDIT: I didn't realise that the way things are being done had been changed already when I said the above.

franzliedke commented 8 years ago

This is now not a problem anymore.

Managed packages will not be hacked into the autoloader files anymore, but instead they will be symlinked into the vendor dir. Composer already provides this for free using path repositories.

Next release is coming up. :)

janhartigan commented 8 years ago

@franzliedke awesome, thank you very much!

janhartigan commented 8 years ago

@franzliedke so out of curiosity, would studio use a behind-the-scenes composer method for adding a new path repository so that we wouldn't have to change our composer.json?

franzliedke commented 8 years ago

@janhartigan Yes, indeed. Unfortunately, it only works for packages that haven't been registered with Packagist, so far... :(

I'm on it, though. Might need a new event or method in Composer to get this working.

franzliedke commented 8 years ago

I'll post here when I need your lobby for my Composer PR ;)

janhartigan commented 8 years ago

@franzliedke ok excellent. Let me know and I'll do what I can.

franzliedke commented 8 years ago

I'll post here when I need your lobby for my Composer PR ;)

That time has finally come. I managed to send a pull request to Composer. The change was super simple, if I had known that earlier...

Now I need your support to get it into Composer. :)

janhartigan commented 8 years ago

I must have enormous power, because they merged it without my even saying anything.

Great work, Franz!

franzliedke commented 8 years ago

Yay, the release is on its way. I still need to go through everything again and prepare a blog post, but it will happen this week. :)