Ocramius / PackageVersions

:package: Composer addon to efficiently get installed packages' version numbers
MIT License
3.22k stars 67 forks source link

Ignore the global plugin when updating local projects #39

Closed AydinHassan closed 7 years ago

AydinHassan commented 8 years ago

I had to refactor a little to reduce repeating code, anyway this fixes #38.

However, I've had a think and I don't know how we can provide an e2e test. We could provide some project assets but we'd have to mess with the systems global composer project, which I don't think we can do safely.

I've also had to modify existing tests, I wasn't sure how to get them working now that we need ocramius/package-versions to be present in the locker.

Thoughts @Ocramius ?

AydinHassan commented 8 years ago

This patch also has the added benefit of fixing another crash. After removing ocramius/package-versions the plugin still runs an attempts to generate the version file, but the folder does not exist anymore.

Running: composer remove ocramius/package-versions (before this patch) results in:

Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Removing ocramius/package-versions (1.1.1)
Package guzzle/guzzle is abandoned, you should avoid using it. Use guzzlehttp/guzzle instead.
Writing lock file
Generating autoload files
ocramius/package-versions:  Generating version class...

Removal failed, reverting ./composer.json to its original content.

  [ErrorException]                                                                                                                                                               
  file_put_contents(/Users/aydin/projects/package-versions-test/vendor/ocramius/package-versions/src/PackageVersions/Versions.php): failed to open stream: No such file or directory  

remove [--dev] [--no-progress] [--no-update] [--no-scripts] [--update-no-dev] [--update-with-dependencies] [--no-update-with-dependencies] [--ignore-platform-reqs] [-o|--optimize-autoloader] [-a|--classmap-authoritative] [--] [<packages>]...

Admittedly this can be circumvented using --no-plugins but I think this is slightly more elegant and provides the consumer with no surprises.

AydinHassan commented 8 years ago

@Ocramius managed to get an E2E test working - it's a bit gross so I'd appreciate any pointers for it 😄

AydinHassan commented 8 years ago

Seems to be failing on lowest deps - trying to figure out now

AydinHassan commented 8 years ago

Hmm - the composer exec output seems to print errors such as

Fatal error: Uncaught Error: Class 'Symfony\Component\Process\ProcessUtils' not found in /Users/aydin/projects/PackageVersions/vendor/composer/composer/src/Composer/Util/ProcessExecutor.php on line 124

On further investigation I would say this is a bug with Composer:

\Composer\Util\ProcessExecutor uses Symfony\Component\Process\ProcessUtils from symfony/process. Composer constraint on symfony/process is "^2.1 || ^3.0" but it seems ProcessUtils was only introduced in 2.3

Ocramius commented 8 years ago

On further investigation I would say this is a bug with Composer:

We should then report it in composer and bump the required version there, I suppose...

AydinHassan commented 8 years ago

I've filed https://github.com/composer/composer/issues/5828 & created https://github.com/composer/composer/pull/5831 to fix it (maybe). I've bumped the minimum in here to ^2.3 also - should get the tests running.

AydinHassan commented 8 years ago

OK composer/composer#5831 is merged now - guessing you want to wait for a tag before we pursue this?

AydinHassan commented 7 years ago

hey @Ocramius composer 1.3 has been released with the necessary fixes. Tests are passing now 😄 .

Ocramius commented 7 years ago

@AydinHassan looking good! Going through a quick sweep of the code, then merging :-)

Ocramius commented 7 years ago

LGTM! \o/

weirdan commented 6 years ago

@samsonasik are there chances this will get backported to your fork? There's no way to report issues on forks, unfortunately.

samsonasik commented 6 years ago

@weirdan I tried backport but it seems needs very long time to get it work as I state at https://github.com/samsonasik/PackageVersions/pull/3#issuecomment-335073563 . Pull Request open if you can invest a time to patch ;)

Ocramius commented 6 years ago

I know @samsonasik did the backport with good intentions, but if you want to talk about older PHP versions support here, I'm gonna lock the thread: just upgrade your installation to a contemporary PHP binary.

weirdan commented 6 years ago

Oh, we were merely talking about porting changes from upstream to the fork. And I'm running 7.1, which, last time I checked, was pretty contemporary.

Ocramius commented 6 years ago

Then you don't need the fork ;-)