franzliedke / studio

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

Autoload Directory Seperator on Windows / Autoloading not doing anything? #30

Closed Rjs37 closed 8 years ago

Rjs37 commented 9 years ago

As you can see from the below example, the auto-generated line from Studio in the vendor/autoload.php file seems to produce an incorrect file-path (actually uses a mixture of separators):

require_once __DIR__ . '/../package\vendor\autoload.php';

This results in a fatal error due to the require failing. The fatal error goes away once all the back slashes are changed to forward slashes. I'm using Windows 10.

franzliedke commented 9 years ago

Hmm, it's probably caused by mixing the two types of slashes, right?

franzliedke commented 9 years ago

Something like str_replace(['\\', '/'], DIRECTORY_SEPARATOR, $file) should then do the trick, right?

Rjs37 commented 9 years ago

Should do yeah

franzliedke commented 9 years ago

Wait a second, which version are you using? I changed this a long time ago, to edit the autoload_XXX.php files instead of the global vendor/autoload.php file.

Please update to dev-master. Will push a new release soon.

Rjs37 commented 9 years ago

Edited after I just read a comment of yours on another issue.

Ah sorry, I was using the latest release. Just tried dev-master instead, I can see that it makes some changes to the files in vendor/composer, but it's not autoloading my package at all. Which it seems you're already aware of.

As I needed a quick fix, I've gone back to the latest release as I can at least get that version working how I need (even though composer update overwrites the changes - which I assume is why you made the changes you described).

I used the change described in the "Avoid duplicate dependencies" issue so that I can work properly on my local.

franzliedke commented 9 years ago

Hmm, interesting. Is the package that you manage with Composer available somewhere? I'd love to see why it isn't working. Are you using the "files" option in Composer's autoloading configuration? Can I see your composer.json (the one from the package)?

Rjs37 commented 9 years ago

I assumed the autoloading not working was related to that other issue that was open. And nah it's a privately hosted package that we use as a base for multiple sites.

The Site/Project composer.json is just what Laravel 5.1 provided with the Laravel and PHP dependencies replaced by the dependency for my package.

kaidesu commented 9 years ago

Can confirm this issue on Windows 10 as well.

screenshot1

Fresh Laravel installation and tried loading my Caffeinated Modules package here: https://github.com/caffeinated/modules

Rjs37 commented 9 years ago

I've had time to do some more experimentation on my laptop (also Windows 10) and got some more observations using dev-master.

My host computer is Windows (and that's where I've globally installed studio) but I'm using vagrant (homestead). I didn't want to globally install studio within the vagrant instance as I would have to do that every time I recreate the instance.

If I manually add the below to autoload_namespaces.php then everything works as I need, until the next composer update

'Pgn\\Phoenix\\' => array ('/home/vagrant/projects/pgn/phoenix/src'),

Not sure why studio didn't change the namespaces file automatically but even if it did, we'd still have the problem between host machine and vagrant instance and I'm not sure what the best solution to that is. Recommend to require studio within the instance?

franzliedke commented 9 years ago

Recommend to require studio within the instance?

For now, yes, please do that.

I should really make sure that I do not resolve the __DIR__ constants in the autoloader files. That should then fix that problem.

composer update still wipes out the changes to both (namespaces and psr4) of those files. Is there a way to avoid this?

If I understand you correctly, then this was fixed in #26. Until I release a new version, please update to the latest dev-master version of Studio.

Rjs37 commented 9 years ago

If I understand you correctly, then this was fixed in #26. Until I release a new version, please update to the latest dev-master version of Studio.

I was on dev-master but because I was running composer update within the instance but studio from the host (I'm awkward lol) I didn't see this behaviour.

Now that I've installed and using studio within the instance, I see studio doing extra stuff during composer update and can see that it has installed all the subpackages but:

EDIT: Actually now that I've installed studio within the instance. Making that one manual change isn't enough. I also need to update the filepaths in autoload_classmap.php too. Should Studio be editing this file too?

Rjs37 commented 9 years ago

With it being a Laravel project (with my package as a dependency), the php artisan optimize command in the post-update-cmd hook causes the behaviour mentioned in my previous edit. It populates the classmap file with file references to the classes from the project. Meaning I need to update those file references.

Possibly needs a post-update-cmd specifically for studio, or we can't have the optimize command there?

Though the namespace.php file not being updated by Studio is a separate problem to that.

Rjs37 commented 9 years ago

I've found what my main issue is. The use of array_diff_keys in autoloadFrom (StudioPlugin.php). It means it doesn't even try to add the override for my package to the autoload_namespaces file.

If I get rid of that function call then it adds entries for all the package dependencies too. Whereas I just need my own package. Not sure on the best way to do that though.

franzliedke commented 8 years ago

This is now finally fixed. I threw out all the autoloader merging and now used Composer's path repositories that can be used to register local folders as packages instead. Works wonderfully.

I will publish a new release very soon. :)

Rjs37 commented 8 years ago

Sounds, very promising! Looking forward to it :).

I'll test it on my laptop once it's released ;). I've got it working pretty well on my PC atm (just an occasional manual file change) and don't want to risk breaking my development flow lol.

franzliedke commented 8 years ago

Hmm, I hope this works on Windows. Since the path repositories work with symlinks, I think the files are only copied (and not synced) on Windows...

Rjs37 commented 8 years ago

So using dev-master I should see the fixed behavior? Or are there any commits still waiting?

I'm assuming the symlink is meant to be vendor/package > my_package_directory yeah?

When I run composer update, I see the [Studio] Registering package... output.

But when I SSH into my vagrant box and run ls -l, I can't see any symlinked directories. I'm running all of my commands from within the vagrant box, that includes the studio load and the composer update.

I've tried a dd call in my package service provider and it's not having any affect so it's still looking at the vendor folder version.

Vagrant syncs my project files between itself and Windows, could that be conflicting? Have you done any testing using Vagrant or Homestead?

franzliedke commented 8 years ago

Hmm, Windows will be a problem with symlinks.

(That said, my solution currently works only for packages that haven't been registered with Packagist yet. Once it does, it will probably copy the entire folder instead of symlinking it on Windows...)

Rjs37 commented 8 years ago

I didn't pick up on the fact that this update requires a manual change to composer.json until I saw a comment on a different issue. I'd assumed it was an automatic symlink that Studio setup.

I'm sure you'll explain that properly in the readme doc once the updates are tagged as a release.

Here are my experiences after that realization:

As my package is a private one it doesn't exist in Packagist, instead I've got a vcs url to it. If I add the path line instead of the vcs one, then composer clears out the existing package.

Composer install never completed, it just got stuck during the installation of the package, though it did seem to have copied all the files across. No sym links as far as I could see but as I said the composer install never finished so that may be why.

If it could work without making changes to the composer.json then that definitely would be a step in the right direction. Whether it'd work on Windows or not though is another question entirely :D.

As the composer.json belongs to my applications repository I can't just remove the vcs reference from the file. For the time being I'll carry on as I was doing, the autoloader solution reliably works, even if it requires an occasional manual change.

franzliedke commented 8 years ago

Yep, the idea is to set up "path repositories" without needing to explicitly define them in the composer.json or the global Composer config.

Rjs37 commented 8 years ago

That'd be great. Good luck with that!

Guessing the main difficulty there would be getting the path based package to take priority over other sources e.g. Packagist (as you mentioned), vcs repositories etc

franzliedke commented 8 years ago

That is indeed the main problem. And the one I currently cannot implement without a PR to Composer itself. Need to get on that ASAP.