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

Fix "Package seems not been installed properly" #87

Open
wants to merge 4 commits into
base: 0.14
Choose a base branch
from
Open

Fix "Package seems not been installed properly" #87

wants to merge 4 commits into from

Conversation

ryzr
Copy link

@ryzr ryzr commented Sep 19, 2018

The installationSource and distType on original packages are reconfigured in the uninstall/install step. This prevents the "Package seems not been installed properly" error, and also "The package has modified files" warning after running composer update. I also made a change to the loading/detection of Studio packages in the pre/post autoload phases. This should ensure consistency and no clashes with other plugins.

I'm not sure what the purpose of the /compost/studio/installed.json file is (except for debugging), but things should run fine without it. I've left it there for now, as it may be valuable info to some.

I was able to composer install, composer update and composer dumpauto without any errors. I made changes to the autoload config in my symlinked packages, and it was reflected in the composer/autoload_x files.

It should be noted however, that I only tested with 'library' package types, downloaded from source (Bitbucket). I'm not sure what troubles might arise with dist packages or other composer package types.

The installationSource and distType on original packages are reconfigured in the uninstall/install step. This prevents the "Package seems not been installed properly" error, and also "The package has modified files" warning after running `composer update`.
Update StudioPlugin.php
@julienfalque
Copy link

I confirm this fixes the "Package seems not been installed properly" error.

I noticed something odd in the output though:

 $ composer update foo/foo-bundle
Loading composer repositories with package information
Updating dependencies (including require-dev)                                        
Package operations: 1 install, 0 updates, 0 removals
  - Installing foo/foo-bundle (dev-some-branch 455d8a2): Cloning 455d8a2cab from cache
Writing lock file
Generating autoload files
[Studio] Loading package foo/foo-bundle
ocramius/package-versions:  Generating version class...
ocramius/package-versions: ...done generating version class
  - Removing foo/foo-bundle (dev-some-branch 455d8a2)
  - Installing foo/foo-bundle (0.4.x-dev): Symlinking from ../foo-bundle

In my composer.json I require foo/foo-bundle: dev-some-branch and composer.lock is updated correctly using the latest commit from some-branch but when the installed package is replaced with a symlink to the local repository (on some-branch), the output shows version 0.4.x-dev is installed, which is the alias of the master branch. Is this expected?

$original = $installed->findPackage($package->getName(), '*');

// Change the source type to path, to prevent 'The package has modified files'
$original->setInstallationSource('dist');
$original->setDistType('path');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line raises an error when $original is an instance of AliasPackage:

PHP Fatal error:  Uncaught Error: Call to undefined method Composer\Package\AliasPackage::setDistType()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for highlighting this. I've just run into this issue myself today. I'm now testing out a fix with my latest commit.

This does highlight a valid concern about the many configurations possible with composer. My workflow is simple, with packages installed through Packagist or VCS by tag or by branch. I'm sure theres plenty of other possibilities that my proposed change would not work for. That said - I'm not exactly sure how many of those workflows were possible before my commit?

@julienfalque
Copy link

These changes seem to have a drawback: requiring a dev branch in composer.json (e.g. dev-master) only works if the branch exists on the remote Git repository. IIRC this used to be possible even when the branch only exists on the local repository.

$installationManager->getInstaller($package->getType())
->install($studioRepo, $package);
$installationManager->getInstaller($original->getType())->uninstall($installed, $original);
$installationManager->getInstaller($package->getType())->install($LocalPackagesRepo, $package);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryzr I am getting issues since this commit. $LocalPackagesRepo is not defined. I have PR'ed to your fork https://github.com/ryzr/studio/pull/1

Fix for undefined LocalPackagesRepo variable
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 this pull request may close these issues.

3 participants